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

Mv level work1 #73

Merged
merged 4 commits into from Mar 14, 2013
Merged

Mv level work1 #73

merged 4 commits into from Mar 14, 2013

Conversation

matthewvon
Copy link
Contributor

I took a series of constants and put them into a static structure such that I can vary the constants by level (instead of one set of constants for all levels). Then I found all places in this file that had special "if (level==0)" code and changed it to "if (m_LevelTraits[level].m_OverlappedFiles==true)". Finally, some code relating to the m_MaxGrandParentOverlapBytes constant was disabled.

Testing shows a 48% increase in write throughput. Slight increases in read and iterate throughput, but not worth mentioning.

@ghost ghost assigned matthewvon and engelsanchez Mar 11, 2013
// write buffer size of 60,000,000. Why five times: 4 level-0 files typically compact
// to one level-1 file and are each slightly larger than 60,000,000.
// level-1 file size of 1,500,000,000 applies to output file of this level
// being writtne to level-2. The value is five times the 300,000,000 of level-1.

Choose a reason for hiding this comment

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

Little typo "writtne" here

@engelsanchez
Copy link

Ok then. So the big change here, to summarize, is we allow overlapping ranges in levels 0-2 instead of just 0. Now compactions take into account moving up a level that allows overlapping ranges and will not merge any files from that level+1, hence possibly creating an overlapping file from the result of the level files. And these constants reflect tuning that you have done and allow you to keep fine tuning for each level. Also, we're now dumping memtables to level 0 only, since the optimization to figure out if it could go up some levels has been removed in favor of the above. Does that sound like a good summary?

It all sounds good to me and I can't find any problems in the code besides the comment/style nitpicks. I can't say I can verify the performance benchmarks. We'd need to clone @matthewvon for that :).

If you could elaborate a bit on why removing the trivial move optimization is OK, that would satisfy my curiosity.

But consider this a 👍 💃 already.

matthewvon pushed a commit that referenced this pull request Mar 14, 2013
@matthewvon matthewvon merged commit 05c900f into master Mar 14, 2013
@ghost ghost assigned matthewvon Mar 14, 2013
@matthewvon matthewvon deleted the mv-level-work1 branch May 5, 2016 17:32
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

2 participants