test(datastore): pin the ordered eviction walks against a bitmap plan
What this fixes
Three test:integration jobs on main failed in the last day on
internal/datastore.TestNpmRemoteVersionStore_ListIDsForDeleteAll_UsesTheKeysetIndexBackward:
job 16059475513 on PostgreSQL 16, and jobs 16058381758 and 16051770827 on
PostgreSQL 18. Each reported the same plan:
Limit
-> Sort
Sort Key: npm_remote_versions.created_at DESC, npm_remote_versions.id DESC
-> Bitmap Heap Scan on npm_remote_versions_p16
Filter: (... ROW(created_at, id) < ROW(...))
-> Bitmap Index Scan on npm_remote_versions_p16_pkeyAll three of the test's assertions rest on the ordering coming from the index:
Scan Backward present, no Sort Key, and the row-value bound folded into an
Index Cond. explainListPlan pins enable_seqscan alone, so a bitmap heap
scan under a Sort stays available — and on a three-row fixture its cost sits
close enough to the ordered index scan's that the choice turns on whatever else
the shared database happened to hold when ANALYZE ran. The assertion was
deciding on statistics the fixture does not control, which is also why two
PostgreSQL versions produced it and the third did not.
What changed
explainKeysetPlanpinsenable_seqscanandenable_bitmapscan, removing the alternative rather than out-costing it.explainListPlankeeps its single setting and its 36 other callers, so no caller that rests on the cost estimate to rule a bitmap plan out loses that signal. Both delegate to a sharedexplainOnPinnedConn.- The three ordered npm remote eviction walks move onto the new helper. The
versions walk is the one that failed; the packages and files walks carry the
same
Sort Keyassertion on fixtures of three rows, so they carry the same exposure and had not yet fired.
The *testing.T parameters on the three helpers are renamed tt to t,
which is internal to each function and changes no call site.
docs/dev/go-testing.md reserves tt for a subtest's inner parameter; these
are shared helpers, so thelper's naming check applies and now passes.
The trade this makes
internal/datastore/migrations/schema_helpers_test.go already made the same
choice for its helper of this name and records the cost: with no bitmap plan
left to produce, a regression whose symptom would have been a bitmap plan comes
back as an index scan, and only the caller's own Sort and Index Cond
assertions separate the two. All three walks moved here carry both, which is
why they are the ones that move.
Verification
- The three walks pass against a PostgreSQL matching the CI service's
max_locks_per_transaction=1024. - The full
internal/datastoreintegration suite runs with one failure,TestListNpmVersionIDsForDeleteAll_HostedKeepsTheAcceptanceBound, which reproduces identically on a cleanorigin/mainand is unrelated to this change. golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/datastore/reports nothing on either changed region.
Scope
Test-only. No production file is touched, and the change is confined to which planner settings one EXPLAIN helper pins.
The two remaining main failures in the same sweep are a different root cause
(the counter pipeline's dirty set is one globally-named key, so concurrently
running suites on the shared CI Redis claim each other's scopes) and are
addressed separately.
Related issue: #817 (closed) Related issue: #818 (closed)