fix(e2e): tolerate missing system.replicas grant in mv_replay - #226
Merged
Merged
Conversation
ObsessionDB's managed role cannot SELECT system.replicas, so the replica count added in #225 failed the empty-target e2e with ACCESS_DENIED. Fall back to one visibility sample, skip SYSTEM SYNC REPLICA only when that command is denied too, and keep parallel-replica disable plus empty-chunk replay. Co-authored-by: Marc Höffl <marc.hoeffl@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
2 of 3 tasks
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
2 participants
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
#225counts active replicas fromsystem.replicasbefore the empty-target mv_replay e2e. ObsessionDB's managed role cannotSELECTthat table (ClickHouse code 497ACCESS_DENIED), sobeforeAllfailed before any chunk ran.The harness still reads
system.replicaswhen the grant exists, and still runsSYSTEM SYNC REPLICA … LIGHTWEIGHTon Shared/Replicated engines. A missing grant no longer fails the test: replica count falls back to one visibility sample, and sync is skipped only if that command is denied too.enable_parallel_replicas = 0and the empty-chunk replay under a new plan id are unchanged.Test plan
ACCESS_DENIEDon the replica count and onSYSTEM SYNC REPLICA, and still require one sample per active replica when the count is readableverifycovers the single-node MergeTree path (that engine never queriessystem.replicas)obsessiondbjob runs only on pushes tomain, so this PR's CI will not prove the live path. After merge, watch that job. A green PRverifydoes not cover the denied-grant fallback against ObsessionDB.