## Summary `MavenPackageReaper.Reap` deletes the package's `maven_versions` rows first and the `maven_packages` row last. It takes no lock of its own on the `maven_packages` row. That is the reverse of the parent-before-child order the Maven family records for its writers. `internal/datastore/maven_file_deleter.go:153-157` states the rule and names the price of an inversion: > ... this transaction's order is advisory lock, version row, file row — the parent-before-child order every other Maven writer takes (`ReVerifyVersionAlive` documents it). A file DELETE before the row lock inverts that order against a concurrent same-coordinate upload, whose `commitFileRow` holds the version row and then waits on this transaction's file row; PostgreSQL aborts one side as `40P01`. The reap's order is on `main` today, and until now it completed no cycle. No Maven writer held the `maven_packages` row while it waited on a `maven_versions` row. While [merge request 2331](https://gitlab.com/gitlab-org/ops/artifact-registry/-/merge_requests/2331) is open, the Maven version insert binds `maven_package_id` as a literal. It takes no `maven_packages` lock, so the cycle has no second side. Once that merge request merges, `MavenVersionStore.FindOrCreateVersion` draws `maven_package_id` from a `SELECT ... FOR SHARE` over the live `maven_packages` row (`internal/datastore/maven_versions.go:120-128`). That `SELECT` is the `INSERT`'s own source query. The statement then holds the share lock while the `ON CONFLICT` arbiter probe waits on a `maven_versions` tuple that an uncommitted purge chunk deleted. The two orders then complete a cycle. **This item does not say that merge request inverts a lock order.** The child-first order in `MavenPackageReaper.Reap` is older than it. What the package row lock adds is the counterparty that makes the existing order reachable. ## The route that reaches it One purge route reaches the cycle. The package-scope purge does not. It only reaps a package that carries a tombstone. Against a tombstoned package the `soft_deleted_at IS NULL` predicate matches no row, so the publish takes no lock at all. The repository purge walk does. `mavenPackagesReapPageStmt` (`internal/datastore/lifecycle_reap_repository.go:730-751`) reads no `soft_deleted_at` column. It hands `MavenPackageReaper.Reap` **live** `maven_packages` rows, and a publish against a live package takes the share lock. ## The participants The second row describes the state after that merge request merges. | Side | Order it holds | What a loss costs | | --- | --- | --- | | Maven hosted package reap | `maven_versions` rows first (`internal/datastore/lifecycle_reap_maven.go:401`), then the `maven_packages` row (`:328`) | SQLSTATE `40P01`. The chunk rolls back whole and the at-least-once re-queue redoes it | | the fenced Maven version insert | the `maven_packages` row `FOR SHARE`, held across the `ON CONFLICT` arbiter probe | HTTP 500 on the publish request | Neither side loses data. The publish answers 500 on a request that `ReVerifyRepositoryAlive` (`internal/format/maven/upload.go:690`) refuses later in the same upload. ## What is established, and what is not Established by reading the code at `e1279c160`: - The two acquisition orders in the table above. - The repository route hands live package rows to the reap, and the package route cannot reach the share lock. - `MavenPackageReaper.Reap` takes no `maven_packages` row lock on any path. Established by a review of that merge request, against PostgreSQL 17: - The deadlock reproduced twice, with two controls. The merge-base `VALUES` form does not deadlock on the same interleaving, and the fenced form against a tombstoned package takes no lock. **Not established: how often the window opens in production.** `RepositoryReaper.Reap` returns as soon as a level page comes back non-empty (`internal/datastore/lifecycle_reap_repository.go:321-338`), and `mavenHostedReapWalk` puts `maven_versions` before `maven_packages` (`:149-160`). So a chunk reaches `MavenPackageReaper.Reap` only when the repository held no `maven_versions` row at the version page's own statement. The version row the reap then deletes must commit inside that same chunk, in a window of about three statements at `READ COMMITTED`. The racing publish is more ordinary. `mvn deploy` sends several primary `PUT`s per version, and each one calls `FindOrCreateVersion` for the same coordinate. ## Two knock-on effects **The outcome label.** `40P01` is not a foreign-key violation, so it goes past `mapReapParentDeleteError` (`internal/datastore/lifecycle_scan.go:168-175`). `purgeOutcomeLabelValue` (`internal/lifecycle/metrics.go:470-483`) then books it as `purge_outcomes_total{purge_outcome="error"}`, and not as the named `parent_pinned` arm. That catch-all arm already carries other routine ends, which `docs/dev/storage-accounting.md` records. **Test coverage of the repository route.** While that merge request is open, its two new race tests drive the package tier only. They cover the mark against a publish, and a sibling publish against a publish. Neither drives the repository route. The two fixtures such a test needs, `startBlocked` and `pinnedPoolConn`, are already in the file that merge request adds. ## A candidate remedy This item states the candidates as options, not as a decision. `MavenPackageReaper.Reap` can take `FOR UPDATE` on the `maven_packages` row as the chunk's first row lock, before `reapVersionPage`. `NpmPackageReaper.Reap` already does this for its own package row (`internal/datastore/lifecycle_reap_npm.go:290-292`). The publish then waits on that exclusive lock, wakes to find the row gone, and gets `ErrNotFound` from the read-back's own `EXISTS`. The blocker recorded for the npm case does not apply here. [Work item 724](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/724) records why the npm reap could not take the parent row first: a counter decrement had to follow its own `DELETE`. `MavenPackageReaper` decrements no counter on the package row. A parent-first lock also opens no second cycle against the repository finalizer. `RepositoryReaper.Reap` returns before its finalize step whenever a level page is non-empty. So no chunk holds a `maven_packages` lock and then deletes the `repositories` row. The other candidate is to accept the cycle and record it. `docs/dev/storage-accounting.md` already accepts a reap-versus-request-path inversion of this class for the npm remote family. Its stated ground is that PostgreSQL picks which side pays, and the at-least-once re-queue redoes the chunk. ## Why this is not 724, 1068, or 1044 [Work item 724](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/724) is the same shape on the hosted npm family: `npm_packages` taken last where every npm write handler takes it first. Its tables are `npm_packages`, `npm_files`, `npm_tags` and `npm_versions`, and no clause of it reaches `maven_packages`. It is the precedent for how this project writes up an inversion of this shape, not a duplicate of it. [Work item 1068](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1068) is the same shape again on the npm remote family, over remote cache tables. [Work item 1044](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1044) covers a different question on the same two Maven tables: whether a `maven_versions` row can land under a package tombstone. That is about which rows a publish leaves behind. This item is about the order two transactions take their locks in. ## Where it is recorded today Nowhere in the tree, as of `e1279c160`. `docs/dev/storage-accounting.md` states one ending for the repository route, the SQLSTATE `23503` the referential check produces. It does not record that a package row lock on the publish side gives that route a second ending. This item is the only place that gap is written down. ## Done when One of these holds, and the Maven reap's own comments and `docs/dev/storage-accounting.md` agree with whichever it is: - `MavenPackageReaper.Reap` takes the `maven_packages` row first, and still reports its totals off each leg's own outcome, or - the cycle is accepted and recorded, with both endings of the repository route named and the `purge_outcome` arm named too, or - the route is shown to be unreachable, and the reason is written where a reader of the reap finds it. An integration test demonstrates the first. It runs a repository purge chunk against a same-coordinate Maven publish, and passes with no `40P01` on either side. ## Not addressed on the merge request that makes it reachable That merge request adds the package row lock and does not change the reap. This item records the remaining work. Related to [work item 1135](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1135) and [work item 611](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/611). _This is a bot message 🤖 — /smurfit_