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

blockstore: Update flaky compaction test #5131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

steviez
Copy link

@steviez steviez commented Mar 4, 2025

Problem

See #4188 for more context, but the TLDR:

  • We have a custom compaction filter that deletes data
  • We have a unit tests that relies on compaction running & deleting this data
  • We have found our unit test is flaky because rocksdb will sometimes no-op when we request a compaction; this is rocksdb functioning as intended but just unfortunate for us. rocksdb was essentially seeing no work to be done in this case

Summary of Changes

Update the compaction call to NOT take start/end keys, which results in the entire column being considered for compaction. This results in a different call path in rocksdb being used which picks up the overlapping data (which signals to run a compaction) every time.

Previously, I could run the failing unit test in a loop and see a failure within ~20 iterations, often times fewer. With this PR, I have run > 500 iterations without failure

This isn't necessarily a perfect solution; however, I think it is a "good enough" solution for now so our team can get back time from flaky CI. RPC will call blockstore functions that check the value of the atomic before proceeding; we could possibly go that route but then this test isn't exercising much more than setting and reading an integer

@steviez steviez requested review from bw-solana and roryharr March 4, 2025 05:24
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.

1 participant