CyberCoveLLC/threshold
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

perf(session): Remove connections from session list endpoint

Browse Source 
Including the nested connection information in the session list response
had a negative impact on performance as it created a significant
multiplier on the number of rows that needed to be fetched from the
database.

It was also determined that this information was used by neither the
admin ui nor the desktop client.

Note that nested connection information can still be retrieved via a
read request for a specific session.
This commit is contained in:
Timothy Messier committed 2022-06-09 19:28:04 +00:00
1 parent 5b0abe2e27
commit 32070678dc
6 files changed
+46 -61
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/daemon/controller/handlers/sessions/session_service_test.go
+5 -25
View File
@@ -281,7 +281,7 @@ func TestList(t *testing.T) {
Endpoint: "tcp://127.0.0.1:22",
})
c := session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1")
session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1")
status, states := convertStates(sess.States)
@@ -304,14 +304,7 @@ func TestList(t *testing.T) {
Certificate: sess.Certificate,
Type: tcp.Subtype.String(),
AuthorizedActions: testAuthorizedActions,
Connections: []*pb.Connection{
{
ClientTcpAddress: c.ClientTcpAddress,
ClientTcpPort: c.ClientTcpPort,
EndpointTcpAddress: c.EndpointTcpAddress,
EndpointTcpPort: c.EndpointTcpPort,
},
},
Connections: []*pb.Connection{}, // connections should not be returned for list
})
totalSession = append(totalSession, wantSession[i])
@@ -326,7 +319,7 @@ func TestList(t *testing.T) {
Endpoint: "tcp://127.0.0.1:22",
})
c = session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1")
session.TestConnection(t, conn, sess.PublicId, "127.0.0.1", 22, "127.0.0.2", 23, "127.0.0.1")
status, states = convertStates(sess.States)
@@ -349,14 +342,7 @@ func TestList(t *testing.T) {
Certificate: sess.Certificate,
Type: tcp.Subtype.String(),
AuthorizedActions: testAuthorizedActions,
Connections: []*pb.Connection{
{
ClientTcpAddress: c.ClientTcpAddress,
ClientTcpPort: c.ClientTcpPort,
EndpointTcpAddress: c.EndpointTcpAddress,
EndpointTcpPort: c.EndpointTcpPort,
},
},
Connections: []*pb.Connection{}, // connections should not be returned for list
})
}
@@ -423,14 +409,8 @@ func TestList(t *testing.T) {
require.Equal(len(tc.res.GetItems()), len(got.GetItems()), "Didn't get expected number of sessions: %v", got.GetItems())
for i, wantSess := range tc.res.GetItems() {
assert.True(got.GetItems()[i].GetExpirationTime().AsTime().Sub(wantSess.GetExpirationTime().AsTime()) < 10*time.Millisecond)
assert.Equal(1, len(wantSess.GetConnections()))
assert.Equal(0, len(wantSess.GetConnections())) // no connections on list
wantSess.ExpirationTime = got.GetItems()[i].GetExpirationTime()
for _, c := range got.GetItems()[i].GetConnections() {
assert.Equal("127.0.0.1", c.ClientTcpAddress)
assert.Equal(uint32(22), c.ClientTcpPort)
assert.Equal("127.0.0.2", c.EndpointTcpAddress)
assert.Equal(uint32(23), c.EndpointTcpPort)
}
}
}
assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListSessions(%q) got response %q, wanted %q", tc.req, got, tc.res)
internal/db/schema/migrations/oss/postgres/20/09_session_list_view.up.sql
+1
View File
@@ -1,6 +1,7 @@
begin;
-- Replaces the view created in 1/01 to include connections
-- Replaced in 31/0_session_list_no_connections
drop view session_with_state;
create view session_list as
select
internal/db/schema/migrations/oss/postgres/31/02_session_list_no_connections.up.sql
+38
View File
@@ -0,0 +1,38 @@
begin;
-- Replaces the view created in 2/09_session_list_view
drop view session_list;
create view session_list as
select
s.public_id,
s.user_id,
s.host_id,
s.server_id,
s.server_type,
s.target_id,
s.host_set_id,
s.auth_token_id,
s.scope_id,
s.certificate,
s.expiration_time,
s.connection_limit,
s.tofu_token,
s.key_id,
s.termination_reason,
s.version,
s.create_time,
s.update_time,
s.endpoint,
s.worker_filter,
ss.state,
ss.previous_end_time,
ss.start_time,
ss.end_time
from
session s
join
session_state ss
on
s.public_id = ss.session_id;
commit;
internal/session/repository.go
-25
View File
@@ -83,8 +83,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi
return nil, nil
}
sessions := []*Session{}
// deduplication map of connections by connection_id
connections := map[string]*Connection{}
// deduplication map of states by end_time
states := map[*timestamp.Timestamp]*State{}
var prevSessionId string
@@ -92,10 +90,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi
for _, sv := range sessionList {
if sv.PublicId != prevSessionId {
if prevSessionId != "" {
for _, c := range connections {
workingSession.Connections = append(workingSession.Connections, c)
}
connections = map[string]*Connection{}
for _, s := range states {
workingSession.States = append(workingSession.States, s)
}
@@ -157,22 +151,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi
}
}
if sv.ConnectionId != "" {
if _, ok := connections[sv.ConnectionId]; !ok {
connections[sv.ConnectionId] = &Connection{
PublicId: sv.ConnectionId,
SessionId: sv.PublicId,
ClientTcpAddress: sv.ClientTcpAddress,
ClientTcpPort: sv.ClientTcpPort,
EndpointTcpAddress: sv.EndpointTcpAddress,
EndpointTcpPort: sv.EndpointTcpPort,
BytesUp: sv.BytesUp,
BytesDown: sv.BytesDown,
ClosedReason: sv.ClosedReason,
}
}
}
}
for _, s := range states {
workingSession.States = append(workingSession.States, s)
@@ -180,9 +158,6 @@ func (r *Repository) convertToSessions(ctx context.Context, sessionList []*sessi
sort.Slice(workingSession.States, func(i, j int) bool {
return workingSession.States[i].StartTime.GetTimestamp().AsTime().After(workingSession.States[j].StartTime.GetTimestamp().AsTime())
})
for _, c := range connections {
workingSession.Connections = append(workingSession.Connections, c)
}
sessions = append(sessions, workingSession)
return sessions, nil
}
internal/session/repository_session_test.go
+2 -1
View File
@@ -126,7 +126,8 @@ func TestRepository_ListSession(t *testing.T) {
require.NoError(err)
assert.Equal(tt.wantCnt, len(got))
for i := 0; i < len(got); i++ {
assert.Equal(tt.withConnections, len(got[i].Connections))
// connections should not be returned for list requests
assert.Equal(0, len(got[i].Connections))
for _, c := range got[i].Connections {
assert.Equal("127.0.0.1", c.ClientTcpAddress)
assert.Equal(uint32(22), c.ClientTcpPort)
internal/session/session.go
-10
View File
@@ -472,16 +472,6 @@ type sessionListView struct {
PreviousEndTime *timestamp.Timestamp `json:"previous_end_time,omitempty" gorm:"default:current_timestamp"`
StartTime *timestamp.Timestamp `json:"start_time,omitempty" gorm:"default:current_timestamp;primary_key"`
EndTime *timestamp.Timestamp `json:"end_time,omitempty" gorm:"default:current_timestamp"`
// Connection fields
ConnectionId string `json:"connection_id,omitempty" gorm:"default:null"`
ClientTcpAddress string `json:"client_tcp_address,omitempty" gorm:"default:null"`
ClientTcpPort uint32 `json:"client_tcp_port,omitempty" gorm:"default:null"`
EndpointTcpAddress string `json:"endpoint_tcp_address,omitempty" gorm:"default:null"`
EndpointTcpPort uint32 `json:"endpoint_tcp_port,omitempty" gorm:"default:null"`
BytesUp uint64 `json:"bytes_up,omitempty" gorm:"default:null"`
BytesDown uint64 `json:"bytes_down,omitempty" gorm:"default:null"`
ClosedReason string `json:"closed_reason,omitempty" gorm:"default:null"`
}
// TableName returns the tablename to override the default gorm table name