Compare commits

..
1 Commits
Author SHA1 Message Date
weaselbot b1c1c39611 Report the version-window reset's releases and erasures in the metrics
CI / pre-commit (pull_request) Successful in 2m18s
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m56s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (pull_request) Successful in 4m2s
CI / release (arm64, , ubuntu-latest-arm64) (pull_request) Successful in 3m32s
CI / test (amd64, -DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 21, , 64-bit-versions, ubuntu-latest-amd64) (pull_request) Successful in 3m37s
CI / test (amd64, -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, 21, , gcc, ubuntu-latest-amd64) (pull_request) Successful in 3m35s
CI / test (amd64, -DUSE_SIMD_FALLBACK=ON, 21, , simd-fallback, ubuntu-latest-amd64) (pull_request) Successful in 3m41s
CI / release (amd64, -DMSAN_TOOLCHAIN_PATH=/opt/msan, ubuntu-latest-amd64) (pull_request) Successful in 6m2s
CI / coverage (pull_request) Successful in 4m9s
`addWrites()` tears down the whole tree with `eraseTree()` when the version
window is exceeded, then calls `init()`, which reconstructs the `WriteContext`
and zeroes `accum`. Everything `eraseTree()` accounted for was discarded
before the metrics were reported, so `nodes_released_total` and
`entries_erased_total` never reflected the one path that frees an entire tree.

`init()` now carries `accum` across the reconstruction of `writeContext`, so
the accounting done before the reset still gets reported by `addWrites()`.

`eraseTree()` also incremented `accum.nodes_released` itself before calling
`release()`, which increments the same counter, double counting every node it
released. That was only invisible because `init()` zeroed `accum`. Drop the
redundant increment and keep the entry accounting, which `release()` does not
do.

Extend ShowMemoryTest.cpp to check that a version jump reports the nodes and
entries it erases.

Closes #100
2026-11-10 15:06:19 -05:00
2 changed files with 34 additions and 1 deletions

No files matched your search

+8 -1
View File
@@ -2823,8 +2823,9 @@ void eraseTree(Node *root, WriteContext *writeContext) {
while (toFree.size() > 0) { while (toFree.size() > 0) {
auto *n = toFree.back(); auto *n = toFree.back();
toFree.pop_back(); toFree.pop_back();
// `release()` below accounts for the node itself, so only the entry needs
// to be accounted for here.
writeContext->accum.entries_erased += n->entryPresent; writeContext->accum.entries_erased += n->entryPresent;
++writeContext->accum.nodes_released;
#if SHOW_MEMORY #if SHOW_MEMORY
const int64_t searchPathLength = searchPathLengths.back(); const int64_t searchPathLength = searchPathLengths.back();
@@ -5377,6 +5378,11 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
int64_t getBytes() const { return totalBytes; } int64_t getBytes() const { return totalBytes; }
void init(int64_t oldestVersion) { void init(int64_t oldestVersion) {
// Metrics are reported by `addWrites()`, which calls this to reset the
// tree. Whatever was accounted for before the reset (in particular by
// `eraseTree()`) must survive the reconstruction of `writeContext` below.
const WriteContext::Accum accum = writeContext.accum;
this->oldestVersion = InternalVersionT(oldestVersion); this->oldestVersion = InternalVersionT(oldestVersion);
oldestVersionFullPrecision = oldestExtantVersion = oldestVersionAtGcBegin = oldestVersionFullPrecision = oldestExtantVersion = oldestVersionAtGcBegin =
newestVersionFullPrecision = oldestVersion; newestVersionFullPrecision = oldestVersion;
@@ -5386,6 +5392,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
writeContext.~WriteContext(); writeContext.~WriteContext();
new (&writeContext) WriteContext(); new (&writeContext) WriteContext();
writeContext.accum = accum;
// Leave removalBuffer as is // Leave removalBuffer as is
removalKey = {}; removalKey = {};
+26
View File
@@ -8,6 +8,7 @@
#include <cassert> #include <cassert>
#include <cstdint> #include <cstdint>
#include <cstring>
#include <string> #include <string>
using namespace weaselab; using namespace weaselab;
@@ -30,6 +31,20 @@ ConflictSet::Result checkPoint(const ConflictSet &cs, const std::string &key,
return result; return result;
} }
// Value of the metric named `name`.
double metricValue(const ConflictSet &cs, const char *name) {
ConflictSet::MetricsV1 *metrics;
int count;
cs.getMetricsV1(&metrics, &count);
for (int i = 0; i < count; ++i) {
if (strcmp(metrics[i].name, name) == 0) {
return metrics[i].getValue();
}
}
assert(false);
return 0;
}
// Everything a ConflictSet allocated must have been accounted for and released // Everything a ConflictSet allocated must have been accounted for and released
// by the time it's destroyed. // by the time it's destroyed.
void checkFullyReleased() { void checkFullyReleased() {
@@ -67,6 +82,12 @@ int main() {
writePoint(cs, "abc", version); writePoint(cs, "abc", version);
assert(checkPoint(cs, "abc", 0) == ConflictSet::TooOld); assert(checkPoint(cs, "abc", 0) == ConflictSet::TooOld);
assert(checkPoint(cs, "abc", version) == ConflictSet::Commit); assert(checkPoint(cs, "abc", version) == ConflictSet::Commit);
// The erased tree held the "" entry in the root and the "abc" entry in a
// single child node. Erasing the tree must be reported by the metrics, and
// each node and entry must be counted exactly once.
assert(metricValue(cs, "entries_erased_total") == 2);
assert(metricValue(cs, "nodes_released_total") == 2);
} }
checkFullyReleased(); checkFullyReleased();
@@ -80,6 +101,11 @@ int main() {
writePoint(cs, "prefix/1", version); writePoint(cs, "prefix/1", version);
assert(checkPoint(cs, "prefix/1", version) == ConflictSet::Commit); assert(checkPoint(cs, "prefix/1", version) == ConflictSet::Commit);
// Erasing a large tree is reported too: the 1000 entries written above
// plus the "" entry in the root, and at least one node per entry.
assert(metricValue(cs, "entries_erased_total") == 1001);
assert(metricValue(cs, "nodes_released_total") >= 1000);
for (int i = 0; i < 1000; ++i) { for (int i = 0; i < 1000; ++i) {
writePoint(cs, "other/" + std::to_string(i), version + i + 1); writePoint(cs, "other/" + std::to_string(i), version + i + 1);
} }