fix(replication): wait for replicas to be ready to serve queries after deploy (fixes #131)#132
Merged
Merged
Conversation
…r deploy (fixes #131) After 'deploy replication' (master/slave), the initialize_slaves script now waits until each replica accepts a simple query from the sandbox user before returning. This eliminates the race where the first command on a just-started replica (e.g. ./s1) fails with 'Access denied' or connection errors, especially visible on MySQL 8.4+ and 9.x. - Added wait_until_replica_ready helper (modeled on wait_until_wsrep_ready). - Called after START REPLICA in both init_slaves and init_slaves_84 templates. - Self-contained function in the templates for robustness. - Also added the helper to sb_include.gotxt for reuse by other scripts. Tested via template changes; no behavior change for older versions.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a ChangesReplica readiness wait
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sandbox/templates/single/sb_include.gotxt (1)
87-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
wait_until_replica_readyis duplicated verbatim across three template files.The same function body appears in
sb_include.gotxt,init_slaves.gotxt(lines 9–28), andinit_slaves_84.gotxt(lines 10–29). If the polling logic needs a future fix (e.g., different timeout, error handling), all three copies must be kept in sync manually. Consider extracting the function into a shared Go template partial (e.g.,{{ template "wait_until_replica_ready" }}) that all three templates include, or having the init_slaves scripts sourcesb_includeat runtime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sandbox/templates/single/sb_include.gotxt` around lines 87 - 107, The wait_until_replica_ready function is duplicated across multiple templates, so update the implementation to come from a single shared definition instead of keeping verbatim copies in sb_include.gotxt, init_slaves.gotxt, and init_slaves_84.gotxt. Extract the polling logic into a shared Go template partial or shared sourced script and have the existing init_slaves templates reuse it, keeping the function name wait_until_replica_ready as the common entry point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sandbox/templates/single/sb_include.gotxt`:
- Around line 87-107: The wait_until_replica_ready function is duplicated across
multiple templates, so update the implementation to come from a single shared
definition instead of keeping verbatim copies in sb_include.gotxt,
init_slaves.gotxt, and init_slaves_84.gotxt. Extract the polling logic into a
shared Go template partial or shared sourced script and have the existing
init_slaves templates reuse it, keeping the function name
wait_until_replica_ready as the common entry point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 199ec1ce-dc33-4600-9f28-05408091fb27
📒 Files selected for processing (3)
sandbox/templates/replication/init_slaves.gotxtsandbox/templates/replication/init_slaves_84.gotxtsandbox/templates/single/sb_include.gotxt
- templates_flavor_test.go: new TestInitSlavesTemplates_IncludeReplicaReadyWait that renders init_slaves / init_slaves_84 with realistic slave data and asserts that wait_until_replica_ready is defined and invoked for each replica after START REPLICA. This is a pure template test (no binaries). - sandbox_test.go: in testCreateReplicationSandbox, immediately after CreateReplicationSandbox (master/slave, 3 nodes) run a query against the first replica (n1/use -BN -e 'SELECT 1;') with no intervening sleep. This exercises the wait added in initialize_slaves* and would have failed before the fix on 8.4+/9.x due to the race described in #131. Together these make the readiness behavior testable and prevent regression.
Lint failure: common.RunCmd takes only a string. Use RunCmdWithArgs(cmd, args) for arguments, matching existing usage in the codebase (e.g. data_load, tests).
- Query both replicas (n1 and n2) immediately after CreateReplicationSandbox with no extra sleep. - Assert the generated initialize_slaves script contains the wait_until_replica_ready calls. - One-line note on RunCmdWithArgs usage. This makes the test actually exercise and protect the readiness wait added for #131.
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
1 participant
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
After
dbdeployer deploy replication, the first command on a replica (e.g../s1) can fail with "Access denied" or connection errors because the deploy returns before the replica is ready to serve queries. This is a long-standing race, made worse on MySQL 8.4+ / 9.x.Root cause
CreateMasterSlaveReplicationrunsinitialize_slaves(orinit_slaves_84) which doesCHANGE ...+START REPLICAand exits immediately. There was no readiness gate for the replica to accept client connections/queries.Fix
Add a small
wait_until_replica_readyhelper (modeled on the existingwait_until_wsrep_readyfor PXC/Galera) that:SELECT 1via the replica'susescript until it succeeds (max 60s, 1s sleep).START REPLICAfor each replica, beforeinitialize_*returns.Changes:
sandbox/templates/replication/init_slaves.gotxtsandbox/templates/replication/init_slaves_84.gotxtsandbox/templates/single/sb_include.gotxt(added reusable helper)Behavior
Related
Fixes #131.
Summary by CodeRabbit