[sql-68] multi: add conf option to remove bbolt tombstone#1293
[sql-68] multi: add conf option to remove bbolt tombstone#1293ViktorT-11 wants to merge 2 commits into
Conversation
Add an explicit config option to remove kvdb deprecation markers from the accounts, session, and firewall stores so the legacy bbolt files can be reopened when an operator intentionally wants to bypass the tombstones. Wire that behavior into dev config through an unsafe startup flag that clears the markers before NewStores opens the bbolt backend. This keeps the override explicit and local to startup instead of weakening the normal deprecation checks. Add coverage for both parts of the flow: the store-specific deprecation tests now verify reopening works again after marker removal, and a new config test verifies startup fails before the unsafe flag and succeeds once the markers are cleared.
Extend the kvdb -> SQL migration itest to cover the unsafe startup flag that clears kvdb deprecation markers before reopening the bbolt stores. Verify that starting litd with the unsafe flag allows the migrated kvdb files to be opened again, that the migrated data is still readable over RPC, and that a later plain bbolt restart also succeeds once the markers have been removed. Rerun the SQL migration after that unsafe path so the deprecated-file startup-order checks can still run against freshly recreated tombstones.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces an opt-in, unsafe mechanism to remove bbolt database deprecation markers that are typically set after a migration to SQL. This provides a way to recover or reuse legacy bbolt files if necessary, while explicitly requiring the user to acknowledge the potential risks of such an action. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to remove KVDB deprecation markers from legacy bbolt database files, allowing them to be reopened if necessary. This is controlled by a new unsafe configuration flag. The changes include the implementation of removal functions for accounts, session, and firewall databases, updates to the configuration, and comprehensive integration tests. I have no other feedback to provide, but please address the grammatical error in the configuration description as suggested.
| // markers from the legacy bbolt database files before opening them. | ||
| // This should only be used if the caller fully understands the | ||
| // consequences of reusing kvdb files that were already migrated to SQL. | ||
| UnsafeRemoveDeprecatedKVDBMarkers bool `long:"unsafe-remove-kvdb-deprecation" description:"Remove the kvdb deprecation markers if set from accounts.db, session.db and rules.db before opening the bbolt backend. Only set this if you know fully understands the consequences of reusing kvdb files that were already migrated to SQL."` |
There was a problem hiding this comment.
The description contains a grammatical error: 'if you know fully understands'. It should be 'if you fully understand'.
| UnsafeRemoveDeprecatedKVDBMarkers bool `long:"unsafe-remove-kvdb-deprecation" description:"Remove the kvdb deprecation markers if set from accounts.db, session.db and rules.db before opening the bbolt backend. Only set this if you know fully understands the consequences of reusing kvdb files that were already migrated to SQL."` | |
| UnsafeRemoveDeprecatedKVDBMarkers bool `long:"unsafe-remove-kvdb-deprecation" description:"Remove the kvdb deprecation markers if set from accounts.db, session.db and rules.db before opening the bbolt backend. Only set this if you fully understand the consequences of reusing kvdb files that were already migrated to SQL."` |
2 participants
Based on #1272
This PR adds a config option
unsafe-remove-kvdb-deprecationthat removes the bbolt tombstone on the next startup the tombstone is added after a kvdb-> SQL migration.Pushing this in draft for now, so that we can decide if we actually want to go ahead with this PR or not.