XLog a key rotation event rather than the result #225
Merged
Conversation
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
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (75.45%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## TDE_REL_17_STABLE #225 +/- ##
=====================================================
- Coverage 76.29% 75.45% -0.85%
=====================================================
Files 23 22 -1
Lines 2523 2465 -58
Branches 395 388 -7
=====================================================
- Hits 1925 1860 -65
- Misses 521 532 +11
+ Partials 77 73 -4
🚀 New features to boost your workflow:
|
dAdAbird
force-pushed
the
xlog_rotate_key
branch
from
b50f546 to
8326d94
Compare
jeltz
requested changes
Apr 17, 2025
Collaborator
jeltz
left a comment
jeltz
left a comment
There was a problem hiding this comment.
I like the general approach but I have some feedback on the code.
dAdAbird
force-pushed
the
xlog_rotate_key
branch
from
8326d94 to
e534a67
Compare
dAdAbird
force-pushed
the
xlog_rotate_key
branch
from
e534a67 to
a34f85f
Compare
Before this commit, we Xlogged the binary result of the _map file content during key rotation. This led to issues: 1. Replicas would rewrite their own WAL keys with the primary's ones. And WAL keys are different on replicas. The same would have happened with SMGR keys since we're also planning to have them different across replicas. 2. The crash recovery would rewrite the latest WAL key as it's being created before redo. This commit switches to rather Xlogging the event of rotation (to which key should rotate) and lets redo/replicas perform the actual rotation. Fixes PG-1468, PG-1541
dAdAbird
force-pushed
the
xlog_rotate_key
branch
from
a34f85f to
b1da29f
Compare
jeltz
approved these changes
Apr 22, 2025
Collaborator
jeltz
left a comment
jeltz
left a comment
There was a problem hiding this comment.
Looks good but it would be nice if we could also start writing TAP tests for things like this. I started with a very simple TAP test for replication.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Before this commit, we Xlogged the binary result of the _map file content during key rotation. This led to issues:
This commit switches to rather Xlogging the event of rotation (to which key should rotate) and lets redo/replicas perform the actual rotation.
Fixes PG-1468, PG-1541