From 78a5e8a5d5677b146978cd28e699182b08e75d8d Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Tue, 10 Nov 2026 15:28:58 -0500 Subject: [PATCH 1/2] hash_table: return TooOld outside the two-billion-version window addWrites now advances newestVersion (even when count == 0), and check treats a readVersion more than two billion versions behind newestVersion as TooOld, matching the documented postcondition and the radix tree and skip list implementations. Previously hash_table discarded writeVersion entirely, so callers that switched implementations silently lost the "this read version is too old, retry" signal. --- HashTable.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/HashTable.cpp b/HashTable.cpp index b1dfc92..58c50cf 100644 --- a/HashTable.cpp +++ b/HashTable.cpp @@ -18,15 +18,21 @@ struct string_hash { } }; +// Reads more than this many versions behind `newestVersion` are `TooOld`, so +// that a caller whose read version fell out of the version window is told to +// retry. See the `addWrites` postcondition in ConflictSet.h. +constexpr int64_t kVersionWindow = 2'000'000'000; + struct __attribute__((visibility("hidden"))) ConflictSet::Impl { - Impl(int64_t oldestVersion) : oldestVersion(oldestVersion) {} + Impl(int64_t oldestVersion) + : oldestVersion(oldestVersion), newestVersion(oldestVersion) {} void check(const ConflictSet::ReadRange *reads, ConflictSet::Result *results, int count) const { for (int i = 0; i < count; ++i) { auto key = std::string_view((const char *)reads[i].begin.p, reads[i].begin.len); auto version = reads[i].readVersion; - if (version < oldestVersion) { + if (version < oldestVersion || version < newestVersion - kVersionWindow) { results[i] = TooOld; continue; } @@ -38,6 +44,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { void addWrites(const ConflictSet::WriteRange *writes, int count, int64_t writeVersion) { + // Advance newestVersion before the loop, so that it also advances when + // count == 0, as documented in ConflictSet.h. + assert(writeVersion >= newestVersion); + newestVersion = writeVersion; for (int i = 0; i < count; ++i) { auto &max = map[std::string((const char *)writes[i].begin.p, writes[i].begin.len)]; @@ -84,6 +94,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { private: int64_t keyUpdates = 0; int64_t oldestVersion; + int64_t newestVersion; std::unordered_map> map; std::string removalKey; }; -- 2.43.0 From 2f4de127c0c45f82cd43c7a6ed0805ca25804fb0 Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Tue, 10 Nov 2026 15:28:59 -0500 Subject: [PATCH 2/2] Test the two-billion-version window in every implementation test_two_billion_versions asserted nothing about the results, and hash_table was never exercised across a version jump larger than two billion. Assert the expected results and add test_two_billion_version_window, which checks the window boundary (and the count == 0 case) for radix_tree, skip_list and hash_table. --- test_conflict_set.py | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/test_conflict_set.py b/test_conflict_set.py index 924e89e..9d5456e 100644 --- a/test_conflict_set.py +++ b/test_conflict_set.py @@ -205,8 +205,32 @@ def test_internal_version_zero(): def test_two_billion_versions(): with DebugConflictSet() as cs: cs.addWrites(int(2e9) + 1) - cs.check(read(0, b"\x00", b"\xff")) - cs.check(read(1, b"\x00", b"\xff")) + assert cs.check(read(0, b"\x00", b"\xff")) == [Result.TOO_OLD] + assert cs.check(read(1, b"\x00", b"\xff")) == [Result.COMMIT] + + +def test_two_billion_version_window(): + # Regression test for issue #98: `addWrites` advances `newestVersion`, so a + # read whose readVersion fell more than two billion versions behind it is + # TooOld. `hash_table` used to discard writeVersion entirely, so it reported + # Conflict/Commit for reads far outside the version window. + window = 2 * 10**9 + newest = 2**32 + + def probe(implementation, read_version, *writes): + with ConflictSet(0, build_dir=build_dir, implementation=implementation) as cs: + cs.addWrites(newest, *writes) + return cs.check(read(read_version, b"k"))[0] + + for implementation in ("radix_tree", "skip_list", "hash_table"): + # The window advances even when addWrites added no writes at all. + assert probe(implementation, 0, write(b"k")) == Result.TOO_OLD + assert probe(implementation, 0) == Result.TOO_OLD + assert probe(implementation, newest - window - 1, write(b"k")) == Result.TOO_OLD + assert probe(implementation, newest - window - 1) == Result.TOO_OLD + # Exactly `window` versions behind newestVersion is still evaluable. + assert probe(implementation, newest - window, write(b"k")) == Result.CONFLICT + assert probe(implementation, newest - window) == Result.COMMIT def test_positive_bytes(): -- 2.43.0