Skip to content

Tracks last cleaned slot in accounts background service#6396

Merged
brooksprumo merged 1 commit into
anza-xyz:masterfrom
brooksprumo:abs/last-cleaned-slot
Jun 4, 2025
Merged

Tracks last cleaned slot in accounts background service#6396
brooksprumo merged 1 commit into
anza-xyz:masterfrom
brooksprumo:abs/last-cleaned-slot

Conversation

@brooksprumo
Copy link
Copy Markdown

Problem

The main problem is #6295. As a means to that end, we need to ensure we don't clean past any enqueued snapshot request's slot. Currently, accounts background service tracks the last cleaned block height, which doesn't easily map to slot (when calling AccountsDb::clean_accounts(), which takes max_clean_root_inclusive, which is a Slot).

Summary of Changes

Track the last cleaned slot, instead of block height.

@brooksprumo brooksprumo self-assigned this Jun 3, 2025
@brooksprumo brooksprumo marked this pull request as ready for review June 3, 2025 20:16
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 3, 2025

Codecov Report

Attention: Patch coverage is 61.53846% with 5 lines in your changes missing coverage. Please review.

Project coverage is 82.8%. Comparing base (39f3aa8) to head (7ec06e9).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #6396    +/-   ##
========================================
  Coverage    82.8%    82.8%            
========================================
  Files         848      848            
  Lines      379048   379050     +2     
========================================
+ Hits       313859   314021   +162     
+ Misses      65189    65029   -160     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brooksprumo brooksprumo requested review from HaoranYi and roryharr June 3, 2025 20:39
HaoranYi
HaoranYi previously approved these changes Jun 3, 2025
Copy link
Copy Markdown

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm.

@brooksprumo
Copy link
Copy Markdown
Author

Force-pushed to resolve merge conflicts. No code was changed.

@brooksprumo brooksprumo requested a review from HaoranYi June 3, 2025 23:23
@jeffwashington
Copy link
Copy Markdown

i'm not sure about skipped slots. we'll clean more often when we skip slots. Today we aren't skipping many slots on mnb. Just thinking about attacks, changes in behavior, etc.

@brooksprumo
Copy link
Copy Markdown
Author

brooksprumo commented Jun 4, 2025

i'm not sure about skipped slots. we'll clean more often when we skip slots. Today we aren't skipping many slots on mnb. Just thinking about attacks, changes in behavior, etc.

Yep! This means we'll strictly clean more than before. Since mnb's skip rate is 0.2%, we'll only be changing the duration between calls to clean by like 1 slot. Note there's already a random amount applied to when we call 'clean' too. And really, when snapshots are enabled, we're cleaning each snapshot request, which have different (longer) runtimes than the no-snapshot-request paths.

Specifically, when snapshots are enabled (assuming the default 100 slot incremental snapshot interval), we never go through this else-if block, which is really the only thing that changed.

                        } else if bank.slot() - last_cleaned_slot
                            > (CLEAN_INTERVAL_SLOTS + thread_rng().gen_range(0..10))

@brooksprumo brooksprumo merged commit 05a4921 into anza-xyz:master Jun 4, 2025
47 checks passed
@brooksprumo brooksprumo deleted the abs/last-cleaned-slot branch June 4, 2025 17:03
mircea-c added a commit to mircea-c/agave that referenced this pull request Jun 12, 2025
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.

5 participants