fix: preserve reader grants across ash.rebuild_partitions() (#213) - #218
Draft
NikolayS wants to merge 1 commit into
Draft
fix: preserve reader grants across ash.rebuild_partitions() (#213)#218NikolayS wants to merge 1 commit into
NikolayS wants to merge 1 commit into
Conversation
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.
Fixes #213 —
ash.rebuild_partitions()silently strips every reader's grants.Found by the pre-tag audit (#160), and independently rediscovered during the PG 17 gate run.
Draft on purpose: needs REV and your explicit approval.
The bug
rebuild_partitions()dropsash.query_map_all, recreatessample_N/query_map_N, and rebuildsthe view — replaying none of the grants. Afterwards, as a reader:
aas(),summary(),samples(),top(),chart(),timeline()all fail the same way. A plainpg_monitormember is hit too, since 2.0 grants it the reader bundle by default.Onset is delayed, which is what makes it nasty: right after the rebuild, raw storage is empty
and every reader returns fine. The outage lands minutes later when the first samples arrive — so
the obvious smoke test passes. The regression test here therefore rebuilds, then samples, then
reads as the reader role.
The fix
Snapshots explicit SELECT ACLs for every recreated relation, then restores them:
ash.grant_reader(), so newly numbered partitions are coverednever widened to the full bundle
Also fixes README:376-378, which attributed the loss to "new partitions" when the recreated view
is the first blocker, and never warned that a rebuild destroys the default
pg_monitorgrant.Verification
RED against unpatched SQL, after
take_sample()inserted exactly one row — the real error above,reproduced through the delayed-onset path rather than an ACL-only check.
GREEN on PG 18.3 and PG 17, through a 3→4→3 rebuild cycle:
pg_monitorboth callash.status()andash.samples()successfully after samplingquery_map_0:true, sample_1:false— not widened{}Timeout:PgSleeprow;samples_total = 1,num_partitions = 4Fresh install, double re-apply, and the full 1.0→2.0 chain all clean with zero errors or warnings.
Review note
No shared helper was factored out with the installer's equivalent #107 block (
:6283-6360),deliberately: that path spans a script-wide function-recreation cycle while this one is
transaction-local and relation-specific, and unifying them would drag install-order and hardening
dependencies into an already-verified path. Reasonable, but it does mean two places now implement
grant preservation — worth a look if you'd rather they converge.
🤖 Generated with Claude Code
https://claude.ai/code/session_014HzBGzjFyN8dXZHbdWmYBj