Skip to content

PoC: Optimize key creation #502

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

Closed

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented Aug 6, 2025

This optimizes key creation in both when doing it and in the WAL replay. Plus fixes a bug when we did unnecessary work when working with temporary table.

Optimizations:

  • Do not log a separate delete key entry when creating an encrypted table since we can delete the old key as part of replaying the key creating
  • Replace the old key when scanning the key file instead of first deleting and then adding
  • Very ugly and experimental use of fread() instead of pread() to reduce number of syscalls

@jeltz jeltz force-pushed the tde/optimize-wal-replay-3 branch from 22dd143 to 2dd1e74 Compare August 6, 2025 19:43
@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2025

Codecov Report

❌ Patch coverage is 94.23077% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (TDE_REL_17_STABLE@602cd73). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on TDE_REL_17_STABLE.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             TDE_REL_17_STABLE     #502   +/-   ##
====================================================
  Coverage                     ?   82.24%           
====================================================
  Files                        ?       25           
  Lines                        ?     3199           
  Branches                     ?      519           
====================================================
  Hits                         ?     2631           
  Misses                       ?      457           
  Partials                     ?      111           
Components Coverage Δ
access 83.44% <90.32%> (?)
catalog 87.61% <ø> (?)
common 77.77% <ø> (?)
encryption 72.97% <ø> (?)
keyring 73.21% <ø> (?)
src 94.15% <ø> (?)
smgr 95.90% <100.00%> (?)
transam ∅ <ø> (?)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AndersAstrand
Copy link
Collaborator

AndersAstrand commented Aug 7, 2025

I wonder if we should explore more drastic options, such as having one key file per relation, to just remove the issue of having to scan the key file completely.

It feels like it's always going to be a bottle neck when dealing with a lot of tables (ie, large key files)

@dutow
Copy link
Collaborator

dutow commented Aug 7, 2025

I wonder if we should explore more drastic options, such as having one key file per relation, to just remove the issue of having to scan the key file completely.

Definitely, but later :)

@AndersAstrand
Copy link
Collaborator

Another idea is to always add keys to the end of the file and take the cost while reading instead. Not sure that's what we want to do though depending on what we want to optimize for.

@jeltz jeltz force-pushed the tde/optimize-wal-replay-3 branch from 2dd1e74 to 90a0e0e Compare August 8, 2025 13:01
jeltz added 3 commits August 8, 2025 15:32
We forgot to have a check against trying to delete leftover SMGR keys
for temporary tables which is a useless operation since they are store
in memory.

Additionally we forgot to prevent WAL from being written when creating
or removing a key in smgrcreate() for temporary tables.
@jeltz jeltz force-pushed the tde/optimize-wal-replay-3 branch from 90a0e0e to db898dc Compare August 9, 2025 13:18
@jeltz
Copy link
Collaborator Author

jeltz commented Aug 13, 2025

Part of this has already been merged as #529. The rest is for another day.

@jeltz jeltz closed this Aug 13, 2025
@jeltz jeltz deleted the tde/optimize-wal-replay-3 branch August 13, 2025 13:18
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.

4 participants