Repository navigation
Conversation
When DELETE /db/ runs on a node after a peer deleted the database, the node now removes the database locally and returns 200 instead of 500. If the config doc is already gone when the node deletes it, the node still finalizes the registry, so no delete stays marked in progress. Registry finalize now removes the entry only if it is still the delete that this node marked. Before, a peer could recreate the database after the config doc was deleted, and finalize removed the new registry entry. Add LeakyBootstrapConnection with hooks around DeleteMetadataDocument, so that tests can simulate a peer that writes during a delete. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
Droid finished @torcolvin's task —— View job Review summary The persistent config delete handling looks good and the test coverage for concurrent deletion/recreation cases is strong. The main follow-up is keeping the new leaky bootstrap connection behavior consistent across all test harness code paths that rebuild bootstrap connections or add nodes. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Interrupted-delete recovery returns ErrConfigRegistryRollback, which remains mapped to HTTP 500.
Review effort: Balanced
Findings: 1
What changed in this PR
Improves multi-node database deletion recovery and prevents deletion finalization from removing a recreated registry entry.
Changes:
- Treats missing config documents as successful concurrent deletion.
- Guards registry finalization using the original config version.
- Adds bootstrap hooks and multi-node concurrency tests.
| File | Description |
|---|---|
base/leaky_bootstrap_connection.go |
Adds test hooks around metadata deletion. |
rest/admin_api.go |
Handles peer-deleted database configs. |
rest/config_manager.go |
Finalizes concurrent deletes safely. |
rest/persistent_config_test.go |
Adds deletion race and recovery tests. |
rest/server_context.go |
Installs the leaky bootstrap wrapper. |
rest/utilities_testing.go |
Exposes wrapper configuration to tests. |
rest/utilities_testing_rest_tester_cluster.go |
Supports per-node test hooks. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2 of 4 tasks
bbrks
approved these changes
Sep 29, 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-5731 Finish a database delete when another node already deleted it
When DELETE /db/ runs on a node after a peer deleted the database, the node now removes the database locally and returns 200 instead of 500. If the config doc is already gone when the node deletes it, the node still finalizes the registry, so no delete stays marked in progress.
Registry finalize now removes the entry only if it is still the delete that this node marked. Before, a peer could recreate the database after the config doc was deleted, and finalize removed the new registry entry.
Add LeakyBootstrapConnection with hooks around DeleteMetadataDocument, so that tests can simulate a peer that writes during a delete.
I don't think this blocks this commit but some tests are skipped on rosmar due to #8832
Pre-review checklist
base.UD(docID),base.MD(dbName))docs/apiIntegration Tests