Skip to content

Commit

Permalink
Avoid shifting component too large error in FileTtlBooster (#11673)
Browse files Browse the repository at this point in the history
Summary:
When `num_levels` > 65, we may be shifting more than 63 bits in FileTtlBooster. This can give errors like: `runtime error: shift exponent 98 is too large for 64-bit type 'uint64_t' (aka 'unsigned long')`. This PR makes a quick fix for this issue by taking a min in the shifting component. This issue should be rare since it requires a user using a large `num_levels`. I'll follow up with a more complex fix if needed.

Pull Request resolved: #11673

Test Plan: * Add a unit test that produce the above error before this PR. Need to compile it with ubsan: `COMPILE_WITH_UBSAN=1 OPT="-fsanitize-blacklist=.circleci/ubsan_suppression_list.txt" ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 make V=1 -j32 compaction_picker_test`

Reviewed By: hx235

Differential Revision: D48074386

Pulled By: cbi42

fbshipit-source-id: 25e59df7e93f20e0793cffb941de70ac815d9392
  • Loading branch information
cbi42 authored and ajkr committed Aug 7, 2023
1 parent 393d2dd commit 6fd663a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 1 deletion.
9 changes: 9 additions & 0 deletions db/compaction/compaction_picker_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1968,6 +1968,15 @@ TEST_F(CompactionPickerTest, OverlappingUserKeys11) {
ASSERT_EQ(7U, compaction->input(1, 0)->fd.GetNumber());
}

TEST_F(CompactionPickerTest, FileTtlBoosterLargeNumLevels) {
const uint64_t kCurrentTime = 1000000;
FileTtlBooster booster(kCurrentTime, /*ttl=*/2048,
/*num_non_empty_levels=*/100, /*level=*/1);
FileMetaData meta;
meta.oldest_ancester_time = kCurrentTime - 1023;
ASSERT_EQ(1, booster.GetBoostScore(&meta));
}

TEST_F(CompactionPickerTest, FileTtlBooster) {
// Set TTL to 2048
// TTL boosting for all levels starts at 1024,
Expand Down
4 changes: 3 additions & 1 deletion db/compaction/file_pri.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ class FileTtlBooster {
enabled_ = true;
uint64_t all_boost_start_age = ttl / 2;
uint64_t all_boost_age_range = (ttl / 32) * 31 - all_boost_start_age;
// TODO(cbi): more reasonable algorithm that gives different values
// when num_non_empty_levels - level - 1 > 63.
uint64_t boost_age_range =
all_boost_age_range >> (num_non_empty_levels - level - 1);
all_boost_age_range >> std::min(63, num_non_empty_levels - level - 1);
boost_age_start_ = all_boost_start_age + boost_age_range;
const uint64_t kBoostRatio = 16;
// prevent 0 value to avoid divide 0 error.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug in FileTTLBooster that can cause users with a large number of levels (more than 65) to see errors like "runtime error: shift exponent .. is too large.." (#11673).

0 comments on commit 6fd663a

Please sign in to comment.