Delete files in multiple ranges at once #3431

huachaohuang · 2018-01-28T15:51:36Z

Using DeleteFilesInRange to delete files in a lot of ranges can be slow, because
VersionSet::LogAndApply is expensive.

This PR adds a new DeleteFilesInRange function to delete files in multiple
ranges at once.

Close #2951

ajkr

this is a good change, thanks :). couple more questions:

do you mind updating HISTORY.md to mention this new public API?
do you have any numbers to share about how long deleting files in ranges took before/after this change?

ajkr · 2018-01-29T20:56:12Z

include/rocksdb/db.h

@@ -100,6 +100,14 @@ struct Range {
  Range(const Slice& s, const Slice& l) : start(s), limit(l) { }
 };

+struct RangePtr {
+  const Slice* start;          // Included in the range
+  const Slice* limit;          // Not included in the range


my understanding of GetCleanInputsWithinInterval is that it'll include the upper endpoint -- do you mind double checking?

My mistake, I just copy it from struct Range. But we are actually intent on supporting exclusive range end in DeleteFilesInRange/DeleteFilesInRanges. Do you think it is OK to add that support in this PR, and make the range end exclusive by default in the new API DeleteFilesInRanges?

Add support in this PR sounds fine. But I think it'd be more consistent to make it inclusive-inclusive, with an option to make the limit exclusive. This PR (#2863) did a good job of adding the option for DeleteFilesInRange, but unfortunately we didn't merge it due to an unsolved test failure.

Let's delete // Not included in the range since it's optional

ajkr · 2018-01-29T21:02:43Z

include/rocksdb/convenience.h

@@ -330,6 +330,8 @@ void CancelAllBackgroundWork(DB* db, bool wait = false);
 // Snapshots before the delete might not see the data in the given range.
 Status DeleteFilesInRange(DB* db, ColumnFamilyHandle* column_family,
                          const Slice* begin, const Slice* end);
+Status DeleteFilesInRange(DB* db, ColumnFamilyHandle* column_family,


can we call the new API DeleteFilesInRanges?

Sounds good.

huachaohuang

I'll share some numbers later :)

huachaohuang · 2018-01-30T02:27:41Z

include/rocksdb/convenience.h

@@ -330,6 +330,8 @@ void CancelAllBackgroundWork(DB* db, bool wait = false);
 // Snapshots before the delete might not see the data in the given range.
 Status DeleteFilesInRange(DB* db, ColumnFamilyHandle* column_family,
                          const Slice* begin, const Slice* end);
+Status DeleteFilesInRange(DB* db, ColumnFamilyHandle* column_family,


Sounds good.

huachaohuang · 2018-01-30T02:33:14Z

include/rocksdb/db.h

@@ -100,6 +100,14 @@ struct Range {
  Range(const Slice& s, const Slice& l) : start(s), limit(l) { }
 };

+struct RangePtr {
+  const Slice* start;          // Included in the range
+  const Slice* limit;          // Not included in the range


My mistake, I just copy it from struct Range. But we are actually intent on supporting exclusive range end in DeleteFilesInRange/DeleteFilesInRanges. Do you think it is OK to add that support in this PR, and make the range end exclusive by default in the new API DeleteFilesInRanges?

Using DeleteFilesInRange to delete files in a lot of ranges can be slow, because VersionSet::LogAndApply is expensive. This PR adds a new DeleteFilesInRange function to delete files in multiple ranges at once.

facebook-github-bot · 2018-01-30T06:33:36Z

@huachaohuang has updated the pull request.

facebook-github-bot · 2018-01-30T06:56:52Z

@huachaohuang has updated the pull request. View: changes

huachaohuang · 2018-01-30T06:57:19Z

Calling DeleteFilesInRange 7356 times which actually deletes 4530 files in 2117 ranges takes Duration { secs: 126, nanos: 856424397 }
Calling DeleteFilesInRanges with 8421 ranges at once which actually deletes 4448 files takes Duration { secs: 2, nanos: 467631117 }
This is a rough comparison, but I think it should be enough to show the benefit to deleting files in multiple ranges at once.

facebook-github-bot

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

ajkr

LGTM, thanks!

ajkr · 2018-01-30T21:16:53Z

include/rocksdb/db.h

@@ -100,6 +100,14 @@ struct Range {
  Range(const Slice& s, const Slice& l) : start(s), limit(l) { }
 };

+struct RangePtr {
+  const Slice* start;          // Included in the range
+  const Slice* limit;          // Not included in the range


Let's delete // Not included in the range since it's optional

remove comments about range inclusion vs exclusion since it should be specified in the APIs.

facebook-github-bot · 2018-01-30T21:31:20Z

@huachaohuang has updated the pull request. View: changes, changes since last import

facebook-github-bot

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

Summary: Using `DeleteFilesInRange` to delete files in a lot of ranges can be slow, because `VersionSet::LogAndApply` is expensive. This PR adds a new `DeleteFilesInRange` function to delete files in multiple ranges at once. Close facebook#2951 Closes facebook#3431 Differential Revision: D6849228 Pulled By: ajkr fbshipit-source-id: daeedcabd8def4b1d9ee95a58266dee77b5d68cb

facebook-github-bot added the CLA Signed label Jan 28, 2018

ajkr self-assigned this Jan 29, 2018

ajkr requested changes Jan 29, 2018

View reviewed changes

huachaohuang commented Jan 30, 2018

View reviewed changes

huachaohuang added 2 commits January 30, 2018 14:32

Delete files in multiple ranges at once

6c83e58

Using DeleteFilesInRange to delete files in a lot of ranges can be slow, because VersionSet::LogAndApply is expensive. This PR adds a new DeleteFilesInRange function to delete files in multiple ranges at once.

Add include_end option to DeleteFilesInRange/DeleteFilesInRanges

589b8d8

Fix comparison between signed and unsigned integer expressions

3bf5904

facebook-github-bot reviewed Jan 30, 2018

View reviewed changes

ajkr approved these changes Jan 30, 2018

View reviewed changes

Update db.h

79cdd42

remove comments about range inclusion vs exclusion since it should be specified in the APIs.

facebook-github-bot reviewed Jan 30, 2018

View reviewed changes

facebook-github-bot closed this in ab43ff5 Jan 30, 2018

huachaohuang mentioned this pull request Jan 31, 2018

[cherry-pick] Delete files in multiple ranges at once tikv/rocksdb#13

Merged

huachaohuang deleted the delete-files-in-multi-ranges branch February 2, 2018 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete files in multiple ranges at once #3431

Delete files in multiple ranges at once #3431

huachaohuang commented Jan 28, 2018

ajkr left a comment

ajkr Jan 29, 2018

huachaohuang Jan 30, 2018

ajkr Jan 30, 2018

ajkr Jan 30, 2018

ajkr Jan 29, 2018

huachaohuang Jan 30, 2018

huachaohuang left a comment

huachaohuang Jan 30, 2018

huachaohuang Jan 30, 2018

facebook-github-bot commented Jan 30, 2018

facebook-github-bot commented Jan 30, 2018

huachaohuang commented Jan 30, 2018

facebook-github-bot left a comment

ajkr left a comment

ajkr Jan 30, 2018

facebook-github-bot commented Jan 30, 2018

facebook-github-bot left a comment

Delete files in multiple ranges at once #3431

Delete files in multiple ranges at once #3431

Conversation

huachaohuang commented Jan 28, 2018

ajkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huachaohuang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 30, 2018

facebook-github-bot commented Jan 30, 2018

huachaohuang commented Jan 30, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

ajkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 30, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment