feat(npm): batch deprecate datastore queries (S11 Step 17, part 1/2)

Dzmitry (Dima) Meshcharakourequested to merge
dm/npm-hosted-step-17-datastore into main

Part 1 of 2 of the npm hosted plan — Step 17: deprecate handler.

🗂️ Stacked MRs

Split into 2 stacked MRs to keep each within the review size limit (~600 ideal reviewable LoC). Each part targets the previous one (part 1 → main); review and merge bottom-up.

📦 What this part adds

Adds the datastore query primitives the deprecate write composes, split ahead of the handler so the query layer reviews on its own (both in the new internal/datastore/npm_versions_deprecate.go):

  • NpmVersionsByPackageAndVersions — resolves a set of versions to their active npm_versions rows in one IN (...) query, batching the handler's per-version point lookups.
  • BatchUpdateNpmVersionPackageJSON — applies many package_json updates in one UPDATE ... FROM unnest(...) statement (raw SQL, per docs/dev/database-query-patterns.md batch-update guidance) and returns RowsAffected, so the caller can detect a version soft-deleted between resolve and write. Takes a batch-specific two-field NpmVersionPackageJSONUpdate item and rejects zero or duplicate NpmVersionIDs (errNpmVersionDuplicateID — the unnest join is not duplicate-safe) and sets larger than the max page size.

The transaction-scoped packument-cache force-expire the deprecate write also composes, ForceExpireNpmMetadataTx, is not in this part — the identical method merged to main with the dist-tag write layer during the restack, so this MR reuses it.

✅ Test coverage

Integration tests (internal/datastore/npm_deprecate_queries_integration_test.go) cover, against the real schema:

  • Batched resolve returns only the requested active rows — soft-deleted versions and other packages excluded.
  • Batched update reports rows affected; a soft-deleted target yields RowsAffected < len (the TOCTOU signal); every document is validated before the write.

DB-free argument-guard unit tests (internal/datastore/npm_versions_test.go) cover both methods' guards: nil ctx/db, zero namespace/package/version id, empty and oversized sets, and duplicate version ids (errNpmVersionDuplicateID).

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. The read query (NpmVersionsByPackageAndVersions) is seeded with 5000 rows in one namespace partition; the write query (BatchUpdateNpmVersionPackageJSON) follows the write-target sizing (1 matching row + 49 siblings), where the planner correctly prefers a Seq Scan over the single pruned partition at that cardinality. Numbers do not capture production-scale effects. See Database review evidence for methodology. Expand each row for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.NpmVersionsByPackageAndVersions Index Scan unique_npm_versions_ns_id_pkg_id_version 2 / 2 14.36 0.016ms 3 / 0 1
datastore.BatchUpdateNpmVersionPackageJSON Update → Hash Join → Seq Scan n/a (Seq Scan at 50-row seed) 1 / 1 1.98 1.389ms 20 / 2 1
datastore.NpmVersionsByPackageAndVersions

Summary: Plan matches intent — Index Scan over the (namespace_id, npm_package_id, version) unique index (partition-local npm_versions_p12_..._version_idx), with the namespace_id literal pruning to one of the 64 hash partitions. The version = ANY (...) membership test is served directly by the index; actual rows match the estimate (2 / 2) at 5000 seeded versions. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1, npm_versions=5000

Rendered SQL:

SELECT npm_versions.namespace_id AS "npm_versions.namespace_id",
     npm_versions.id AS "npm_versions.id",
     npm_versions.npm_package_id AS "npm_versions.npm_package_id",
     npm_versions.last_downloaded_at AS "npm_versions.last_downloaded_at",
     npm_versions.soft_deleted_at AS "npm_versions.soft_deleted_at",
     npm_versions.created_at AS "npm_versions.created_at",
     npm_versions.version AS "npm_versions.version",
     npm_versions.package_json AS "npm_versions.package_json",
     npm_versions.gitlab_user_id AS "npm_versions.gitlab_user_id",
     npm_versions.gitlab_project_id AS "npm_versions.gitlab_project_id",
     npm_versions.gitlab_git_commit_sha AS "npm_versions.gitlab_git_commit_sha"
FROM public.npm_versions
WHERE (((npm_versions.namespace_id = $1::uuid) AND (npm_versions.npm_package_id = $2::uuid)) AND (npm_versions.version IN ($3::text, $4::text))) AND (npm_versions.soft_deleted_at IS NULL);

Bound args: [<ns>, <pkg>, '1.0.0', '2.0.0']

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Index Scan using npm_versions_p12_namespace_id_npm_package_id_version_idx on npm_versions_p12 npm_versions  (cost=0.28..14.36 rows=2 width=183) (actual time=0.008..0.009 rows=2 loops=1)
   Index Cond: ((namespace_id = '<ns>'::uuid) AND (npm_package_id = '<pkg>'::uuid) AND (version = ANY ('{1.0.0,2.0.0}'::text[])))
   Buffers: shared hit=3
 Planning:
   Buffers: shared hit=428 read=1
 Planning Time: 0.662 ms
 Execution Time: 0.016 ms

Timings: planning 0.662ms, execution 0.016ms, total 0.678ms.

datastore.BatchUpdateNpmVersionPackageJSON

Summary: Plan matches intent — pruned to a single partition (npm_versions_p25); the unnest(...) input is hash-joined to the version rows on v.id = u.id::uuid, and the soft_deleted_at IS NULL + namespace_id filter discriminates (1 matched). The Seq Scan reflects the 50-row write-target seed; at production cardinality the v.id join under the namespace_id literal is served by pk_npm_versions (id, namespace_id). The RowsAffected count (1) is what drives the caller's TOCTOU check. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1, npm_versions=50

Rendered SQL:

UPDATE npm_versions AS v
SET package_json = u.doc::jsonb
FROM unnest($2::text[], $3::text[]) AS u(id, doc)
WHERE v.namespace_id = $1 AND v.id = u.id::uuid AND v.soft_deleted_at IS NULL;

Bound args: [<ns>, {<target npm_versions.id>}, {'{"deprecated":"x"}'}]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Update on npm_versions v  (cost=0.03..1.98 rows=0 width=0) (actual time=0.090..0.091 rows=0 loops=1)
   Update on npm_versions_p25 v_1
   Buffers: shared hit=20 read=2
   ->  Hash Join  (cost=0.03..1.98 rows=1 width=130) (actual time=0.019..0.024 rows=1 loops=1)
         Hash Cond: (v_1.id = (u.id)::uuid)
         Buffers: shared hit=1
         ->  Seq Scan on npm_versions_p25 v_1  (cost=0.00..1.62 rows=50 width=26) (actual time=0.003..0.005 rows=50 loops=1)
               Filter: ((soft_deleted_at IS NULL) AND (namespace_id = '<ns>'::uuid))
               Buffers: shared hit=1
         ->  Hash  (cost=0.01..0.01 rows=1 width=152) (actual time=0.009..0.009 rows=1 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 9kB
               ->  Function Scan on u  (cost=0.01..0.01 rows=1 width=152) (actual time=0.006..0.006 rows=1 loops=1)
 Planning:
   Buffers: shared hit=253 read=2
 Planning Time: 0.717 ms
 Trigger for constraint fk_npm_versions_namespace_id_namespaces on npm_versions_p25: time=0.017 calls=1
 Trigger for constraint fk_npm_versions_npm_package_id_npm_packages on npm_versions_p25: time=1.172 calls=1
 Execution Time: 1.389 ms

Timings: planning 0.717ms, execution 1.389ms, total 2.106ms.

📚 References

Related to #135 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports