Skip to content
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

Remove dummy entries in Blockstore special columns #33511

Merged
merged 4 commits into from
Oct 3, 2023

Conversation

steviez
Copy link
Contributor

@steviez steviez commented Oct 3, 2023

Problem

These entries were found to improve compaction performance when
LedgerCleanupService was performing direct compactions on each primary
index. Cleaning by primary index has been deprecated for a while, and
as such, these dummy entries are no longer needed and can be removed.

Summary of Changes

Remove the dummy entries + a few comments that will no longer be correct

This is broken out from #33370

Steven Czabaniuk added 2 commits October 3, 2023 12:52
These entries were found to improve compaction performance when ledger
compaction was done by LedgerCleanupService on a primary index. Cleaning
by primary index has been deprecated for a while, and these dummy
entries are no longer needed.
@steviez steviez changed the title Bstore rm dummy entries Remove dummy entries in Blockstore special columns Oct 3, 2023
@steviez steviez requested a review from CriesofCarrots October 3, 2023 20:06
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #33511 (e9c6026) into master (3008cd8) will increase coverage by 0.0%.
The diff coverage is 93.3%.

@@           Coverage Diff           @@
##           master   #33511   +/-   ##
=======================================
  Coverage    81.7%    81.7%           
=======================================
  Files         802      802           
  Lines      217835   217832    -3     
=======================================
+ Hits       178123   178137   +14     
+ Misses      39712    39695   -17     

@steviez steviez merged commit 9761e6f into solana-labs:master Oct 3, 2023
16 checks passed
@steviez steviez deleted the bstore_rm_dummy_entries branch October 3, 2023 20:53
tao-stones pushed a commit to tao-stones/solana that referenced this pull request Oct 6, 2023
These entries were found to improve compaction performance when
LedgerCleanupService was performing direct compactions on each primary
index. Cleaning by primary index has been deprecated for a while, and
as such, these dummy entries are no longer needed and can be removed.
@steviez
Copy link
Contributor Author

steviez commented Oct 10, 2023

Dang, I just realized this commit won't function as intended. The function that removes the dummy entries only gets called if the TransactionStatusIndex entries are not populated. Those entries will only be absent on a brand new Blockstore that also does not have the dummy entries yet populated. I'll fix this

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