From 571b78fc3ec3dfa76ac3cf4dac01ead1af497909 Mon Sep 17 00:00:00 2001 From: Jakub Novak Date: Thu, 1 Oct 2026 14:11:28 +0000 Subject: [PATCH] perf(db): pick the snapshot list page before reading wide columns The snapshot cursor queries sorted every matching snapshot of a team with all of its columns, metadata and config included, before LIMIT could apply: a LIMIT cannot bound a sort through the lateral joins above it. For large teams the planner's parallel bitmap plan spilled that sort to disk on every call. Each query now picks the page in a subquery that returns only the id, the sort keys and the build columns, then reads the full row and the aliases for the page alone. The ready-build lookup stays in the subquery because it drops rows and must run before LIMIT. Generated row types and parameters are unchanged. --- .../snapshot_latest_assignment_test.go | 49 +++ .../db/queries/get_snapshots_with_cursor.sql | 337 +++++++++++------- .../queries/get_snapshots_with_cursor.sql.go | 337 +++++++++++------- .../queries/snapshot_cursor_queries_test.go | 62 ++-- 4 files changed, 488 insertions(+), 297 deletions(-) diff --git a/packages/db/pkg/tests/snapshots/snapshot_latest_assignment_test.go b/packages/db/pkg/tests/snapshots/snapshot_latest_assignment_test.go index 2a6d0e4251..f59ba64f55 100644 --- a/packages/db/pkg/tests/snapshots/snapshot_latest_assignment_test.go +++ b/packages/db/pkg/tests/snapshots/snapshot_latest_assignment_test.go @@ -300,6 +300,55 @@ func TestGetSnapshotsWithCursorAsc_OrdersOldestFirstAndPaginates(t *testing.T) { assert.Equal(t, oldestToNewest[1:], activeIDs) } +// TestGetSnapshotsWithCursor_OrdersNewestFirstAndPaginates verifies the descending keyset +// query returns snapshots newest-first and walks the pages without gaps or overlaps, and +// that a snapshot without a ready build is dropped before LIMIT rather than taking a slot. +func TestGetSnapshotsWithCursor_OrdersNewestFirstAndPaginates(t *testing.T) { + t.Parallel() + db := testutils.SetupDatabase(t) + ctx := t.Context() + + teamID := testutils.CreateTestTeam(t, db) + baseTemplateID := testutils.CreateTestTemplate(t, db, teamID) + + oldestToNewest := make([]string, 0, 3) + for range 3 { + sandboxID := "sandbox-" + uuid.New().String() + testutils.UpsertTestSnapshot(t, ctx, db, "snapshot-template-"+uuid.New().String(), sandboxID, teamID, baseTemplateID) + oldestToNewest = append(oldestToNewest, sandboxID) + time.Sleep(10 * time.Millisecond) + } + + // Newest of all, but still snapshotting: it has no ready build. + testutils.UpsertTestSnapshotWithStatus(t, ctx, db, "snapshot-template-"+uuid.New().String(), + "sandbox-"+uuid.New().String(), teamID, baseTemplateID, types.BuildStatusSnapshotting) + + page1, err := db.SqlcClient.GetSnapshotsWithCursor(ctx, queries.GetSnapshotsWithCursorParams{ + TeamID: teamID, + Metadata: types.JSONBStringMap{}, + CursorTime: pgtype.Timestamptz{Time: time.Now().Add(time.Hour), Valid: true}, + Limit: 2, + }) + require.NoError(t, err) + require.Len(t, page1, 2, "a snapshot without a ready build must not take a page slot") + assert.Equal(t, []string{oldestToNewest[2], oldestToNewest[1]}, []string{ + page1[0].Snapshot.SandboxID, + page1[1].Snapshot.SandboxID, + }, "descending query should return newest sandbox first") + + last := page1[1].Snapshot + page2, err := db.SqlcClient.GetSnapshotsWithCursor(ctx, queries.GetSnapshotsWithCursorParams{ + TeamID: teamID, + Metadata: types.JSONBStringMap{}, + CursorID: last.SandboxID, + CursorTime: last.SandboxStartedAt, + Limit: 2, + }) + require.NoError(t, err) + require.Len(t, page2, 1) + assert.Equal(t, oldestToNewest[0], page2[0].Snapshot.SandboxID) +} + func TestGetSnapshotsByTemplateWithCursor_FiltersTemplateAndStartedAfter(t *testing.T) { t.Parallel() db := testutils.SetupDatabase(t) diff --git a/packages/db/queries/get_snapshots_with_cursor.sql b/packages/db/queries/get_snapshots_with_cursor.sql index 1a677b6c7d..53296400a8 100644 --- a/packages/db/queries/get_snapshots_with_cursor.sql +++ b/packages/db/queries/get_snapshots_with_cursor.sql @@ -1,176 +1,237 @@ -- name: GetSnapshotsWithCursor :many +-- The page subquery leaves out the wide columns, so a sort over a team's snapshots never +-- carries their metadata and config; full rows and aliases are fetched for the page alone. +-- The build lookup stays inside because it drops snapshots without a ready build, which +-- must happen before LIMIT. The outer ORDER BY reads page columns so the planner can keep +-- the subquery's order instead of sorting full rows again. All four queries share this shape. SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - sqlc.embed(s), - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + sqlc.embed(snap), + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = @team_id + -- The order here is important, we want started_at descending, but sandbox_id ascending + -- Short-circuit empty filters only for objects; legacy values still use containment. + AND CASE + WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> @metadata + END + AND s.sandbox_started_at >= @started_after::timestamptz + AND (s.sandbox_started_at, @cursor_id::text) < (@cursor_time, s.sandbox_id) + ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = @team_id - -- The order here is important, we want started_at descending, but sandbox_id ascending - -- Short-circuit empty filters only for objects; legacy values still use containment. - AND CASE - WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> @metadata - END - AND s.sandbox_started_at >= @started_after::timestamptz - AND (s.sandbox_started_at, @cursor_id::text) < (@cursor_time, s.sandbox_id) -ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC -LIMIT $1; +ORDER BY page.sandbox_started_at DESC, page.sandbox_id ASC; -- name: GetSnapshotsWithCursorAsc :many -- Ascending counterpart of GetSnapshotsWithCursor. It is the exact reverse order -- (started_at ASC, sandbox_id DESC) which maps onto a backward scan of the -- idx_snapshots_team_time_id (team_id, sandbox_started_at DESC, sandbox_id) index. SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - sqlc.embed(s), - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + sqlc.embed(snap), + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = @team_id + AND CASE + WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> @metadata + END + AND s.sandbox_started_at >= @started_after::timestamptz + -- The lower bound supplies the timestamp constraint for the ID tie-breaker and + -- gives the planner an indexable starting point for the ascending scan. + AND s.sandbox_started_at >= @cursor_time + AND (s.sandbox_started_at > @cursor_time OR s.sandbox_id < @cursor_id::text) + ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = @team_id - AND CASE - WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> @metadata - END - AND s.sandbox_started_at >= @started_after::timestamptz - -- The lower bound supplies the timestamp constraint for the ID tie-breaker and - -- gives the planner an indexable starting point for the ascending scan. - AND s.sandbox_started_at >= @cursor_time - AND (s.sandbox_started_at > @cursor_time OR s.sandbox_id < @cursor_id::text) -ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC -LIMIT $1; +ORDER BY page.sandbox_started_at ASC, page.sandbox_id DESC; -- name: GetSnapshotsByTemplateWithCursor :many SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - sqlc.embed(s), - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + sqlc.embed(snap), + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = @team_id + AND s.base_env_id = @template_id + AND CASE + WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> @metadata + END + AND s.sandbox_started_at >= @started_after::timestamptz + AND (s.sandbox_started_at, @cursor_id::text) < (@cursor_time, s.sandbox_id) + ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = @team_id - AND s.base_env_id = @template_id - AND CASE - WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> @metadata - END - AND s.sandbox_started_at >= @started_after::timestamptz - AND (s.sandbox_started_at, @cursor_id::text) < (@cursor_time, s.sandbox_id) -ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC -LIMIT $1; +ORDER BY page.sandbox_started_at DESC, page.sandbox_id ASC; -- name: GetSnapshotsByTemplateWithCursorAsc :many SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - sqlc.embed(s), - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + sqlc.embed(snap), + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = @team_id + AND s.base_env_id = @template_id + AND CASE + WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> @metadata + END + AND s.sandbox_started_at >= @started_after::timestamptz + -- The lower bound supplies the timestamp constraint for the ID tie-breaker and + -- gives the planner an indexable starting point for the ascending scan. + AND s.sandbox_started_at >= @cursor_time + AND (s.sandbox_started_at > @cursor_time OR s.sandbox_id < @cursor_id::text) + ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = @team_id - AND s.base_env_id = @template_id - AND CASE - WHEN @metadata::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> @metadata - END - AND s.sandbox_started_at >= @started_after::timestamptz - -- The lower bound supplies the timestamp constraint for the ID tie-breaker and - -- gives the planner an indexable starting point for the ascending scan. - AND s.sandbox_started_at >= @cursor_time - AND (s.sandbox_started_at > @cursor_time OR s.sandbox_id < @cursor_id::text) -ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC -LIMIT $1; +ORDER BY page.sandbox_started_at ASC, page.sandbox_id DESC; diff --git a/packages/db/queries/get_snapshots_with_cursor.sql.go b/packages/db/queries/get_snapshots_with_cursor.sql.go index 679ebadea4..c63ed905a6 100644 --- a/packages/db/queries/get_snapshots_with_cursor.sql.go +++ b/packages/db/queries/get_snapshots_with_cursor.sql.go @@ -16,45 +16,59 @@ import ( const getSnapshotsByTemplateWithCursor = `-- name: GetSnapshotsByTemplateWithCursor :many SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - s.created_at, s.env_id, s.sandbox_id, s.id, s.metadata, s.base_env_id, s.sandbox_started_at, s.env_secure, s.origin_node_id, s.allow_internet_access, s.auto_pause, s.team_id, s.config, - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + snap.created_at, snap.env_id, snap.sandbox_id, snap.id, snap.metadata, snap.base_env_id, snap.sandbox_started_at, snap.env_secure, snap.origin_node_id, snap.allow_internet_access, snap.auto_pause, snap.team_id, snap.config, + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = $2 + AND s.base_env_id = $3 + AND CASE + WHEN $4::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> $4 + END + AND s.sandbox_started_at >= $5::timestamptz + AND (s.sandbox_started_at, $6::text) < ($7, s.sandbox_id) + ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = $2 - AND s.base_env_id = $3 - AND CASE - WHEN $4::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> $4 - END - AND s.sandbox_started_at >= $5::timestamptz - AND (s.sandbox_started_at, $6::text) < ($7, s.sandbox_id) -ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC -LIMIT $1 +ORDER BY page.sandbox_started_at DESC, page.sandbox_id ASC ` type GetSnapshotsByTemplateWithCursorParams struct { @@ -131,48 +145,62 @@ func (q *Queries) GetSnapshotsByTemplateWithCursor(ctx context.Context, arg GetS const getSnapshotsByTemplateWithCursorAsc = `-- name: GetSnapshotsByTemplateWithCursorAsc :many SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - s.created_at, s.env_id, s.sandbox_id, s.id, s.metadata, s.base_env_id, s.sandbox_started_at, s.env_secure, s.origin_node_id, s.allow_internet_access, s.auto_pause, s.team_id, s.config, - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + snap.created_at, snap.env_id, snap.sandbox_id, snap.id, snap.metadata, snap.base_env_id, snap.sandbox_started_at, snap.env_secure, snap.origin_node_id, snap.allow_internet_access, snap.auto_pause, snap.team_id, snap.config, + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = $2 + AND s.base_env_id = $3 + AND CASE + WHEN $4::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> $4 + END + AND s.sandbox_started_at >= $5::timestamptz + -- The lower bound supplies the timestamp constraint for the ID tie-breaker and + -- gives the planner an indexable starting point for the ascending scan. + AND s.sandbox_started_at >= $6 + AND (s.sandbox_started_at > $6 OR s.sandbox_id < $7::text) + ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = $2 - AND s.base_env_id = $3 - AND CASE - WHEN $4::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> $4 - END - AND s.sandbox_started_at >= $5::timestamptz - -- The lower bound supplies the timestamp constraint for the ID tie-breaker and - -- gives the planner an indexable starting point for the ascending scan. - AND s.sandbox_started_at >= $6 - AND (s.sandbox_started_at > $6 OR s.sandbox_id < $7::text) -ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC -LIMIT $1 +ORDER BY page.sandbox_started_at ASC, page.sandbox_id DESC ` type GetSnapshotsByTemplateWithCursorAscParams struct { @@ -249,46 +277,60 @@ func (q *Queries) GetSnapshotsByTemplateWithCursorAsc(ctx context.Context, arg G const getSnapshotsWithCursor = `-- name: GetSnapshotsWithCursor :many SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - s.created_at, s.env_id, s.sandbox_id, s.id, s.metadata, s.base_env_id, s.sandbox_started_at, s.env_secure, s.origin_node_id, s.allow_internet_access, s.auto_pause, s.team_id, s.config, - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + snap.created_at, snap.env_id, snap.sandbox_id, snap.id, snap.metadata, snap.base_env_id, snap.sandbox_started_at, snap.env_secure, snap.origin_node_id, snap.allow_internet_access, snap.auto_pause, snap.team_id, snap.config, + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = $2 + -- The order here is important, we want started_at descending, but sandbox_id ascending + -- Short-circuit empty filters only for objects; legacy values still use containment. + AND CASE + WHEN $3::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> $3 + END + AND s.sandbox_started_at >= $4::timestamptz + AND (s.sandbox_started_at, $5::text) < ($6, s.sandbox_id) + ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = $2 - -- The order here is important, we want started_at descending, but sandbox_id ascending - -- Short-circuit empty filters only for objects; legacy values still use containment. - AND CASE - WHEN $3::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> $3 - END - AND s.sandbox_started_at >= $4::timestamptz - AND (s.sandbox_started_at, $5::text) < ($6, s.sandbox_id) -ORDER BY s.sandbox_started_at DESC, s.sandbox_id ASC -LIMIT $1 +ORDER BY page.sandbox_started_at DESC, page.sandbox_id ASC ` type GetSnapshotsWithCursorParams struct { @@ -312,6 +354,11 @@ type GetSnapshotsWithCursorRow struct { BuildCreatedAt time.Time } +// The page subquery leaves out the wide columns, so a sort over a team's snapshots never +// carries their metadata and config; full rows and aliases are fetched for the page alone. +// The build lookup stays inside because it drops snapshots without a ready build, which +// must happen before LIMIT. The outer ORDER BY reads page columns so the planner can keep +// the subquery's order instead of sorting full rows again. All four queries share this shape. func (q *Queries) GetSnapshotsWithCursor(ctx context.Context, arg GetSnapshotsWithCursorParams) ([]GetSnapshotsWithCursorRow, error) { rows, err := q.db.Query(ctx, getSnapshotsWithCursor, arg.Limit, @@ -363,47 +410,61 @@ func (q *Queries) GetSnapshotsWithCursor(ctx context.Context, arg GetSnapshotsWi const getSnapshotsWithCursorAsc = `-- name: GetSnapshotsWithCursorAsc :many SELECT COALESCE(ea.aliases, ARRAY[]::text[])::text[] AS aliases, COALESCE(ea.names, ARRAY[]::text[])::text[] AS names, - s.created_at, s.env_id, s.sandbox_id, s.id, s.metadata, s.base_env_id, s.sandbox_started_at, s.env_secure, s.origin_node_id, s.allow_internet_access, s.auto_pause, s.team_id, s.config, - eb.id AS build_id, - eb.vcpu AS build_vcpu, - eb.ram_mb AS build_ram_mb, - eb.total_disk_size_mb AS build_total_disk_size_mb, - eb.envd_version AS build_envd_version, - eb.created_at AS build_created_at -FROM "public"."snapshots" s -JOIN "public"."active_envs" e ON e.id = s.env_id + snap.created_at, snap.env_id, snap.sandbox_id, snap.id, snap.metadata, snap.base_env_id, snap.sandbox_started_at, snap.env_secure, snap.origin_node_id, snap.allow_internet_access, snap.auto_pause, snap.team_id, snap.config, + page.build_id, + page.build_vcpu, + page.build_ram_mb, + page.build_total_disk_size_mb, + page.build_envd_version, + page.build_created_at +FROM ( + SELECT + s.id, + s.sandbox_started_at, + s.sandbox_id, + eb.id AS build_id, + eb.vcpu AS build_vcpu, + eb.ram_mb AS build_ram_mb, + eb.total_disk_size_mb AS build_total_disk_size_mb, + eb.envd_version AS build_envd_version, + eb.created_at AS build_created_at + FROM "public"."snapshots" s + JOIN "public"."active_envs" e ON e.id = s.env_id + JOIN LATERAL ( + SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at + FROM "public"."env_build_assignments" eba + JOIN "public"."env_builds" eb ON eb.id = eba.build_id + WHERE + eba.env_id = s.env_id + AND eba.tag = 'default' + AND eb.status_group = 'ready' + ORDER BY eba.created_at DESC + LIMIT 1 + ) eb ON TRUE + WHERE + s.team_id = $2 + AND CASE + WHEN $3::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' + THEN TRUE + ELSE s.metadata @> $3 + END + AND s.sandbox_started_at >= $4::timestamptz + -- The lower bound supplies the timestamp constraint for the ID tie-breaker and + -- gives the planner an indexable starting point for the ascending scan. + AND s.sandbox_started_at >= $5 + AND (s.sandbox_started_at > $5 OR s.sandbox_id < $6::text) + ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC + LIMIT $1 +) page +JOIN "public"."snapshots" snap ON snap.id = page.id LEFT JOIN LATERAL ( SELECT ARRAY_AGG(alias ORDER BY alias) AS aliases, ARRAY_AGG(CASE WHEN namespace IS NOT NULL THEN namespace || '/' || alias ELSE alias END ORDER BY alias) AS names FROM "public"."env_aliases" - WHERE env_id = s.base_env_id + WHERE env_id = snap.base_env_id ) ea ON TRUE -JOIN LATERAL ( - SELECT eb.id, eb.vcpu, eb.ram_mb, eb.total_disk_size_mb, eb.envd_version, eb.created_at - FROM "public"."env_build_assignments" eba - JOIN "public"."env_builds" eb ON eb.id = eba.build_id - WHERE - eba.env_id = s.env_id - AND eba.tag = 'default' - AND eb.status_group = 'ready' - ORDER BY eba.created_at DESC - LIMIT 1 -) eb ON TRUE -WHERE - s.team_id = $2 - AND CASE - WHEN $3::jsonb = '{}'::jsonb AND jsonb_typeof(s.metadata) = 'object' - THEN TRUE - ELSE s.metadata @> $3 - END - AND s.sandbox_started_at >= $4::timestamptz - -- The lower bound supplies the timestamp constraint for the ID tie-breaker and - -- gives the planner an indexable starting point for the ascending scan. - AND s.sandbox_started_at >= $5 - AND (s.sandbox_started_at > $5 OR s.sandbox_id < $6::text) -ORDER BY s.sandbox_started_at ASC, s.sandbox_id DESC -LIMIT $1 +ORDER BY page.sandbox_started_at ASC, page.sandbox_id DESC ` type GetSnapshotsWithCursorAscParams struct { diff --git a/packages/db/queries/snapshot_cursor_queries_test.go b/packages/db/queries/snapshot_cursor_queries_test.go index 147451a40a..1be32e1950 100644 --- a/packages/db/queries/snapshot_cursor_queries_test.go +++ b/packages/db/queries/snapshot_cursor_queries_test.go @@ -12,35 +12,55 @@ import ( // against silent divergence. // // sqlc has no include mechanism, so each of the ascending, descending, filtered and -// unfiltered variants carries its own copy of the projection and the two lateral -// subqueries; only the WHERE and ORDER BY clauses are meant to differ. The generated row -// structs are convertible in Go, so a change to the selected columns is a compile error -// at the call site -- but changing the build assignment's tag or status filter, or the -// alias aggregation, in one copy and not the others would silently make the two orders -// (or the filtered and unfiltered paths) return different rows for the same sandbox, -// with nothing failing to tell us. +// unfiltered variants carries its own copy of the projection, the page subquery's joins +// and the alias lookup; only the page's WHERE and ORDER BY clauses are meant to differ. +// The generated row structs are convertible in Go, so a change to the selected columns is +// a compile error at the call site -- but changing the build assignment's tag or status +// filter, or the alias aggregation, in one copy and not the others would silently make +// the two orders (or the filtered and unfiltered paths) return different rows for the +// same sandbox, with nothing failing to tell us. +// +// Each query also orders twice: the page subquery picks its rows in one order and the +// outer query must return them in that same order. // // If this test fails, the fix is to apply the edit to all four queries in // get_snapshots_with_cursor.sql and regenerate, not to relax the assertion. func TestSnapshotCursorQueriesShareOneProjection(t *testing.T) { t.Parallel() - // projection returns everything from SELECT up to the WHERE clause: the column - // list and the joins, which every variant must share. The `-- name:` header and - // everything from WHERE onwards are per-query by construction. - projection := func(t *testing.T, query string) string { + // split returns the text every variant must share -- the column list, the page + // subquery's joins, and the outer joins -- plus the page's ORDER BY and the outer + // ORDER BY. The `-- name:` header and the page's WHERE clause are per-query by + // construction. + split := func(t *testing.T, query string) (shared, pageOrder, outerOrder string) { t.Helper() start := strings.Index(query, "SELECT ") require.NotEqual(t, -1, start, "query should have a SELECT") - body, _, found := strings.Cut(query[start:], "\nWHERE\n") - require.True(t, found, "query should have a WHERE clause") + head, rest, found := strings.Cut(query[start:], "\n WHERE\n") + require.True(t, found, "page subquery should have a WHERE clause") + + _, rest, found = strings.Cut(rest, "\n ORDER BY ") + require.True(t, found, "page subquery should have an ORDER BY") + + pageOrder, rest, found = strings.Cut(rest, "\n LIMIT $1\n) page\n") + require.True(t, found, "page subquery should end with LIMIT $1") + + tail, outerOrder, found := strings.Cut(rest, "\nORDER BY ") + require.True(t, found, "query should order the page") - return body + return head + tail, pageOrder, strings.TrimSpace(outerOrder) } - reference := projection(t, getSnapshotsWithCursor) + queries := map[string]string{ + "GetSnapshotsWithCursor": getSnapshotsWithCursor, + "GetSnapshotsWithCursorAsc": getSnapshotsWithCursorAsc, + "GetSnapshotsByTemplateWithCursor": getSnapshotsByTemplateWithCursor, + "GetSnapshotsByTemplateWithCursorAsc": getSnapshotsByTemplateWithCursorAsc, + } + + reference, _, _ := split(t, getSnapshotsWithCursor) // Sanity-check that the shared body really is the part worth pinning, so this test // cannot pass by comparing two empty strings. @@ -48,12 +68,12 @@ func TestSnapshotCursorQueriesShareOneProjection(t *testing.T) { require.Contains(t, reference, "eb.status_group = 'ready'") require.Contains(t, reference, "ARRAY_AGG(alias ORDER BY alias)") - for name, query := range map[string]string{ - "GetSnapshotsWithCursorAsc": getSnapshotsWithCursorAsc, - "GetSnapshotsByTemplateWithCursor": getSnapshotsByTemplateWithCursor, - "GetSnapshotsByTemplateWithCursorAsc": getSnapshotsByTemplateWithCursorAsc, - } { - assert.Equal(t, reference, projection(t, query), + for name, query := range queries { + shared, pageOrder, outerOrder := split(t, query) + + assert.Equal(t, reference, shared, "%s must select and join exactly as GetSnapshotsWithCursor does", name) + assert.Equal(t, strings.ReplaceAll(pageOrder, "s.", "page."), outerOrder, + "%s must return the page in the order the page subquery picked it", name) } }