mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: exclude provisioner_state from workspace_build_with_user view (#22159)
The provisioner state for a workspace build was being loaded for every long-lived agent rpc connection. Since this state can be anywhere from kilobytes to megabytes this can gradually cause the `coderd` memory footprint to grow over time. It's also a lot of unnecessary allocations for every query that fetches a workspace build since only a few callers ever actually reference the provisioner state. This PR removes it from the returned workspace build and adds a query to fetch the provisioner state explicitly.
This commit is contained in:
@@ -452,7 +452,7 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
|
||||
// to read all provisioner daemons. We need to retrieve the eligible
|
||||
// provisioner daemons for this job to show in the UI if there is no
|
||||
// matching provisioner daemon.
|
||||
provisionerDaemons, err := b.store.GetEligibleProvisionerDaemonsByProvisionerJobIDs(dbauthz.AsSystemReadProvisionerDaemons(b.ctx), []uuid.UUID{provisionerJob.ID})
|
||||
provisionerDaemons, err := b.store.GetEligibleProvisionerDaemonsByProvisionerJobIDs(dbauthz.AsWorkspaceBuilder(b.ctx), []uuid.UUID{provisionerJob.ID})
|
||||
if err != nil {
|
||||
// NOTE: we do **not** want to fail a workspace build if we fail to
|
||||
// retrieve provisioner daemons. This is just to show in the UI if there
|
||||
@@ -570,8 +570,8 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
|
||||
}
|
||||
}
|
||||
if b.state.orphan && !hasActiveEligibleProvisioner {
|
||||
// nolint: gocritic // At this moment, we are pretending to be provisionerd.
|
||||
if err := store.UpdateProvisionerJobWithCompleteWithStartedAtByID(dbauthz.AsProvisionerd(b.ctx), database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{
|
||||
// nolint: gocritic // User won't necessarily have the permission to do this so we act as a system user.
|
||||
if err := store.UpdateProvisionerJobWithCompleteWithStartedAtByID(dbauthz.AsWorkspaceBuilder(b.ctx), database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{
|
||||
CompletedAt: sql.NullTime{Valid: true, Time: now},
|
||||
Error: sql.NullString{Valid: false},
|
||||
ErrorCode: sql.NullString{Valid: false},
|
||||
@@ -815,7 +815,12 @@ func (b *Builder) getState() ([]byte, error) {
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get last build to get state: %w", err)
|
||||
}
|
||||
return bld.ProvisionerState, nil
|
||||
// nolint: gocritic // Workspace builder needs to read provisioner state for the new build.
|
||||
state, err := b.store.GetWorkspaceBuildProvisionerStateByID(dbauthz.AsWorkspaceBuilder(b.ctx), bld.ID)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get workspace build provisioner state: %w", err)
|
||||
}
|
||||
return state.ProvisionerState, nil
|
||||
}
|
||||
|
||||
func (b *Builder) getParameters() (names, values []string, err error) {
|
||||
|
||||
@@ -65,6 +65,7 @@ func TestBuilder_NoOptions(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(nil),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -124,6 +125,7 @@ func TestBuilder_Initiator(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(nil),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -174,6 +176,7 @@ func TestBuilder_Baggage(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(nil),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -216,6 +219,7 @@ func TestBuilder_Reason(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(nil),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -365,6 +369,7 @@ func TestWorkspaceBuildWithTags(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(richParameters),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, templateVersionVariables),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -464,6 +469,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(richParameters),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(initialBuildParameters),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -515,6 +521,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(richParameters),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(initialBuildParameters),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -661,6 +668,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
|
||||
withTemplate,
|
||||
withActiveVersion(version2params),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(activeVersionID, nil),
|
||||
withRichParameters(initialBuildParameters),
|
||||
withParameterSchemas(activeJobID, nil),
|
||||
@@ -727,6 +735,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
|
||||
withTemplate,
|
||||
withActiveVersion(version2params),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(activeVersionID, nil),
|
||||
withRichParameters(initialBuildParameters),
|
||||
withParameterSchemas(activeJobID, nil),
|
||||
@@ -791,6 +800,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
|
||||
withTemplate,
|
||||
withActiveVersion(version2params),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(activeVersionID, nil),
|
||||
withRichParameters(initialBuildParameters),
|
||||
withParameterSchemas(activeJobID, nil),
|
||||
@@ -1062,6 +1072,7 @@ func TestWorkspaceBuildUsageChecker(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(nil),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -1175,6 +1186,7 @@ func TestWorkspaceBuildWithTask(t *testing.T) {
|
||||
withTemplate,
|
||||
withInactiveVersion(nil),
|
||||
withLastBuildFound,
|
||||
withLastBuildState,
|
||||
withTemplateVersionVariables(inactiveVersionID, nil),
|
||||
withRichParameters(nil),
|
||||
withParameterSchemas(inactiveJobID, nil),
|
||||
@@ -1378,7 +1390,6 @@ func withLastBuildFound(mTx *dbmock.MockStore) {
|
||||
Transition: database.WorkspaceTransitionStart,
|
||||
InitiatorID: userID,
|
||||
JobID: lastBuildJobID,
|
||||
ProvisionerState: []byte("last build state"),
|
||||
Reason: database.BuildReasonInitiator,
|
||||
}, nil)
|
||||
|
||||
@@ -1398,6 +1409,14 @@ func withLastBuildFound(mTx *dbmock.MockStore) {
|
||||
}, nil)
|
||||
}
|
||||
|
||||
func withLastBuildState(mTx *dbmock.MockStore) {
|
||||
mTx.EXPECT().GetWorkspaceBuildProvisionerStateByID(gomock.Any(), lastBuildID).
|
||||
Times(1).
|
||||
Return(database.GetWorkspaceBuildProvisionerStateByIDRow{
|
||||
ProvisionerState: []byte("last build state"),
|
||||
}, nil)
|
||||
}
|
||||
|
||||
func withLastBuildNotFound(mTx *dbmock.MockStore) {
|
||||
mTx.EXPECT().GetLatestWorkspaceBuildByWorkspaceID(gomock.Any(), workspaceID).
|
||||
Times(1).
|
||||
|
||||
Reference in New Issue
Block a user