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

Allow IntraL0 compaction in FIFO Compaction #2163

Closed
wants to merge 1 commit into from

Conversation

siying
Copy link
Contributor

@siying siying commented Apr 15, 2017

Summary: Allow an option for users to do some compaction in FIFO compaction, to pay some write amplification for fewer number of files.

Test Plan: Add a unit test

@siying
Copy link
Contributor Author

siying commented Apr 15, 2017

Some benchmark results:

Without the change:

[sdong@dev12075.prn1 /data/users/sdong/rocksdb7] TEST_TMPDIR=/dev/shm/ ./db_bench --benchmarks=readwhilewriting -compaction_style=2 --num=20000000 -level0_file_num_compaction_trigger=8 -bloom_bits=10 --threads=16

readwhilewriting : 0.725 micros/op 1378846 ops/sec; 54.7 MB/s (7070947 of 20000000 found)

With the change:

] TEST_TMPDIR=/dev/shm/ ./db_bench --benchmarks=readwhilewriting -compaction_style=2 --num=20000000 -level0_file_num_compaction_trigger=8 -bloom_bits=10 --threads=16 --fifo_compaction_allow_compaction --fifo_compaction_max_table_files_size_mb=1000

readwhilewriting : 0.485 micros/op 2062525 ops/sec; 78.0 MB/s (6766309 of 20000000 found)

With the change, the SST files under the directory after the compaction looks like this:

-rw-r--r--. 1 sdong users 241354289 Apr 14 17:45 000108.sst
-rw-r--r--. 1 sdong users 241391023 Apr 14 17:45 000125.sst
-rw-r--r--. 1 sdong users 241431557 Apr 14 17:45 000142.sst
-rw-r--r--. 1 sdong users 32490220 Apr 14 17:45 000144.sst
-rw-r--r--. 1 sdong users 32478382 Apr 14 17:45 000146.sst
-rw-r--r--. 1 sdong users 32498268 Apr 14 17:45 000148.sst

@facebook-github-bot
Copy link
Contributor

@siying updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@siying updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@siying updated the pull request - view changes - changes since last import

@ajkr
Copy link
Contributor

ajkr commented Apr 24, 2017

just curious what was the motivation to introduce this for FIFO compaction?

@siying
Copy link
Contributor Author

siying commented Apr 24, 2017

@ajkr a user sees high CPU overhead when there are many files in FIFO. I just want to create a solution for them to try.

Copy link
Contributor

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

LGTM, let's try it.

One thing I'm worried about is the intra-L0 compaction generating one giant file that's close to FIFO's max_table_files_size limit. Then the next flush will cause max_table_files_size to be exceeded. The giant file would be deleted, which contains almost all the data.

@@ -36,6 +36,39 @@ uint64_t TotalCompensatedFileSize(const std::vector<FileMetaData*>& files) {
}
return sum;
}

bool FindIntraLOCompaction(const std::vector<FileMetaData*>& level_files,
Copy link
Contributor

Choose a reason for hiding this comment

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

s/LO/L0/ :)

// If true, try to do compaction to compact smaller files into larger ones.
// Minimum files to compact follows options.level0_file_num_compaction_trigger
// and compaction won't trigger if average compact bytes per del file is
// larger than options.write_buffer_size * 2. This is to protect large files
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the "* 2" is no longer the case

Summary: Allow an option for users to do some compaction in FIFO compaction, to pay some write amplification for fewer number of files.

Test Plan: Add a unit test
@facebook-github-bot
Copy link
Contributor

@siying updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

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

siying added a commit that referenced this pull request May 5, 2017
Summary:
Allow an option for users to do some compaction in FIFO compaction, to pay some write amplification for fewer number of files.
Closes #2163

Differential Revision: D4895953

Pulled By: siying

fbshipit-source-id: a1ab608dd0627211f3e1f588a2e97159646e1231
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.

3 participants