feat: allow ALTER TABLE to modify the skip_wal option dynamically#8048
Draft
JoeS51 wants to merge 6 commits intoGreptimeTeam:mainfrom
Draft
feat: allow ALTER TABLE to modify the skip_wal option dynamically#8048JoeS51 wants to merge 6 commits intoGreptimeTeam:mainfrom
JoeS51 wants to merge 6 commits intoGreptimeTeam:mainfrom
Conversation
Signed-off-by: Johannes Sluis <joesluis51@gmail.com>
Signed-off-by: Johannes Sluis <joesluis51@gmail.com>
Signed-off-by: Johannes Sluis <joesluis51@gmail.com>
Signed-off-by: Johannes Sluis <joesluis51@gmail.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a skip_wal option for regions and tables, allowing the Write-Ahead Log to be bypassed for specific operations. The changes include updates to region and table options, modifications to the write and close paths to respect the new flag, and validation to prevent enabling WAL on legacy regions created with a no-op provider. Review feedback suggests improving maintainability by refactoring the WAL-skipping check into a centralized helper method in RegionOptions and simplifying the validation logic in the alter handler for better idiomaticity.
…d be skipped Signed-off-by: Johannes Sluis <joesluis51@gmail.com>
7440d5f to
37a4c2b
Compare
Signed-off-by: Johannes Sluis <joesluis51@gmail.com>
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.
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
Fixes this issue
What's changed and what's your intention?
This PR now allows the ALTER TABLE command to update
skip_walat runtime instead of only at table creation time. To support this operation safely, this PR separates WAL provider configuration from the runtime “skip WAL” toggle. Previously, Greptime usedWalOptions::Noopto indicate that a region should skip WAL writes. This makes re-enabling WAL unsafe because the original provider information will be be discarded. (see discussion)This PR introduces skip_wal as a new region option and uses it as the runtime signal for whether WAL writes should be skipped, while preserving the region’s provider specific wal_options. New regions created with skip_wal=true still keep real WAL provider metadata, so WAL can later be re-enabled safely.
For backwards compatibility, legacy regions that already persist WalOptions::Noop are still supported as "skip wal" regions, but attempting to re-enable WAL for those legacy regions now fails explicitly because the original provider information is unavailable (please let me know if there is a better solution for this).
PR Checklist
Please convert it to a draft if some of the following conditions are not met.