Skip to content

Commit c7d06f3

Browse files
Hui Xiaofacebook-github-bot
authored andcommitted
Size-tolerant deserialization for compaction service response's counts array (#14807)
Summary: The "counts" array in CompactionServiceResult (one compaction count per CompactionReason) is non-critical information. During cross-process remote compaction the primary and the remote worker can run different RocksDB versions and therefore disagree on kNumOfReasons, so the serialized counts array can have a different length than the reader expects. The plain OptionTypeInfo::Array parser rejects any element-count mismatch, which fails deserialization of the entire result over this non-critical field. This makes deserialization of "counts" size-tolerant via SizeTolerantDeserializeArray, which overrides only the parse callback: extra trailing elements are dropped and missing trailing elements are zero-filled, so a length mismatch no longer fails deserialization. Serialization and comparison are inherited from Array<T, kSize> unchanged. The serialized width is intentionally kept at kNumOfReasons - 1 here, so this change does not alter what is written -- only the read path becomes tolerant. Widening the serialized width to the full kNumOfReasons is done as a separate follow-up change, which must not land until this tolerant read has been deployed to every primary (including rollback targets), so that no reader still rejects a wider result. == Test plan == **Legend** CompactionReason is an enum whose last entry is kNumOfReasons (currently 21). CompactionStats::counts is an int[kNumOfReasons] holding one count per reason; it is serialized into the remote compaction result and read back by the primary. - "narrow" = kNumOfReasons - 1 counts (omits the newest reason); what current code writes. - "wide" = kNumOfReasons counts (includes the newest reason). - strict reader: accepts only its exact expected element count; any other count fails deserialization (the plain OptionTypeInfo::Array behavior). - tolerant reader: accepts any count -- more elements than expected are dropped, fewer are zero-filled; never fails on a count mismatch (this change, SizeTolerantDeserializeArray). **Unit test** CompactionServiceTest.CompatCheckCountsWidthTolerance -- verifies that deserializing "counts" tolerates a serialized element count equal to, one greater than, or one less than the reader's expected width (extras dropped, missing zero-filled), and that the binary serializes exactly the expected number of elements. **Manual cross-version verification (ldb remote_compaction)** Test 1: baseline (unchanged behavior) -- expect-narrow strict-read primary reads narrow counts written by a worker ``` $ strict_narrow remote_compaction_worker --db=$DB --job_dir=$JOB & $ strict_narrow remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4913 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Test 2: expect-narrow tolerant-read primary reads the narrow counts a worker writes ``` $ strict_narrow remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_narrow remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4912 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Test 3: expect-narrow tolerant-read primary reads the wide counts a worker writes (extra reason dropped) ``` $ tolerant_wide remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_narrow remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4915 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Test 4: expect-wide, tolerant primary reads the narrow counts a worker writes (missing reason zero-filled) ``` $ strict_narrow remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_wide remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4911 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Test 5: expect-wide, tolerant primary reads a wide counts a worker write ``` $ tolerant_wide remote_compaction_worker --db=$DB --job_dir=$JOB & $ tolerant_wide remote_compaction_primary --db=$DB --job_dir=$JOB remote_compaction_worker: OK (4917 bytes result) remote_compaction_primary: OK $ grep 'remote compaction result' $DB/LOG [default] [JOB 3] Received remote compaction result, output path: /tmp/cs_run/output, files: 000022.sst ``` => PASS: the primary parsed and installed the remote result. Differential Revision: D106321320
1 parent c3f6e4d commit c7d06f3

2 files changed

Lines changed: 176 additions & 5 deletions

File tree

db/compaction/compaction_service_job.cc

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,40 @@ static std::unordered_map<std::string, OptionTypeInfo>
777777
OptionTypeFlags::kNone}},
778778
};
779779

780+
namespace {
781+
782+
// Like OptionTypeInfo::Array<T, kSize> but deserialization tolerates
783+
// size mismatches: extra elements beyond kSize are silently ignored,
784+
// missing elements are zero-filled. Serialization and comparison are
785+
// inherited from Array<T, kSize> unchanged; only the parse
786+
// (deserialize) callback is overridden.
787+
template <typename T, size_t kSize>
788+
OptionTypeInfo SizeTolerantDeserializeArray(int offset,
789+
OptionVerificationType verification,
790+
OptionTypeFlags flags,
791+
const OptionTypeInfo& elem_info,
792+
char separator = ':') {
793+
return OptionTypeInfo::Array<T, kSize>(offset, verification, flags, elem_info,
794+
separator)
795+
.SetParseFunc([elem_info, separator](
796+
const ConfigOptions& opts, const std::string& name,
797+
const std::string& value, void* addr) {
798+
std::vector<T> parsed;
799+
Status s =
800+
ParseVector<T>(opts, elem_info, separator, name, value, &parsed);
801+
if (!s.ok()) {
802+
return Status(std::move(s));
803+
}
804+
auto* arr = static_cast<std::array<T, kSize>*>(addr);
805+
for (size_t i = 0; i < kSize; ++i) {
806+
(*arr)[i] = (i < parsed.size()) ? parsed[i] : T{};
807+
}
808+
return Status::OK();
809+
});
810+
}
811+
812+
} // namespace
813+
780814
static std::unordered_map<std::string, OptionTypeInfo>
781815
compaction_stats_type_info = {
782816
{"micros",
@@ -868,12 +902,20 @@ static std::unordered_map<std::string, OptionTypeInfo>
868902
{offsetof(struct InternalStats::CompactionStats, count),
869903
OptionType::kUInt64T, OptionVerificationType::kNormal,
870904
OptionTypeFlags::kNone}},
905+
// The remote worker may serialize a different number of
906+
// CompactionReason counts than this primary expects (RocksDB versions
907+
// can disagree on kNumOfReasons). Deserialization tolerates the
908+
// mismatch: extra trailing elements are dropped, missing ones are
909+
// zero-filled.
910+
//
911+
// The serialized width is intentionally kept at kNumOfReasons - 1 (the
912+
// reduced width) so this change does NOT alter what is written -- only
913+
// the read path becomes tolerant. Widening the serialized width to the
914+
// full kNumOfReasons is a separate, later change that must not land
915+
// until this tolerant read has been deployed to every primary,
916+
// including rollback targets.
871917
{"counts",
872-
OptionTypeInfo::Array<
873-
/* In release 11.2, a new compaction reason was added. This broken
874-
* the reader and writer. To unblock release 11.1, we temporarily
875-
* reduce the count array size to the old one. TODO add a proper
876-
* serialization and deserialization method. */
918+
SizeTolerantDeserializeArray<
877919
int, static_cast<int>(CompactionReason::kNumOfReasons) - 1>(
878920
offsetof(struct InternalStats::CompactionStats, counts),
879921
OptionVerificationType::kNormal, OptionTypeFlags::kNone,

db/compaction/compaction_service_test.cc

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// COPYING file in the root directory) and Apache 2.0 License
44
// (found in the LICENSE.Apache file in the root directory).
55

6+
#include <memory>
7+
68
#include "db/db_test_util.h"
79
#include "file/file_util.h"
810
#include "port/stack_trace.h"
@@ -12,6 +14,71 @@
1214

1315
namespace ROCKSDB_NAMESPACE {
1416

17+
namespace {
18+
19+
// Append ":extra_value" to every "counts=...;" field in a serialized
20+
// CompactionServiceResult string to simulate a worker with more
21+
// CompactionReason enums than the reader. Loops because the serialized
22+
// result contains multiple CompactionStats structs (output_level_stats,
23+
// proximal_level_stats), each with its own "counts=" field.
24+
void AppendToCountsFields(std::string* s, const std::string& extra_value) {
25+
const std::string prefix = "counts=";
26+
size_t pos = s->find(prefix);
27+
while (pos != std::string::npos) {
28+
// Find the semicolon that terminates this field's value.
29+
size_t semi = s->find(';', pos + prefix.size());
30+
assert(semi != std::string::npos);
31+
// Insert ":extra_value" just before the semicolon.
32+
s->insert(semi, ":" + extra_value);
33+
// Advance past the inserted text to find the next occurrence.
34+
pos = s->find(prefix, semi + extra_value.size() + 2);
35+
}
36+
}
37+
38+
// Remove the last colon-separated element from every "counts=...;" field
39+
// to simulate a worker with fewer CompactionReason enums than the reader.
40+
// Loops for the same reason as AppendToCountsFields (multiple counts fields).
41+
void RemoveLastFromCountsFields(std::string* s) {
42+
const std::string prefix = "counts=";
43+
size_t pos = s->find(prefix);
44+
while (pos != std::string::npos) {
45+
size_t val_begin = pos + prefix.size();
46+
size_t semi = s->find(';', val_begin);
47+
assert(semi != std::string::npos);
48+
// Find the last ':' separator within the value.
49+
size_t last_sep = s->rfind(':', semi - 1);
50+
assert(last_sep != std::string::npos && last_sep >= val_begin);
51+
// Erase from the last ':' up to (not including) the ';'.
52+
// e.g. "counts=0:3:5;" -> erase ":5" -> "counts=0:3;"
53+
s->erase(last_sep, semi - last_sep);
54+
pos = s->find(prefix, last_sep);
55+
}
56+
}
57+
58+
// Count the colon-separated elements in the first "counts=...;" field of a
59+
// serialized CompactionServiceResult. Used to assert the serialized width
60+
// (how many CompactionReason counts this binary writes).
61+
size_t CountFirstCountsFieldElements(const std::string& s) {
62+
const std::string prefix = "counts=";
63+
size_t pos = s.find(prefix);
64+
assert(pos != std::string::npos);
65+
size_t val_begin = pos + prefix.size();
66+
size_t semi = s.find(';', val_begin);
67+
assert(semi != std::string::npos);
68+
if (semi == val_begin) {
69+
return 0;
70+
}
71+
size_t count = 1;
72+
for (size_t i = val_begin; i < semi; ++i) {
73+
if (s[i] == ':') {
74+
++count;
75+
}
76+
}
77+
return count;
78+
}
79+
80+
} // namespace
81+
1582
class MyTestCompactionService : public CompactionService {
1683
public:
1784
MyTestCompactionService(
@@ -1609,6 +1676,68 @@ TEST_F(CompactionServiceTest, InvalidResultFallsBackToLocal) {
16091676
my_cs->GetFinalCompactionServiceJobStatus());
16101677
}
16111678

1679+
TEST_F(CompactionServiceTest, CompatCheckCountsWidthTolerance) {
1680+
// Number of per-CompactionReason counts this binary serializes and expects.
1681+
constexpr size_t kExpected =
1682+
static_cast<size_t>(CompactionReason::kNumOfReasons) - 1;
1683+
constexpr size_t kLastIndex = kExpected - 1;
1684+
constexpr int kFirstCount = 3;
1685+
constexpr int kLastCount = 5;
1686+
1687+
CompactionServiceResult result;
1688+
result.status = Status::OK();
1689+
result.output_level = 2;
1690+
result.output_path = "/tmp/output";
1691+
result.internal_stats.output_level_stats.counts[0] = kFirstCount;
1692+
result.internal_stats.output_level_stats.counts[kLastIndex] = kLastCount;
1693+
std::string serialized;
1694+
ASSERT_OK(result.Write(&serialized));
1695+
1696+
// Guard the produced format: this binary writes exactly kExpected counts.
1697+
// That element count is what a remote worker hands to the primary to read,
1698+
// so pinning it catches an accidental change to what is written.
1699+
ASSERT_EQ(CountFirstCountsFieldElements(serialized), kExpected);
1700+
1701+
// Peer agrees on kNumOfReasons: every written count is read back unchanged.
1702+
{
1703+
CompactionServiceResult parsed;
1704+
ASSERT_OK(CompactionServiceResult::Read(serialized, &parsed));
1705+
ASSERT_OK(parsed.status);
1706+
const auto& counts = parsed.internal_stats.output_level_stats.counts;
1707+
ASSERT_EQ(counts[0], kFirstCount);
1708+
ASSERT_EQ(counts[kLastIndex], kLastCount);
1709+
}
1710+
1711+
// Peer serialized one more count than this binary knows (a worker with an
1712+
// extra CompactionReason): the surplus trailing count is ignored, and the
1713+
// counts this binary understands are read unchanged.
1714+
{
1715+
std::string from_wider_peer = serialized;
1716+
AppendToCountsFields(&from_wider_peer, "7");
1717+
ASSERT_EQ(CountFirstCountsFieldElements(from_wider_peer), kExpected + 1);
1718+
CompactionServiceResult parsed;
1719+
ASSERT_OK(CompactionServiceResult::Read(from_wider_peer, &parsed));
1720+
ASSERT_OK(parsed.status);
1721+
const auto& counts = parsed.internal_stats.output_level_stats.counts;
1722+
ASSERT_EQ(counts[0], kFirstCount);
1723+
ASSERT_EQ(counts[kLastIndex], kLastCount);
1724+
}
1725+
1726+
// Peer serialized one fewer count than this binary knows (a worker missing
1727+
// the newest CompactionReason): the absent trailing count reads as zero.
1728+
{
1729+
std::string from_narrower_peer = serialized;
1730+
RemoveLastFromCountsFields(&from_narrower_peer);
1731+
ASSERT_EQ(CountFirstCountsFieldElements(from_narrower_peer), kExpected - 1);
1732+
CompactionServiceResult parsed;
1733+
ASSERT_OK(CompactionServiceResult::Read(from_narrower_peer, &parsed));
1734+
ASSERT_OK(parsed.status);
1735+
const auto& counts = parsed.internal_stats.output_level_stats.counts;
1736+
ASSERT_EQ(counts[0], kFirstCount);
1737+
ASSERT_EQ(counts[kLastIndex], 0);
1738+
}
1739+
}
1740+
16121741
TEST_F(CompactionServiceTest, SubCompaction) {
16131742
Options options = CurrentOptions();
16141743
options.max_subcompactions = 10;

0 commit comments

Comments
 (0)