Purge tombstoned metastore rows from the control plane - #486
Draft
em3s wants to merge 6 commits into
Draft
Conversation
Empty commit opening the PR for #485. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The purge opens a database directly, so an allowlist decides which ones. A target absent from `actionbase.control.metastores` is refused rather than resolved, or the endpoint would connect wherever a request body pointed it. Credentials stay in the registry and never reach a `MetastoreTarget`, which travels in request bodies and logs. `MetastoreTarget` lands in engine rather than server, and shares no code with `Graph`, `JdbcHashLabel` or the metastore's serving path. Deleting rows out from under the running metastore is not an ability the serving path should grow, and a tool that dies with JDBC should not be entangled with what is being retired around it. It also means no new dependency: the purge uses plain JDBC, so server needs neither Exposed nor a connection pool. The table name is interpolated into every statement the purge runs, so `MetastoreTarget` checks it is a plain identifier at construction. Step 1 of #485 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`scanStorage` cannot filter on `active` in SQL, so a scan spends its limit on tombstones before anything looks at them. The purge walks `WHERE id > :cursor ORDER BY id` instead: indexed, resumable, and bounded by a row cap rather than by how much rubbish sits under one prefix. It reads two fields and no model. `active` is the front of the encoded value, and the service is the front of the encoded key, so both come off `OrderedBytes` and `ValueUtils` directly - no `HashEdgeValue`, no `DecodedEdge`, no `EdgeSchema`, nothing that would tie a temporary tool to the metastore's serving path. `PurgeTable` is its own Exposed definition rather than `MetadataTable` for the same reason. How old a tombstone is comes from `update_ts` rather than the encoded `delete_ts`. Reaching that property means walking the whole property list, while the column says the same thing for a row that is currently inactive - the last write was the one that deactivated it - and says it in a `WHERE` clause against an index. Two limits, because there are two costs: `maxRows` caps what comes back and `maxScan` caps how far the walk goes when tombstones are sparse. The cursor stops on the row that fills the page rather than draining it, or resuming would skip the rows that were fetched and never returned. A row that cannot be read is reported and never deleted. Step 2 of #485 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`candidates` hands back what it would delete, contents and all. `execute` deletes exactly those rows. `restore` puts them back. All three exchange one document, so there is no reduced projection to keep in step and the CLI never reshapes anything - it saves a response and later posts it as it stands. Stateless is what makes the saved file a backup. The server remembers nothing between calls, so the rows have to reach the client before anything is deleted, and there is no window where rows are gone and the only copy was lost in a dropped response. Committing a purge is keeping the file; reverting one is handing the same file to `restore`. `execute` deletes on `k` and `v` together, so a tombstone recreated since it was listed holds different bytes and comes back as `CHANGED` rather than being destroyed, and a row a previous call already removed is `ABSENT` - repeating a request that was answered but never seen changes nothing. `restore` writes only where the key is free, because an old tombstone must not land on metadata created since the purge. The document carries resolved coordinates rather than the name that was asked for, and both are checked against the configured list before anything opens, so a file taken from one metastore cannot be applied to another. `candidates` is a read and a read-only instance answers it; the other two are writes and join `CONTROL_WRITE_ENDPOINTS`. Steps 3 through 7 of #485 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…toString Review findings on this branch. `maxRows` and `maxScan` exist to bound a response and a walk, and a caller could set either to something enormous and undo that - a page is held whole in memory before it is serialised. `/control` has no authorization yet, so the ceiling belongs on the server. `PurgeQuery.bounded()` clamps both, and the age and cursor alongside them. `MetastoreProperties.Metastore` is a data class holding a password, so its generated `toString` would put the password into any log line or exception message that touched it. Actuator masks by key name; nothing masks a data class. It now prints the password as `****`. Also here: `execute` and `restore` check which metastore a document names before whether it has rows, so a file aimed at a database this instance does not serve says that rather than reporting an empty one; `restore` inserts as one batch, which needs `id` declared as the generated column it actually is; and the reason `delete` stays one statement per row is written down, since deleting exactly the bytes that were shown is the property that rules out reading first and deleting by key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two fixes from the last round went in without tests. Both are the kind that regress quietly: a clamp that stops clamping still answers, and a password reaches a log only once something prints the object. `PurgeQueryTest` pins the ceilings and the floors, including that `maxScan` is raised to the clamped `maxRows` rather than the requested one. `MetastorePropertiesTest` asserts the password is absent from both the metastore and the properties object that encloses it, since the enclosing one is what gets logged. The comment on `resolve` said which metastore is checked before whether there is anything to do, but that second check no longer exists - the emptiness guard was dropped, not reordered, and an empty document now applies nothing and reports `requested=0`. The one-line `resolve` is inlined into its only caller and the comment there says what actually happens, with two E2E cases holding it: an empty document is a no-op, and an empty document still has to name a metastore this instance serves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
em3s
marked this pull request as draft
August 11, 2026 00:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The metastore keeps every row it was ever told to delete. A metadata
DELETEwrites the row back withActive.INACTIVE, andJdbcHashLabel.scanStorageappliesLIMITbefore the active filter, so tombstones spend the budget a live scan needs. Past 1000 of them under one service,DatastoreTableReferences.whole()refuses and the control plane's own cleanup endpoints stop answering.This adds a purge that reads a metastore table directly, hands back what it would delete, deletes exactly that, and can put it back. Stateless:
candidatesreturns aPurgeSet, andexecuteandrestoreaccept the same document unchanged, so the file an operator holds is both the plan and the backup.Temporary. It targets a table the data plane is already abandoning and goes away with JDBC in Phase 4 of #347.
Closes #485
Plan
Created by claude code (opus 5)
actionbase.control.metastores.*config — the JDBC URI and table pairs the purge may open, credentials from the environment, resolved at startupactiveanddeleteTswith an empty field-name map, filter by service, report undecodable keysPurgeSetas one type, serving as thecandidatesresponse and the request body of the other twoPurgeSetwhose target is not in the configured listReadOnlyRequestFilter—executeandrestoreare writespurge plan/show/apply/restorein the Go CLIcandidatesresponse back verbatim — resurrected tombstone, undecodable row, repeatedexecute, restore over a live keyProgress
resolvecomment corrected — claude code (opus 5) (commit 0fcd65f)Changes
engine —
metastore/purge/, standalone.MetastorePurgewalksWHERE id > :cursor ORDER BY id, decodes two fields, and deletes onkandvtogether.PurgeFieldsreads the active flag off the front of the encoded value and the service off the front of the encoded key.PurgeTableis its own Exposed definition.server —
control/metastore/holds the allowlist, the credentials and the service;api/control/metastore/holds the three endpoints.candidatesjoins the read-only filter's allowed suffixes,executeandrestorejoinCONTROL_WRITE_ENDPOINTS.cli —
purge plan | show | apply | restore, driving the endpoints over a file.applyandrestoreneed--yes.Three things worth naming
Nothing from the data model. The purge reads
activeand the service straight off the encodings withOrderedBytesandValueUtils. NoHashEdgeValue, noDecodedEdge, noEdgeSchema, noGraph. A tool that dies with JDBC should not be entangled with what is being retired around it, and it means no new dependency inserverat all.How old a tombstone is comes from
update_ts, not the encodeddelete_ts. Reaching that property means walking the whole property list, while the column says the same thing for a row that is currently inactive — the last write was the one that deactivated it — and says it in aWHEREclause against an index.One document, three endpoints.
candidatesreturns it and the other two take it back unchanged, so the CLI never reshapes anything. The rows therefore reach the client before anything is deleted, which is what makes the saved file a backup rather than a convenience. Committing a purge is keeping the file; reverting one is posting it torestore.How to Test
ControlPurgeE2ETestruns against a real table and posts thecandidatesresponse back byte for byte, which is the claim that has to keep holding.Not touched
JdbcHashLabel.cadis broken — itsdeleteWherelambda hask eq ...andv eq ...as separate statements, so Exposed takes the last as the predicate and the delete matches on value alone, ignoring the key.deleteOnLockis the caller. Out of scope here and it needs its own issue.The root cause outlives this PR:
scanStoragestill cannot filter onactivein SQL. Anactivecolumn with an index would fix it properly, at the cost of a schema change and a backfill.AI Assistance