Skip to content

Remove an invalid optimization when reading WAL #477

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

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented Jul 21, 2025

It is a bit unclear what this optimization was supposed to do, presumably it was to break from the loop once the whole buffer was decrypted, but the logic was very confused since if we want to do a similar optimization it should be something like if (dec_off + read_sz == offset + readsz).

@jeltz jeltz requested review from dutow and dAdAbird as code owners July 21, 2025 19:27
It is a bit unclear what this optimization was supposed to do,
presumably it was to break from the loop once the whole buffer was
decrypted, but the logic was very confused since if we want to do a
similar optimization it should be something like
if (dec_off + read_sz == offset + readsz).
@jeltz jeltz force-pushed the tde/rm-invalid-wal-optimization branch from 12542e1 to c1d1d29 Compare July 21, 2025 19:27
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.67%. Comparing base (54d0128) to head (c1d1d29).
Report is 6 commits behind head on TDE_REL_17_STABLE.

❌ Your project status has failed because the head coverage (83.67%) 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     #477      +/-   ##
=====================================================
+ Coverage              83.47%   83.67%   +0.19%     
=====================================================
  Files                     21       21              
  Lines                   2766     2763       -3     
  Branches                 435      433       -2     
=====================================================
+ Hits                    2309     2312       +3     
+ Misses                   371      367       -4     
+ Partials                  86       84       -2     
Components Coverage Δ
access 82.20% <ø> (+1.08%) ⬆️
catalog 87.89% <ø> (ø)
common 77.77% <ø> (ø)
encryption 73.45% <ø> (ø)
keyring 73.21% <ø> (ø)
src 87.52% <ø> (ø)
smgr 94.85% <ø> (-0.03%) ⬇️
transam ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeltz jeltz merged commit 4a9e091 into percona:TDE_REL_17_STABLE Jul 23, 2025
18 of 21 checks passed
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