HashTable.cpp never tracked newestVersion, so it could not implement the addWrites postcondition (newestVersion becomes writeVersion; oldestVersion
becomes the larger of its previous value and newestVersion - two billion) or
the class invariant oldestVersion >= newestVersion - two billion. Reads whose readVersion fell out of the two-billion-version window were reported Conflict/Commit instead of TooOld, so a caller that switched to the hash_table implementation silently lost the "this read version is too old,
retry" signal.
Fix
Mirror SkipList.cpp / ReferenceImpl:
Impl gains an int64_t newestVersion, initialized to the constructor's oldestVersion.
addWrites asserts writeVersion >= newestVersion and sets newestVersion = writeVersion before the loop, so it advances even when count == 0.
check returns TooOld when version < oldestVersion || version < newestVersion - kVersionWindow, where kVersionWindow is 2'000'000'000 (integer arithmetic, no float rounding).
oldestVersion itself is left to setOldestVersion, exactly like the skip
list: the window is enforced at check time.
Tests
test_two_billion_versions now asserts its results (it previously asserted
nothing, so it would have passed even if all implementations agreed on a
wrong answer).
New test_two_billion_version_window exercises radix_tree, skip_list and hash_table with and without writes, at newestVersion, just inside the
window (newestVersion - 2e9, still Conflict/Commit) and just outside
it (newestVersion - 2e9 - 1, TooOld). It fails on main and passes here.
Full ctest suite passes in a Debug build (8603 tests, 0 failures), and hash_table also builds cleanly with -DCMAKE_BUILD_TYPE=Release. clang-format
and black pass via pre-commit.
Closes #98.
## Problem
`HashTable.cpp` never tracked `newestVersion`, so it could not implement the
`addWrites` postcondition (`newestVersion` becomes `writeVersion`; `oldestVersion`
becomes the larger of its previous value and `newestVersion - two billion`) or
the class invariant `oldestVersion >= newestVersion - two billion`. Reads whose
`readVersion` fell out of the two-billion-version window were reported
`Conflict`/`Commit` instead of `TooOld`, so a caller that switched to the
`hash_table` implementation silently lost the "this read version is too old,
retry" signal.
## Fix
Mirror `SkipList.cpp` / `ReferenceImpl`:
* `Impl` gains an `int64_t newestVersion`, initialized to the constructor's
`oldestVersion`.
* `addWrites` asserts `writeVersion >= newestVersion` and sets
`newestVersion = writeVersion` before the loop, so it advances even when
`count == 0`.
* `check` returns `TooOld` when
`version < oldestVersion || version < newestVersion - kVersionWindow`, where
`kVersionWindow` is `2'000'000'000` (integer arithmetic, no float rounding).
`oldestVersion` itself is left to `setOldestVersion`, exactly like the skip
list: the window is enforced at `check` time.
## Tests
* `test_two_billion_versions` now asserts its results (it previously asserted
nothing, so it would have passed even if all implementations agreed on a
wrong answer).
* New `test_two_billion_version_window` exercises `radix_tree`, `skip_list` and
`hash_table` with and without writes, at `newestVersion`, just inside the
window (`newestVersion - 2e9`, still `Conflict`/`Commit`) and just outside
it (`newestVersion - 2e9 - 1`, `TooOld`). It fails on `main` and passes here.
Full `ctest` suite passes in a `Debug` build (8603 tests, 0 failures), and
`hash_table` also builds cleanly with `-DCMAKE_BUILD_TYPE=Release`. `clang-format`
and `black` pass via pre-commit.
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.
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 4m6s
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 4m10s
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.
Some checks are pending
CI / test (amd64, -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, 21, , gcc, ubuntu-latest-amd64) (pull_request) Waiting to run
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 4m6s
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 4m10s
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #98.
Problem
HashTable.cppnever trackednewestVersion, so it could not implement theaddWritespostcondition (newestVersionbecomeswriteVersion;oldestVersionbecomes the larger of its previous value and
newestVersion - two billion) orthe class invariant
oldestVersion >= newestVersion - two billion. Reads whosereadVersionfell out of the two-billion-version window were reportedConflict/Commitinstead ofTooOld, so a caller that switched to thehash_tableimplementation silently lost the "this read version is too old,retry" signal.
Fix
Mirror
SkipList.cpp/ReferenceImpl:Implgains anint64_t newestVersion, initialized to the constructor'soldestVersion.addWritesassertswriteVersion >= newestVersionand setsnewestVersion = writeVersionbefore the loop, so it advances even whencount == 0.checkreturnsTooOldwhenversion < oldestVersion || version < newestVersion - kVersionWindow, wherekVersionWindowis2'000'000'000(integer arithmetic, no float rounding).oldestVersionitself is left tosetOldestVersion, exactly like the skiplist: the window is enforced at
checktime.Tests
test_two_billion_versionsnow asserts its results (it previously assertednothing, so it would have passed even if all implementations agreed on a
wrong answer).
test_two_billion_version_windowexercisesradix_tree,skip_listandhash_tablewith and without writes, atnewestVersion, just inside thewindow (
newestVersion - 2e9, stillConflict/Commit) and just outsideit (
newestVersion - 2e9 - 1,TooOld). It fails onmainand passes here.Full
ctestsuite passes in aDebugbuild (8603 tests, 0 failures), andhash_tablealso builds cleanly with-DCMAKE_BUILD_TYPE=Release.clang-formatand
blackpass via pre-commit.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.