Skip to content

Commit 2990689

Browse files
committed
MB-71428: Fix datatype validation and handling in stat collection
Refactor KVBucket::appendDatatypeStats to eliminate code duplication by extracting common logic into a lambda function. The method now validates each datatype value using cb::mcbp::datatype::is_valid() before processing, preventing invalid datatype indices from being included in statistics collection. Additionally, fix the datatypeValues() helper to filter invalid datatype values, ensuring only valid datatypes are included in test operations. Fix createWithFactory() to explicitly set the datatype on the newly created StoredValue, ensuring the datatype is properly initialized when the stored value is created with a specified type. These changes improve datatype handling consistency and prevent potential issues with invalid or uninitialized datatype values in statistics and test scenarios. Change-Id: Ief6b6c400b3a0867f33cfe45fe5d3c2ff46550bf Reviewed-on: https://review.couchbase.org/c/kv_engine/+/243364 Reviewed-by: Jim Walker <jim@couchbase.com> Tested-by: Build Bot <build@couchbase.com>
1 parent 652bdf5 commit 2990689

2 files changed

Lines changed: 16 additions & 14 deletions

File tree

engines/ep/src/kv_bucket.cc

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,21 +1525,22 @@ void KVBucket::appendDatatypeStats(const DatatypeStatVisitor& active,
15251525
const BucketStatCollector& collector) {
15261526
using namespace cb::stats;
15271527

1528-
for (uint8_t ii = 0; ii < active.getNumDatatypes(); ++ii) {
1529-
auto datatypeStr = cb::mcbp::datatype::to_string(ii);
1530-
auto labelled = collector.withLabels(
1531-
{{"datatype", datatypeStr}, {"vbucket_state", "active"}});
1532-
1533-
labelled.addStat(Key::datatype_count, active.getDatatypeCount(ii));
1534-
}
1528+
const auto addStats = [&collector](const DatatypeStatVisitor& visitor,
1529+
std::string_view state) {
1530+
for (uint8_t ii = 0; ii < visitor.getNumDatatypes(); ++ii) {
1531+
if (!cb::mcbp::datatype::is_valid(ii)) {
1532+
continue;
1533+
}
1534+
auto datatypeStr = cb::mcbp::datatype::to_string(ii);
1535+
auto labelled = collector.withLabels(
1536+
{{"datatype", datatypeStr}, {"vbucket_state", state}});
15351537

1536-
for (uint8_t ii = 0; ii < replica.getNumDatatypes(); ++ii) {
1537-
auto datatypeStr = cb::mcbp::datatype::to_string(ii);
1538-
auto labelled = collector.withLabels(
1539-
{{"datatype", datatypeStr}, {"vbucket_state", "replica"}});
1538+
labelled.addStat(Key::datatype_count, visitor.getDatatypeCount(ii));
1539+
}
1540+
};
15401541

1541-
labelled.addStat(Key::datatype_count, replica.getDatatypeCount(ii));
1542-
}
1542+
addStats(active, "active");
1543+
addStats(replica, "replica");
15431544
}
15441545

15451546
void KVBucket::completeBGFetchMulti(

engines/ep/tests/module_tests/test_helpers.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ std::vector<protocol_binary_datatype_t> datatypeValues(
484484
std::vector<protocol_binary_datatype_t> results;
485485
for (protocol_binary_datatype_t i = 0; i < cb::mcbp::datatype::highest;
486486
i++) {
487-
if ((i & datatype) == i) {
487+
if ((i & datatype) == i && cb::mcbp::datatype::is_valid(i)) {
488488
results.push_back(i);
489489
}
490490
}
@@ -557,6 +557,7 @@ StoredValue::UniquePtr createWithFactory(AbstractStoredValueFactory& factory,
557557
}
558558

559559
auto sv = factory(item, {});
560+
sv->setDatatype(t);
560561

561562
if (s != State::Document) {
562563
// Temp items have no value.

0 commit comments

Comments
 (0)