Skip to content

Remove our own shared memory tooling #408

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

Merged
merged 3 commits into from
Jun 10, 2025

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented Jun 10, 2025

We had an own layer on top of PostgreSQL's shared memory utilities which was of doubtful usefulness since while it may have saved a few bytes of memory if we would have had many users of the system it also made it harder to inspect our allocations since a lot of the built-in tooling could not be used.

It also turns out there was only one serious user of the code, the principal key code. The other user, the key provider code, actually did not store anything interesting in shared memory at all. And with only on real user these inhouse utilities added even less value.

@jeltz jeltz requested review from dutow and dAdAbird as code owners June 10, 2025 09:10
@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2025

Codecov Report

Attention: Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.23%. Comparing base (1749d8c) to head (b213f1c).
Report is 5 commits behind head on TDE_REL_17_STABLE.

❌ Your project status has failed because the head coverage (85.23%) 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     #408      +/-   ##
=====================================================
- Coverage              85.51%   85.23%   -0.28%     
=====================================================
  Files                     22       21       -1     
  Lines                   2547     2499      -48     
  Branches                 389      385       -4     
=====================================================
- Hits                    2178     2130      -48     
  Misses                   293      293              
  Partials                  76       76              
Components Coverage Δ
access 83.73% <ø> (ø)
catalog 88.71% <96.55%> (-0.18%) ⬇️
common 77.77% <ø> (-14.03%) ⬇️
encryption 72.56% <ø> (ø)
keyring 72.94% <ø> (ø)
src 91.40% <100.00%> (-0.02%) ⬇️
smgr 94.88% <ø> (ø)
transam ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeltz jeltz force-pushed the tde/clean-shmem2 branch from 44ab87d to 5e3e1c0 Compare June 10, 2025 09:22
@jeltz jeltz force-pushed the tde/clean-shmem2 branch 2 times, most recently from 8edf4c6 to 7971e17 Compare June 10, 2025 09:58
Copy link
Collaborator

@AndersAstrand AndersAstrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I had a naming question, but I realize that name was weird even before this PR.

jeltz added 3 commits June 10, 2025 14:58
Since the only thing our key provider code does with shared memory is
look up a LWLock tranche it is quite a waste of lines of codes to use
our own layer on top of the PostgreSQL shared memory.
Since it just has one user now let's remove our own shared memory code
and remove a lot of generic code that we do not need.
Now that we have removed our shared memory tools the remaining contents
of the header can be moved elsewhere.
@jeltz jeltz force-pushed the tde/clean-shmem2 branch from 7971e17 to b213f1c Compare June 10, 2025 13:03
@jeltz jeltz merged commit 589bcab into percona:TDE_REL_17_STABLE Jun 10, 2025
16 checks passed
@jeltz jeltz deleted the tde/clean-shmem2 branch June 13, 2025 12:40
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.

3 participants