Skip to content

Add obsolete account test support to geyser#7714

Merged
roryharr merged 1 commit intoanza-xyz:masterfrom
roryharr:geyser_test_fixes_obsolete_accounts
Aug 27, 2025
Merged

Add obsolete account test support to geyser#7714
roryharr merged 1 commit intoanza-xyz:masterfrom
roryharr:geyser_test_fixes_obsolete_accounts

Conversation

@roryharr
Copy link
Copy Markdown

@roryharr roryharr commented Aug 25, 2025

Problem

Geyser tests expect that clean hasn't been run, and it accomplishes this by flushing slots individually. With obsolete accounts, clean is still run in this scenario, leading to a test failure.

Summary of Changes

  • Switch to a version of flush_write_cache that does not perform clean. This assumes that the test is attempting to simulate on going scans during flush.

Fixes #

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.0%. Comparing base (1efc488) to head (e55f2db).
⚠️ Report is 2383 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7714   +/-   ##
=======================================
  Coverage    83.0%    83.0%           
=======================================
  Files         812      812           
  Lines      357076   357088   +12     
=======================================
+ Hits       296666   296698   +32     
+ Misses      60410    60390   -20     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roryharr roryharr force-pushed the geyser_test_fixes_obsolete_accounts branch 2 times, most recently from 6a7e072 to a25a145 Compare August 26, 2025 18:34
@roryharr roryharr force-pushed the geyser_test_fixes_obsolete_accounts branch from a25a145 to e55f2db Compare August 26, 2025 19:20
@roryharr roryharr marked this pull request as ready for review August 26, 2025 20:22
@brooksprumo
Copy link
Copy Markdown

This assumes that the test is attempting to simulate on going scans during flush.

Wdym here, I'm not sure I follow? The test is w.r.t. geyser's account notification at startup from snapshot, so there is no foreground transaction replay nor rpc request handling yet running.

Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

@roryharr
Copy link
Copy Markdown
Author

This assumes that the test is attempting to simulate on going scans during flush.

Wdym here, I'm not sure I follow? The test is w.r.t. geyser's account notification at startup from snapshot, so there is no foreground transaction replay nor rpc request handling yet running.

Will post this here in case anyone is wondering in the future!

What I mean to say is that this is simulating scans prior to the restart. It's forcing every slot to be fully flushed and not cleaned so multiple versions of the accounts are guaranteed to be present.

In a non RPC validator, not all older version of accounts will be found during replay as they will be cleaned/shrunk etc...

@roryharr roryharr merged commit 7efa1df into anza-xyz:master Aug 27, 2025
43 checks passed
@brooksprumo
Copy link
Copy Markdown

this is simulating scans prior to the restart.

What do you mean here by "scans"?

It's forcing every slot to be fully flushed and not cleaned so multiple versions of the accounts are guaranteed to be present.

Yeah, the test is making storage files for the test. But that's all just the setup, so we can change it to still end up with storages that have duplicates across slots.

In a non RPC validator, not all older version of accounts will be found during replay as they will be cleaned/shrunk etc...

I'm not sure I follow this part either. This test is exercising geyser account notifications for startup, which happens before replay begins.

alexpyattaev pushed a commit to alexpyattaev/agave that referenced this pull request Aug 27, 2025
Modify geyser test to avoid flushing the write cache
alexpyattaev pushed a commit to alexpyattaev/agave that referenced this pull request Aug 27, 2025
Modify geyser test to avoid flushing the write cache
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