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

Deflake DBBlockCacheTest1.WarmCacheWithBlocksDuringFlush #10635

Closed
wants to merge 2 commits into from

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented Sep 4, 2022

Previously, automatic compaction could be triggered prior to the test invoking CompactRange(). It could lead to the following flaky failure:

/root/project/db/db_block_cache_test.cc:753: Failure
Expected equality of these values:
  1 + kNumBlocks
    Which is: 11
  options.statistics->getTickerCount(BLOCK_CACHE_INDEX_ADD)
    Which is: 10

A sequence leading to this failure was:

  • Automatic compaction
    • files [1] [2] trivially moved
    • files [3] [4] [5] [6] trivially moved
  • CompactRange()
    • files [7] [8] [9] trivially moved
    • file [10] trivially moved

In such a case, the index/filter block adds that the test expected did not happen since there were no new files.

This PR just tweaks settings to ensure the CompactRange() produces one new file.

@facebook-github-bot
Copy link
Contributor

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ajkr ajkr requested a review from cbi42 September 4, 2022 20:57
Copy link
Member

@cbi42 cbi42 left a comment

Choose a reason for hiding this comment

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

LGTM thanks.

Curious what's causing CompactRange() to do or not do trivial move in this case? The files are all non-overlapping whether there's automatic compaction or not.

@ajkr
Copy link
Contributor Author

ajkr commented Sep 4, 2022

Thanks for the review! If CompactRange() picked [7] [8] [9] [10] it would overlap [2] .. [6] on L1, because it's using std::to_string() which does not do zero padding so [10] gets ordered between [1] and [2].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants