opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

chore: replace original GetPrebuiltWorkspaces with optimized version (#18832)

Browse Source 
Fixes https://github.com/coder/internal/issues/715

Follow-up from https://github.com/coder/coder/pull/18717

Now that we've determined the updated query is safe, remove the duplication.
This commit is contained in:
Cian Johnston
2025-07-21 15:31:11 +01:00
committed by GitHub
parent af01562e35
commit 198d50dbc2
10 changed files with 7 additions and 313 deletions
Download Patch File Download Diff File Expand all files Collapse all files
enterprise/coderd/prebuilds/reconcile.go
-37
View File
@@ -12,7 +12,6 @@ import (
"sync/atomic"
"time"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/go-multierror"
"github.com/prometheus/client_golang/prometheus"
@@ -405,15 +404,6 @@ func (c *StoreReconciler) SnapshotState(ctx context.Context, store database.Stor
return xerrors.Errorf("failed to get running prebuilds: %w", err)
}
// Compare with optimized query to ensure behavioral correctness
optimized, err := db.GetRunningPrebuiltWorkspacesOptimized(ctx)
if err != nil {
// Log the error but continue with original results
c.logger.Error(ctx, "optimized GetRunningPrebuiltWorkspacesOptimized failed", slog.Error(err))
} else {
CompareGetRunningPrebuiltWorkspacesResults(ctx, c.logger, allRunningPrebuilds, optimized)
}
allPrebuildsInProgress, err := db.CountInProgressPrebuilds(ctx)
if err != nil {
return xerrors.Errorf("failed to get prebuilds in progress: %w", err)
@@ -933,30 +923,3 @@ func SetPrebuildsReconciliationPaused(ctx context.Context, db database.Store, pa
}
return db.UpsertPrebuildsSettings(ctx, string(settingsJSON))
}
// CompareGetRunningPrebuiltWorkspacesResults compares the original and optimized
// query results and logs any differences found. This function can be easily
// removed once we're confident the optimized query works correctly.
// TODO(Cian): Remove this function once the optimized query is stable and correct.
func CompareGetRunningPrebuiltWorkspacesResults(
ctx context.Context,
logger slog.Logger,
original []database.GetRunningPrebuiltWorkspacesRow,
optimized []database.GetRunningPrebuiltWorkspacesOptimizedRow,
) {
if len(original) == 0 && len(optimized) == 0 {
return
}
// Convert optimized results to the same type as original for comparison
optimizedConverted := make([]database.GetRunningPrebuiltWorkspacesRow, len(optimized))
for i, row := range optimized {
optimizedConverted[i] = database.GetRunningPrebuiltWorkspacesRow(row)
}
// Compare the results and log an error if they differ.
// NOTE: explicitly not sorting here as both query results are ordered by ID.
if diff := cmp.Diff(original, optimizedConverted); diff != "" {
logger.Error(ctx, "results differ for GetRunningPrebuiltWorkspacesOptimized",
slog.F("diff", diff))
}
}
enterprise/coderd/prebuilds/reconcile_test.go
-163
View File
@@ -5,7 +5,6 @@ import (
"database/sql"
"fmt"
"sort"
"strings"
"sync"
"testing"
"time"
@@ -27,7 +26,6 @@ import (
"tailscale.com/types/ptr"
"cdr.dev/slog"
"cdr.dev/slog/sloggers/slogjson"
"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/quartz"
@@ -2333,164 +2331,3 @@ func TestReconciliationRespectsPauseSetting(t *testing.T) {
require.NoError(t, err)
require.Len(t, workspaces, 2, "should have recreated 2 prebuilds after resuming")
}
func TestCompareGetRunningPrebuiltWorkspacesResults(t *testing.T) {
t.Parallel()
ctx := context.Background()
// Helper to create test data
createWorkspaceRow := func(id string, name string, ready bool) database.GetRunningPrebuiltWorkspacesRow {
uid := uuid.MustParse(id)
return database.GetRunningPrebuiltWorkspacesRow{
ID: uid,
Name: name,
TemplateID: uuid.New(),
TemplateVersionID: uuid.New(),
CurrentPresetID: uuid.NullUUID{UUID: uuid.New(), Valid: true},
Ready: ready,
CreatedAt: time.Now(),
}
}
createOptimizedRow := func(row database.GetRunningPrebuiltWorkspacesRow) database.GetRunningPrebuiltWorkspacesOptimizedRow {
return database.GetRunningPrebuiltWorkspacesOptimizedRow(row)
}
t.Run("identical results - no logging", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
original := []database.GetRunningPrebuiltWorkspacesRow{
createWorkspaceRow("550e8400-e29b-41d4-a716-446655440000", "workspace1", true),
createWorkspaceRow("550e8400-e29b-41d4-a716-446655440001", "workspace2", false),
}
optimized := []database.GetRunningPrebuiltWorkspacesOptimizedRow{
createOptimizedRow(original[0]),
createOptimizedRow(original[1]),
}
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, original, optimized)
// Should not log any errors when results are identical
require.Empty(t, strings.TrimSpace(sb.String()))
})
t.Run("count mismatch - logs error", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
original := []database.GetRunningPrebuiltWorkspacesRow{
createWorkspaceRow("550e8400-e29b-41d4-a716-446655440000", "workspace1", true),
}
optimized := []database.GetRunningPrebuiltWorkspacesOptimizedRow{
createOptimizedRow(original[0]),
createOptimizedRow(createWorkspaceRow("550e8400-e29b-41d4-a716-446655440001", "workspace2", false)),
}
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, original, optimized)
// Should log exactly one error.
if lines := strings.Split(strings.TrimSpace(sb.String()), "\n"); assert.NotEmpty(t, lines) {
require.Len(t, lines, 1)
assert.Contains(t, lines[0], "ERROR")
assert.Contains(t, lines[0], "workspace2")
assert.Contains(t, lines[0], "CurrentPresetID")
}
})
t.Run("count mismatch - other direction", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
original := []database.GetRunningPrebuiltWorkspacesRow{}
optimized := []database.GetRunningPrebuiltWorkspacesOptimizedRow{
createOptimizedRow(createWorkspaceRow("550e8400-e29b-41d4-a716-446655440001", "workspace2", false)),
}
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, original, optimized)
if lines := strings.Split(strings.TrimSpace(sb.String()), "\n"); assert.NotEmpty(t, lines) {
require.Len(t, lines, 1)
assert.Contains(t, lines[0], "ERROR")
assert.Contains(t, lines[0], "workspace2")
assert.Contains(t, lines[0], "CurrentPresetID")
}
})
t.Run("field differences - logs errors", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
workspace1 := createWorkspaceRow("550e8400-e29b-41d4-a716-446655440000", "workspace1", true)
workspace2 := createWorkspaceRow("550e8400-e29b-41d4-a716-446655440001", "workspace2", false)
original := []database.GetRunningPrebuiltWorkspacesRow{workspace1, workspace2}
// Create optimized with different values
optimized1 := createOptimizedRow(workspace1)
optimized1.Name = "different-name" // Different name
optimized1.Ready = false // Different ready status
optimized2 := createOptimizedRow(workspace2)
optimized2.CurrentPresetID = uuid.NullUUID{Valid: false} // Different preset ID (NULL)
optimized := []database.GetRunningPrebuiltWorkspacesOptimizedRow{optimized1, optimized2}
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, original, optimized)
// Should log exactly one error with a cmp.Diff output
if lines := strings.Split(strings.TrimSpace(sb.String()), "\n"); assert.NotEmpty(t, lines) {
require.Len(t, lines, 1)
assert.Contains(t, lines[0], "ERROR")
assert.Contains(t, lines[0], "different-name")
assert.Contains(t, lines[0], "workspace1")
assert.Contains(t, lines[0], "Ready")
assert.Contains(t, lines[0], "CurrentPresetID")
}
})
t.Run("empty results - no logging", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
original := []database.GetRunningPrebuiltWorkspacesRow{}
optimized := []database.GetRunningPrebuiltWorkspacesOptimizedRow{}
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, original, optimized)
// Should not log any errors when both results are empty
require.Empty(t, strings.TrimSpace(sb.String()))
})
t.Run("nil original", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, nil, []database.GetRunningPrebuiltWorkspacesOptimizedRow{})
// Should not log any errors when original is nil
require.Empty(t, strings.TrimSpace(sb.String()))
})
t.Run("nil optimized ", func(t *testing.T) {
t.Parallel()
var sb strings.Builder
logger := slog.Make(slogjson.Sink(&sb))
prebuilds.CompareGetRunningPrebuiltWorkspacesResults(ctx, logger, []database.GetRunningPrebuiltWorkspacesRow{}, nil)
// Should not log any errors when optimized is nil
require.Empty(t, strings.TrimSpace(sb.String()))
})
}