Skip to content

PG-1441 Do not replicate relation keys #234

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

Open
wants to merge 4 commits into
base: TDE_REL_17_STABLE
Choose a base branch
from

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented Apr 17, 2025

Instead of replicating relation keys we generate new ones on replay of
the XLOG_TDE_ADD_RELATION_KEY record at the replica server. This means a
replica and its master server will end up with different sets of
relation keys making a simple binary diff impossible but that is a
dubious advantage since the WAL keys will differ anyway and on on the
flip-side the new code is simpler and easier to reason about. Especially
since now WAL keys and relation keys are treated in a bit more similar
ways.

To prevent duplicate keys in the key file we skip generating and adding
a key if there already is an entry in the file for the same relation.

Also while changing this we make sure to add a simple replication test plus add code preventing the SMGR code at a replica from generating any new keys. Keys should only be created on WAL replay. But these changes should have been done either way.

@jeltz jeltz requested review from dutow and dAdAbird as code owners April 17, 2025 23:48
@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2025

Codecov Report

Attention: Patch coverage is 77.55102% with 11 lines in your changes missing coverage. Please review.

Project coverage is 77.69%. Comparing base (3b6d3d4) to head (01a5801).

❌ Your project status has failed because the head coverage (77.69%) 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     #234      +/-   ##
=====================================================
+ Coverage              76.09%   77.69%   +1.59%     
=====================================================
  Files                     23       23              
  Lines                   2523     2520       -3     
  Branches                 394      396       +2     
=====================================================
+ Hits                    1920     1958      +38     
+ Misses                   526      483      -43     
- Partials                  77       79       +2     
Components Coverage Δ
access 76.79% <62.06%> (+5.27%) ⬆️
catalog 86.41% <ø> (+0.34%) ⬆️
common 92.50% <ø> (ø)
encryption 71.90% <ø> (ø)
keyring 69.87% <ø> (ø)
src 53.94% <ø> (+0.98%) ⬆️
smgr 97.91% <100.00%> (+1.14%) ⬆️
transam 95.91% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jeltz added 4 commits April 18, 2025 01:57
The old code was harder to read than necessary since it had exactly two
callers of which one had each value of the boolean flag. Breaking it up
into two functions makes the intent clearer. While at it we also clean
up the flow a bit more.
This way we can avoid obvious regression when refactoring the code for
replicating keys in future commits. This test can in the future be
expanded to test more interesting cases.
Make sure we can never generate relation keys on a streaming replica or
in recovery in the SMGR code. Instead the key should always have been
already generated when replaying the XLOG_TDE_ADD_RELATION_KEY record.
Instead of replicating relation keys we generate new ones on replay of
the XLOG_TDE_ADD_RELATION_KEY record at the replica server. This means a
replica and its master server will end up with different sets of
relation keys making a simple binary diff impossible but that is a
dubious advantage since the WAL keys will differ anyway and on on the
flip-side the new code is simpler and easier to reason about. Especially
since now WAL keys and relation keys are treated in a bit more similar
ways.

To prevent duplicate keys in the key file we skip generating and adding
a key if there already is an entry in the file for the same relation.
@jeltz jeltz force-pushed the tde/no-relkey-replication branch from 3fa064b to 01a5801 Compare April 17, 2025 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants