From 6fd663a22f199656e3cec051c44d60672e87650e Mon Sep 17 00:00:00 2001 From: Changyu Bi Date: Fri, 4 Aug 2023 14:29:50 -0700 Subject: [PATCH] Avoid shifting component too large error in FileTtlBooster (#11673) 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: https://github.com/facebook/rocksdb/pull/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 --- db/compaction/compaction_picker_test.cc | 9 +++++++++ db/compaction/file_pri.h | 4 +++- .../shifting_componeng_too_large_file_ttl_booster.md | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 unreleased_history/bug_fixes/shifting_componeng_too_large_file_ttl_booster.md diff --git a/db/compaction/compaction_picker_test.cc b/db/compaction/compaction_picker_test.cc index fd14322b202..6aec0384010 100644 --- a/db/compaction/compaction_picker_test.cc +++ b/db/compaction/compaction_picker_test.cc @@ -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, diff --git a/db/compaction/file_pri.h b/db/compaction/file_pri.h index 82dddcf9384..e60d73e88d4 100644 --- a/db/compaction/file_pri.h +++ b/db/compaction/file_pri.h @@ -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. diff --git a/unreleased_history/bug_fixes/shifting_componeng_too_large_file_ttl_booster.md b/unreleased_history/bug_fixes/shifting_componeng_too_large_file_ttl_booster.md new file mode 100644 index 00000000000..f768302328c --- /dev/null +++ b/unreleased_history/bug_fixes/shifting_componeng_too_large_file_ttl_booster.md @@ -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). \ No newline at end of file