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

Set lower-bound on dynamic level sizes #2123

Closed
wants to merge 1 commit into from

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented Apr 10, 2017

Changed dynamic leveling to stop setting the base level's size bound below max_bytes_for_level_base.

Behavior for config where max_bytes_for_level_base == level0_file_num_compaction_trigger * write_buffer_size and same amount of data in L0 and base-level:

  • Before Level-based L0->L0 compaction #2027, compaction scoring would favor base-level due to dividing by size smaller than max_bytes_for_level_base.
  • After Level-based L0->L0 compaction #2027, L0 and Lbase get equal scores. The disadvantage is L0 is often compacted before reaching the num files trigger since write_buffer_size can be bigger than the dynamically chosen base-level size. This increased write-amp.
  • After this diff, L0 and Lbase still get equal scores. Now it takes level0_file_num_compaction_trigger files of size write_buffer_size to trigger L0 compaction by size, fixing the write-amp problem above.

Test Plan:

command: ./db_bench -benchmarks=fillrandom -options_file=/home/andrewkr/myrocks-options-modified -statistics -stats_per_interval=1 -stats_interval=10000000 -num=500000000

@facebook-github-bot
Copy link
Contributor

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

@ajkr
Copy link
Contributor Author

ajkr commented May 3, 2017

ping, I've been running fillrandom benchmarks on larger DBs (2B keys) and am finding this has some small benefit to stalling, throughput, and write-amp. I also think it's more intuitive to have the base level's size bound be exactly max_bytes_for_level_base, rather than a constantly-changing number less than that. Especially since we guide people to set max_bytes_for_level_base = L0 size, this diff is useful as it prevents the LSM from assuming an hourglass shape.

@ajkr ajkr requested a review from siying May 3, 2017 23:01
@ajkr
Copy link
Contributor Author

ajkr commented May 4, 2017

ping @siying, can we commit today or tomorrow?

@siying
Copy link
Contributor

siying commented May 5, 2017

@ajkr yes go ahead.

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