ConflictSet.cpp

@@ -96,7 +96,6 @@ int64_t getSearchPathLength(struct Node *);
 
 // ==================== BEGIN IMPLEMENTATION ====================
 
-constexpr int64_t kNominalVersionWindow = 2e9;
 constexpr int64_t kMaxCorrectVersionWindow =
     std::numeric_limits<int32_t>::max();
 static_assert(kNominalVersionWindow <= kMaxCorrectVersionWindow);

Internal.h

@@ -27,6 +27,15 @@ using namespace weaselab;
 #define SHOW_MEMORY 0
 #endif
 
+// The window of versions, ending at `newestVersion`, that a subsequent `check`
+// is guaranteed to answer correctly. Reads older than
+// `newestVersion - kNominalVersionWindow` are reported as `TooOld`.
+//
+// This must be an integer constant: `newestVersion - 2e9` is computed in
+// `double`, whose 53 bit mantissa makes the boundary off by up to 512 versions
+// once `newestVersion >= 2^53`.
+constexpr int64_t kNominalVersionWindow = 2000000000;
+
 // std::span is not trivially constructible. We want a span that leaves its
 // members uninitialized for performance reasons.
 struct TrivialSpan {
@@ -513,7 +522,7 @@ struct ReferenceImpl {
              int count) const {
     for (int i = 0; i < count; ++i) {
       if (reads[i].readVersion < oldestVersion ||
-          reads[i].readVersion < newestVersion - 2e9) {
+          reads[i].readVersion < newestVersion - kNominalVersionWindow) {
         results[i] = ConflictSet::TooOld;
         continue;
       }

SkipList.cpp

@@ -741,7 +741,7 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     int iters = skipList.detectConflicts(ranges, count, results);
     for (int i = 0; i < count; ++i) {
       if (reads[i].readVersion < oldestVersion ||
-          reads[i].readVersion < newestVersion - 2e9) {
+          reads[i].readVersion < newestVersion - kNominalVersionWindow) {
         results[i] = TooOld;
       }
       tls.commits_accum += results[i] == Commit;

test_conflict_set.py

@@ -209,6 +209,22 @@ def test_two_billion_versions():
         cs.check(read(1, b"\x00", b"\xff"))
 
 
+def test_too_old_window_precision():
+    # Regression test for issue #99: the TooOld window boundary
+    # (`newestVersion - 2e9`) must be computed with integer arithmetic. It used
+    # to be computed in `double`, whose 53 bit mantissa makes the boundary off
+    # by up to ~512 versions once newestVersion >= 2^53.
+    for newest in (1 << 53, (1 << 53) + 1, (1 << 53) + 7, 2**63 - 1):
+        with DebugConflictSet() as cs:
+            cs.addWrites(newest)  # No writes; advances newestVersion
+            boundary = newest - 2 * 10**9
+            # One below the window is TooOld, even though `double` rounding
+            # used to put it inside the window.
+            assert cs.check(read(boundary - 1, b"k")) == [Result.TOO_OLD]
+            # Exactly at the boundary is not TooOld.
+            assert cs.check(read(boundary, b"k")) == [Result.COMMIT]
+
+
 def test_positive_bytes():
     with DebugConflictSet() as cs:
         cs.addWrites(1, write(b"hello"))