mirror of
https://github.com/facebook/rocksdb.git
synced 2026-07-07 14:47:40 +08:00
Persist tail size of remote compaction output file to manifest (#13522)
Summary: **Context/Summary:** This is to fix a bug that tail size of remote compaction output SST file is not persisted to manifest in primary instance. This prevent us from using direct tail prefetch optimization each time opening this SST file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13522 Test Plan: Modify existing UT that failed before the fix Reviewed By: anand1976 Differential Revision: D72479612 Pulled By: hx235 fbshipit-source-id: 1ba8aa66fac71b9196589f60076229c29a103706
This commit is contained in:
committed by
Facebook GitHub Bot
parent
4069afeede
commit
07b09c7548
@@ -240,7 +240,8 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService(
|
||||
meta.marked_for_compaction = file.marked_for_compaction;
|
||||
meta.unique_id = file.unique_id;
|
||||
meta.temperature = file.file_temperature;
|
||||
|
||||
meta.tail_size =
|
||||
FileMetaData::CalculateTailSize(file_size, file.table_properties);
|
||||
auto cfd = compaction->column_family_data();
|
||||
CompactionOutputs* compaction_outputs =
|
||||
sub_compact->Outputs(file.is_proximal_level_output);
|
||||
|
||||
@@ -277,8 +277,17 @@ TEST_F(CompactionServiceTest, BasicCompactions) {
|
||||
Statistics* primary_statistics = GetPrimaryStatistics();
|
||||
Statistics* compactor_statistics = GetCompactorStatistics();
|
||||
|
||||
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
|
||||
"BlockBasedTable::PrefetchTail::TaiSizeNotRecorded",
|
||||
[&](void* /* arg */) {
|
||||
// Trigger assertion to verify precise tail prefetch size calculation
|
||||
assert(false);
|
||||
});
|
||||
|
||||
SyncPoint::GetInstance()->EnableProcessing();
|
||||
GenerateTestData();
|
||||
ASSERT_OK(dbfull()->TEST_WaitForCompact());
|
||||
SyncPoint::GetInstance()->DisableProcessing();
|
||||
VerifyTestData();
|
||||
|
||||
auto my_cs = GetCompactionService();
|
||||
@@ -380,6 +389,7 @@ TEST_F(CompactionServiceTest, BasicCompactions) {
|
||||
ASSERT_FALSE(result.stats.is_full_compaction);
|
||||
|
||||
Close();
|
||||
SyncPoint::GetInstance()->DisableProcessing();
|
||||
}
|
||||
|
||||
TEST_F(CompactionServiceTest, ManualCompaction) {
|
||||
@@ -890,6 +900,12 @@ TEST_F(CompactionServiceTest, TruncatedOutput) {
|
||||
Slice end(end_str);
|
||||
uint64_t comp_num = my_cs->GetCompactionNum();
|
||||
|
||||
// Skip calculating tail size to avoid crashing due to truncated file size
|
||||
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
|
||||
"FileMetaData::CalculateTailSize", [&](void* arg) {
|
||||
bool* skip = static_cast<bool*>(arg);
|
||||
*skip = true;
|
||||
});
|
||||
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
|
||||
"CompactionServiceCompactionJob::Run:0", [&](void* arg) {
|
||||
CompactionServiceResult* compaction_result =
|
||||
@@ -906,7 +922,7 @@ TEST_F(CompactionServiceTest, TruncatedOutput) {
|
||||
ASSERT_OK(s);
|
||||
ASSERT_GT(file_size, 0);
|
||||
|
||||
ASSERT_OK(test::TruncateFile(env_, file_name, file_size / 2));
|
||||
ASSERT_OK(test::TruncateFile(env_, file_name, file_size / 4));
|
||||
}
|
||||
});
|
||||
SyncPoint::GetInstance()->EnableProcessing();
|
||||
|
||||
@@ -380,6 +380,33 @@ struct FileMetaData {
|
||||
assert(!res || fd.smallest_seqno == fd.largest_seqno);
|
||||
return res;
|
||||
}
|
||||
|
||||
static uint64_t CalculateTailSize(uint64_t file_size,
|
||||
const TableProperties& props) {
|
||||
#ifndef NDEBUG
|
||||
bool skip = false;
|
||||
TEST_SYNC_POINT_CALLBACK("FileMetaData::CalculateTailSize", &skip);
|
||||
if (skip) {
|
||||
return 0;
|
||||
}
|
||||
#endif // NDEBUG
|
||||
uint64_t tail_size = 0;
|
||||
|
||||
// Differentiate between a file with no data blocks (tail_start_offset = 0)
|
||||
// and a file with unknown tail_start_offset (also set to 0 due to
|
||||
// non-negative integer storage limitation)
|
||||
bool contain_no_data_blocks =
|
||||
props.num_entries == 0 ||
|
||||
(props.num_entries > 0 &&
|
||||
(props.num_entries == props.num_range_deletions));
|
||||
|
||||
if (props.tail_start_offset > 0 || contain_no_data_blocks) {
|
||||
assert(props.tail_start_offset <= file_size);
|
||||
tail_size = file_size - props.tail_start_offset;
|
||||
}
|
||||
|
||||
return tail_size;
|
||||
}
|
||||
};
|
||||
|
||||
// A compressed copy of file meta data that just contain minimum data needed
|
||||
|
||||
@@ -914,6 +914,7 @@ Status BlockBasedTable::PrefetchTail(
|
||||
"TailPrefetchStats.",
|
||||
file->file_name().c_str(), tail_prefetch_size);
|
||||
}
|
||||
TEST_SYNC_POINT("BlockBasedTable::PrefetchTail::TaiSizeNotRecorded");
|
||||
}
|
||||
size_t prefetch_off;
|
||||
size_t prefetch_len;
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
Fix a bug where tail size of remote compaction output is not persisted in primary db's manifest
|
||||
Reference in New Issue
Block a user