test(nodestore): assert store() times itself, now that it does

The write-duration test encoded the behaviour from before the store path was
timed. It populated the source database through store(), then asserted that
database's write duration was still zero, which was only true while
importDatabase was the sole timed path. Both concrete store overrides now
time their backend write, so an ordinary store run accumulates a duration
and the assertion failed.

Turned that stale assertion into a positive one: an ordinary storeBatch must
produce a non-zero duration, which is the case a real node actually
exercises. The negative half of the test, proving the accumulation is
per-database rather than a shared global, now checks that the import leaves
the source's store COUNT unchanged, since the source's duration is
legitimately non-zero from its own writes.

80552 tests across the six affected suites pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-07-27 14:07:42 +01:00
parent c000417ff4
commit c9b90ec527

View File

@@ -566,15 +566,18 @@ public:
fetchCopyOfBatch(*db, &copy, batch);
BEAST_EXPECT(db->getFetchTotalCount() == kNumObjectsToTest);
// store() is pure virtual, so the write path is timed per concrete
// database rather than in one shared wrapper. storeStats keeps the
// count, and the object count must match exactly.
// store() is pure virtual, so the write path is timed inside each
// concrete database rather than in one shared wrapper. Both overrides
// time their backend write, so an ordinary store run -- not just the
// import path -- accumulates a duration. That is the whole point of the
// write-latency signal: on a real node the only writes are these.
storeBatch(*db, batch);
BEAST_EXPECT(db->getStoreCount() == kNumObjectsToTest);
BEAST_EXPECT(db->getStoreDurationUs() > 0);
// importDatabase routes through Database::importInternal, the store
// path this work package instruments, so the write duration must be
// non-zero afterwards. Asserted as a strict advance from the zero
// importDatabase routes through Database::importInternal, one of the
// three timed store paths, so the write duration must be non-zero
// afterwards. Asserted as a strict advance from the zero
// above: the exact microsecond value is wall-clock dependent, but
// "still exactly zero after real writes" is precisely the dead-member
// bug being guarded against.
@@ -592,10 +595,12 @@ public:
BEAST_EXPECT(dest->getStoreCount() == kNumObjectsToTest);
BEAST_EXPECT(dest->getStoreDurationUs() > 0);
// The import wrote into `dest`, not `db`, so the source's write
// duration must be unchanged. This is the negative half: it proves the
// accumulation is per-database state and not a shared global.
BEAST_EXPECT(db->getStoreDurationUs() == 0);
// The import wrote into `dest`, not `db`, so the source's store COUNT
// must be unchanged by the import. This is the negative half: it proves
// the accumulation is per-database state and not a shared global. The
// source's duration is non-zero from its own storeBatch above, so the
// count is what isolates the import's effect.
BEAST_EXPECT(db->getStoreCount() == kNumObjectsToTest);
// A mean derived the way the telemetry gauge derives it must be a
// sane, non-zero microsecond figure rather than a division artifact.