Skip to content

Commit fe4efdd

Browse files
gavinchougavinchou
authored
[fix](cloud) Fix possible incorrect merged tablet stats while iterating detached tablet stats (apache#40494)
Previous impl. of `get_detached_tablet_stats()` may miss some detached KVs or tablet stats due to KV iterating paging (`RangeGetIterator.more() == true`), which leads to zero detached stats and produce buggy data size report to FE.
1 parent f9a5c92 commit fe4efdd

5 files changed

Lines changed: 565 additions & 452 deletions

File tree

‎cloud/src/meta-service/http_encode_key.cpp‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -135,32 +135,50 @@ static std::string parse_tablet_schema(const ValueBuf& buf) {
135135

136136
static std::string parse_tablet_stats(const ValueBuf& buf) {
137137
if (buf.iters.empty()) {
138-
return "";
138+
return "stats_kvs not found\n";
139139
}
140140

141-
TabletStatsPB stats;
142-
auto&& it = buf.iters[0];
143-
if (!it->has_next()) {
144-
return "";
141+
std::vector<std::pair<std::string, std::string>> stats_kvs;
142+
stats_kvs.reserve(5);
143+
for (auto& i : buf.iters) {
144+
while (i->has_next()) {
145+
auto [k, v] = i->next();
146+
stats_kvs.emplace_back(std::string {k.data(), k.size()},
147+
std::string {v.data(), v.size()});
148+
}
149+
}
150+
151+
if (stats_kvs.empty()) {
152+
return "stats_kvs not found\n";
145153
}
146154

147-
auto [k, v] = it->next();
155+
TabletStatsPB stats;
156+
auto [k, v] = stats_kvs[0];
148157
stats.ParseFromArray(v.data(), v.size());
149158

159+
std::string json;
160+
json += "aggregated_stats: " + proto_to_json(stats) + "\n";
161+
150162
// Parse split tablet stats
151163
TabletStats detached_stats;
152-
int ret = get_detached_tablet_stats(*it, detached_stats);
164+
int ret = get_detached_tablet_stats(stats_kvs, detached_stats);
153165
if (ret != 0) {
154-
return "";
166+
json += "failed to get detached_stats, ret=" + std::to_string(ret) + "\n";
167+
return json;
155168
}
169+
TabletStatsPB detached_stats_pb;
170+
merge_tablet_stats(detached_stats_pb, detached_stats); // convert to pb
171+
json += "detached_stats: " + proto_to_json(detached_stats_pb) + "\n";
156172

157173
merge_tablet_stats(stats, detached_stats);
158174

159-
return proto_to_json(stats);
175+
json += "merged_stats: " + proto_to_json(stats) + "\n";
176+
return json;
160177
}
161178

162179
// See keys.h to get all types of key, e.g: MetaRowsetKeyInfo
163-
// key_type -> {{param1, param2 ...}, encoding_func, value_parsing_func}
180+
// key_type -> {{param1, param2 ...}, key_encoding_func, value_parsing_func}
181+
// where params are the input for key_encoding_func
164182
// clang-format off
165183
static std::unordered_map<std::string_view,
166184
std::tuple<std::vector<std::string_view>, std::function<std::string(param_type&)>, std::function<std::string(const ValueBuf&)>>> param_set {
@@ -250,11 +268,17 @@ HttpResponse process_http_get_value(TxnKv* txn_kv, const brpc::URI& uri) {
250268
ValueBuf value;
251269
if (key_type == "StatsTabletKey") {
252270
// FIXME(plat1ko): hard code
253-
std::string end_key {key};
254-
encode_bytes("\xff", &end_key);
271+
std::string begin_key {key};
272+
std::string end_key = key + "\xff";
255273
std::unique_ptr<RangeGetIterator> it;
256-
err = txn->get(key, end_key, &it, true);
257-
value.iters.push_back(std::move(it));
274+
bool more = false;
275+
do {
276+
err = txn->get(begin_key, end_key, &it, true);
277+
if (err != TxnErrorCode::TXN_OK) break;
278+
begin_key = it->next_begin_key();
279+
more = it->more();
280+
value.iters.push_back(std::move(it));
281+
} while (more);
258282
} else {
259283
err = cloud::get(txn.get(), key, &value, true);
260284
}

‎cloud/src/meta-service/meta_service_job.cpp‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,16 +733,33 @@ void process_compaction_job(MetaServiceCode& code, std::string& msg, std::string
733733
}
734734
auto stats_key = stats_tablet_key({instance_id, table_id, index_id, partition_id, tablet_id});
735735
auto stats_val = stats->SerializeAsString();
736+
737+
VLOG_DEBUG << "data size, tablet_id=" << tablet_id << " stats.num_rows=" << stats->num_rows()
738+
<< " stats.data_size=" << stats->data_size()
739+
<< " stats.num_rowsets=" << stats->num_rowsets()
740+
<< " stats.num_segments=" << stats->num_segments()
741+
<< " detached_stats.num_rows=" << detached_stats.num_rows
742+
<< " detached_stats.data_size=" << detached_stats.data_size
743+
<< " detached_stats.num_rowset=" << detached_stats.num_rowsets
744+
<< " detached_stats.num_segments=" << detached_stats.num_segs
745+
<< " compaction.size_output_rowsets=" << compaction.size_output_rowsets()
746+
<< " compaction.size_input_rowsets=" << compaction.size_input_rowsets();
736747
txn->put(stats_key, stats_val);
737-
merge_tablet_stats(*stats, detached_stats);
748+
merge_tablet_stats(*stats, detached_stats); // this is to check
738749
if (stats->data_size() < 0 || stats->num_rowsets() < 1) [[unlikely]] {
739750
INSTANCE_LOG(ERROR) << "buggy data size, tablet_id=" << tablet_id
751+
<< " stats.num_rows=" << stats->num_rows()
740752
<< " stats.data_size=" << stats->data_size()
753+
<< " stats.num_rowsets=" << stats->num_rowsets()
754+
<< " stats.num_segments=" << stats->num_segments()
755+
<< " detached_stats.num_rows=" << detached_stats.num_rows
756+
<< " detached_stats.data_size=" << detached_stats.data_size
757+
<< " detached_stats.num_rowset=" << detached_stats.num_rowsets
758+
<< " detached_stats.num_segments=" << detached_stats.num_segs
741759
<< " compaction.size_output_rowsets="
742760
<< compaction.size_output_rowsets()
743-
<< " compaction.size_input_rowsets= "
744-
<< compaction.size_input_rowsets();
745-
DCHECK(false) << "buggy data size";
761+
<< " compaction.size_input_rowsets=" << compaction.size_input_rowsets();
762+
DCHECK(false) << "buggy data size, tablet_id=" << tablet_id;
746763
}
747764

748765
VLOG_DEBUG << "update tablet stats tablet_id=" << tablet_id << " key=" << hex(stats_key)

‎cloud/src/meta-service/meta_service_tablet_stats.cpp‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,49 +30,63 @@ namespace doris::cloud {
3030
void internal_get_tablet_stats(MetaServiceCode& code, std::string& msg, Transaction* txn,
3131
const std::string& instance_id, const TabletIndexPB& idx,
3232
TabletStatsPB& stats, TabletStats& detached_stats, bool snapshot) {
33-
auto begin_key = stats_tablet_key(
34-
{instance_id, idx.table_id(), idx.index_id(), idx.partition_id(), idx.tablet_id()});
35-
auto end_key = stats_tablet_key(
36-
{instance_id, idx.table_id(), idx.index_id(), idx.partition_id(), idx.tablet_id() + 1});
33+
// clang-format off
34+
auto begin_key = stats_tablet_key({instance_id, idx.table_id(), idx.index_id(), idx.partition_id(), idx.tablet_id()});
35+
auto begin_key_check = begin_key;
36+
auto end_key = stats_tablet_key({instance_id, idx.table_id(), idx.index_id(), idx.partition_id(), idx.tablet_id() + 1});
37+
// clang-format on
38+
std::vector<std::pair<std::string, std::string>> stats_kvs;
39+
stats_kvs.reserve(5); // aggregate + data_size + num_rows + num_rowsets + num_segments
40+
3741
std::unique_ptr<RangeGetIterator> it;
38-
TxnErrorCode err = txn->get(begin_key, end_key, &it, snapshot);
39-
if (err != TxnErrorCode::TXN_OK) {
40-
code = cast_as<ErrCategory::READ>(err);
41-
msg = fmt::format("failed to get tablet stats, err={} tablet_id={}", err, idx.tablet_id());
42-
return;
43-
}
44-
if (!it->has_next()) {
42+
do {
43+
TxnErrorCode err = txn->get(begin_key, end_key, &it, snapshot);
44+
if (err != TxnErrorCode::TXN_OK) {
45+
code = cast_as<ErrCategory::READ>(err);
46+
msg = fmt::format("failed to get tablet stats, err={} tablet_id={}", err,
47+
idx.tablet_id());
48+
return;
49+
}
50+
while (it->has_next()) {
51+
auto [k, v] = it->next();
52+
stats_kvs.emplace_back(std::string {k.data(), k.size()},
53+
std::string {v.data(), v.size()});
54+
}
55+
begin_key = it->next_begin_key();
56+
} while (it->more());
57+
58+
if (stats_kvs.empty()) {
4559
code = MetaServiceCode::TABLET_NOT_FOUND;
4660
msg = fmt::format("tablet stats not found, tablet_id={}", idx.tablet_id());
4761
return;
4862
}
49-
auto [k, v] = it->next();
50-
// First key MUST be tablet stats key
51-
DCHECK(k == begin_key) << hex(k) << " vs " << hex(begin_key);
63+
64+
auto& [first_stats_key, v] = stats_kvs[0];
65+
// First key MUST be tablet stats key, the original non-detached one
66+
DCHECK(first_stats_key == begin_key_check)
67+
<< hex(first_stats_key) << " vs " << hex(begin_key_check);
5268
if (!stats.ParseFromArray(v.data(), v.size())) {
5369
code = MetaServiceCode::PROTOBUF_PARSE_ERR;
54-
msg = fmt::format("marformed tablet stats value, key={}", hex(k));
70+
msg = fmt::format("marformed tablet stats value, key={}", hex(first_stats_key));
5571
return;
5672
}
5773
// Parse split tablet stats
58-
int ret = get_detached_tablet_stats(*it, detached_stats);
74+
int ret = get_detached_tablet_stats(stats_kvs, detached_stats);
75+
5976
if (ret != 0) {
6077
code = MetaServiceCode::PROTOBUF_PARSE_ERR;
61-
msg = fmt::format("marformed splitted tablet stats kv, key={}", hex(k));
78+
msg = fmt::format("marformed splitted tablet stats kv, key={}", hex(first_stats_key));
6279
return;
6380
}
6481
}
6582

66-
int get_detached_tablet_stats(RangeGetIterator& iter, TabletStats& detached_stats) {
67-
while (iter.has_next()) {
68-
auto [k, v] = iter.next();
69-
int64_t val;
70-
if (v.size() != sizeof(val)) [[unlikely]] {
71-
LOG(WARNING) << "malformed tablet stats value. key=" << hex(k);
72-
return -1;
73-
}
74-
75-
// 0x01 "stats" ${instance_id} "tablet" ${table_id} ${index_id} ${partition_id} ${tablet_id} "data_size"
83+
int get_detached_tablet_stats(const std::vector<std::pair<std::string, std::string>>& stats_kvs,
84+
TabletStats& detached_stats) {
85+
if (stats_kvs.size() != 5 && stats_kvs.size() != 1) {
86+
LOG(WARNING) << "incorrect tablet stats_kvs, it should be 1 or 5 size=" << stats_kvs.size();
87+
}
88+
for (size_t i = 1; i < stats_kvs.size(); ++i) {
89+
std::string_view k(stats_kvs[i].first), v(stats_kvs[i].second);
7690
k.remove_prefix(1);
7791
constexpr size_t key_parts = 8;
7892
std::vector<std::tuple<std::variant<int64_t, std::string>, int, int>> out;
@@ -84,9 +98,14 @@ int get_detached_tablet_stats(RangeGetIterator& iter, TabletStats& detached_stat
8498
auto* suffix = std::get_if<std::string>(&std::get<0>(out.back()));
8599
if (!suffix) [[unlikely]] {
86100
LOG(WARNING) << "malformed tablet stats key. key=" << hex(k);
87-
return -1;
101+
return -2;
88102
}
89103

104+
int64_t val = 0;
105+
if (v.size() != sizeof(val)) [[unlikely]] {
106+
LOG(WARNING) << "malformed tablet stats value v.size=" << v.size() << " key=" << hex(k);
107+
return -3;
108+
}
90109
std::memcpy(&val, v.data(), sizeof(val));
91110
if constexpr (std::endian::native == std::endian::big) {
92111
val = bswap_64(val);

‎cloud/src/meta-service/meta_service_tablet_stats.h‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,23 @@ void internal_get_tablet_stats(MetaServiceCode& code, std::string& msg, Transact
4747
const std::string& instance_id, const TabletIndexPB& idx,
4848
TabletStatsPB& stats, bool snapshot = false);
4949

50-
// Get detached tablet stats via `iter`, `iter.next` SHOULD be the first splitted tablet stats KV.
51-
// Return 0 if success, otherwise error.
52-
[[nodiscard]] int get_detached_tablet_stats(RangeGetIterator& iter, TabletStats& detached_stats);
50+
// clang-format off
51+
/**
52+
* Get detached tablet stats via with given stats_kvs
53+
*
54+
* stats_kvs stores the following KVs, see keys.h for more details
55+
* 0x01 "stats" ${instance_id} "tablet" ${table_id} ${index_id} ${partition_id} ${tablet_id} -> TabletStatsPB
56+
* 0x01 "stats" ${instance_id} "tablet" ${table_id} ${index_id} ${partition_id} ${tablet_id} "data_size" -> int64
57+
* 0x01 "stats" ${instance_id} "tablet" ${table_id} ${index_id} ${partition_id} ${tablet_id} "num_rows" -> int64
58+
* 0x01 "stats" ${instance_id} "tablet" ${table_id} ${index_id} ${partition_id} ${tablet_id} "num_rowsets" -> int64
59+
* 0x01 "stats" ${instance_id} "tablet" ${table_id} ${index_id} ${partition_id} ${tablet_id} "num_segments" -> int64
60+
*
61+
* @param stats_kvs the tablet stats kvs to process, it is in size of 5 or 1
62+
* @param detached_stats output param for the detached stats
63+
* @return 0 for success otherwise error
64+
*/
65+
[[nodiscard]] int get_detached_tablet_stats(const std::vector<std::pair<std::string, std::string>>& stats_kvs,
66+
TabletStats& detached_stats);
67+
// clang-format on
5368

5469
} // namespace doris::cloud

0 commit comments

Comments
 (0)