-
Notifications
You must be signed in to change notification settings - Fork 457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
db: ensure checkpointed database with WAL failover is openable #4130
Conversation
c27c13a
to
057260d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 8 files reviewed, 1 unresolved discussion (waiting on @itsbilal and @jbowens)
checkpoint.go
line 382 at r1 (raw file):
// because Checkpoint will copy all WAL segment files from both the primary and // secondary WAL directories into the checkpoint. func copyCheckpointOptions(fs vfs.FS, srcPath, dstPath string) error {
[nit] This could use a unit test (could be a datadriven test where we specify the src OPTIONS contents)
This commit adapts Checkpoint to omit the [WAL Failover] section of the OPTIONS file from the checkpoint's OPTIONS file. The WAL failover configuration is specific to the original database. We want our checkpoints to be fully encapsulated and complete, so we copy WAL files from both the primary WAL directory and the failover secondary. This means a database opening the checkpoint does not need the secondary directory to be provided as a WALRecoveryDir. With this commit, Checkpoint parses the old OPTIONS file and copies its contents verbatim to a new file in the checkpointed directory. If the old OPTIONS file had a WAL Failover configuration, its entire section is commented out within the checkpoint.
057260d
to
1154cb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 9 files reviewed, all discussions resolved (waiting on @itsbilal)
checkpoint.go
line 382 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] This could use a unit test (could be a datadriven test where we specify the src OPTIONS contents)
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 8 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @itsbilal)
TFTR! |
This commit adapts Checkpoint to omit the [WAL Failover] section of the OPTIONS file from the checkpoint's OPTIONS file. The WAL failover configuration is specific to the original database. We want our checkpoints to be fully encapsulated and complete, so we copy WAL files from both the primary WAL directory and the failover secondary. This means a database opening the checkpoint does not need the secondary directory to be provided as a WALRecoveryDir.
With this commit, Checkpoint parses the old OPTIONS file and copies its contents verbatim to a new file in the checkpointed directory. If the old OPTIONS file had a WAL Failover configuration, its entire section is commented out within the checkpoint.