Repository navigation
[4.1.3 Backport] CBG-5913: Make database config updates safe against concurrent registry writers - #8893
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The cross-node regression test is unconditionally skipped because the exercised concurrency path can still panic.
1 open finding
What changed in this PR
Improves persistent database configuration updates under concurrent registry writes and reloads.
Changes:
- Reuses update callback results when the source config CAS is unchanged.
- Reloads stale registries and safely updates in-memory config CAS values.
- Adds concurrency test infrastructure and regression coverage.
| File | Description |
|---|---|
rest/admin_api.go |
Guards in-memory CAS assignment after concurrent reloads. |
rest/cluster_compat_test.go |
Tests updates during peer heartbeats. |
rest/config_concurrency_test.go |
Adds cross-node concurrency test, currently skipped. |
rest/config_manager.go |
Improves config and registry retry handling. |
rest/config_manager_test.go |
Tests stale registry reads after updates and deletes. |
rest/utilities_testing.go |
Adds bucket connection hooks and safer URL templating. |
rest/utilities_testing_rest_tester_cluster.go |
Propagates connection hooks across test nodes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…concurrent registry writers Cherry-picked from 9b2008d (CBG-5909, #8836), adapted to 4.1.3 without its main-only prerequisites (#8486, CBG-4244, CBG-5731), per team advice. - rest/config_manager.go: main's UpdateConfig retry loop comes from #8486, not on 4.1.3. Kept 4.1.3's loop and added the fix's guard: a callbackCfgCas tracks which bucket config produced updatedConfig, and the update callback is re-run when the config changed underneath it. - rest/utilities_testing_rest_tester_cluster.go: conflicts came from CBG-4244/CBG-5731. Kept 4.1.3's file and added only what the new tests need: the ConnectToBucketFn option and its pass-through to each RestTester. - rest/config_concurrency_test.go: imports renamed from the testing/{require,assert} wrappers to testify, which 4.1.3 uses. Co-authored-by: Tor Colvin <tor.colvin@couchbase.com>
Collaborator
|
@vipbhardwaj is there a jenkins integration test for the latest rebased version of this change? |
Contributor
Author
|
Added it to the description ✅ |
torcolvin
approved these changes
Oct 28, 2026
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.
CBG-5913 (backport of CBG-5909)
PUT/POST
/db/_configcould return 500 when a database reload took more than 20s, because a peer node's registry write (for example a heartbeat) changed the bucket config between reading it and running the update callback. Config updates now re-run the callback whenever the bucket config they were computed from has changed.Unclean cherry pick of #8836 to 4.1.3
Adapted to 4.1.3 without main-only prerequisites (#8486, CBG-4244, CBG-5731), per team advice.
Changes from main commit:
rest/config_manager.go— main'sUpdateConfigretry loop comes from Fix flaky TestRaceBetweenConfigPollAndDbConfigUpdate #8486, which is not on 4.1.3; kept 4.1.3's loop and added the fix's guard (callbackCfgCasrecords which bucket config producedupdatedConfig; the callback re-runs when it changes, and its error returns0, callbackErr)rest/utilities_testing_rest_tester_cluster.go— conflicts came from CBG-4244 / CBG-5731; kept 4.1.3's file and added only theConnectToBucketFnoption and its pass-through to eachRestTester, which the new tests needrest/config_concurrency_test.go— release branch predates thetesting/{require,assert}wrappers; imports renamed back togithub.com/stretchr/testify/{require,assert}🤖 Opened with the
sync-gateway-backportskill in Claude CodeLink to the integration test run for the same PR : https://jenkins.sgwdev.com/job/SyncGatewayIntegration-Pipeline/181/