diff --git a/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp b/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp index 46c6d01c..c810a0c8 100644 --- a/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp +++ b/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp @@ -71,20 +71,16 @@ bool KnownBlockVersions::checkAndUpdateVersion(uint32_t clientId, const Key &key uint64_t KnownBlockVersions::incrementVersion(const Key &key, uint64_t lastVersion) { unique_lock lock(_mutex); uint64_t &found = _knownVersions[{_myClientId, key}]; // If the entry doesn't exist, this creates it with value 0. - found = std::max(lastVersion + 1, found + 1); + uint64_t newVersion = std::max(lastVersion + 1, found + 1); + if (newVersion == std::numeric_limits::max()) { + // It's *very* unlikely we ever run out of version numbers in 64bit...but just to be sure... + throw std::runtime_error("Version overflow"); + } + found = newVersion; _lastUpdateClientId[key] = _myClientId; return found; } -void KnownBlockVersions::setVersion(uint32_t clientId, const Key &key, uint64_t version) { - uint64_t &found = _knownVersions[{clientId, key}]; - if (found > version) { - throw std::runtime_error("Version can only be set to higher version numbers."); - } - found = version; - _lastUpdateClientId[key] = clientId; -} - void KnownBlockVersions::_loadStateFile() { optional file = Data::LoadFromFile(_stateFilePath); if (file == none) { diff --git a/src/blockstore/implementations/versioncounting/KnownBlockVersions.h b/src/blockstore/implementations/versioncounting/KnownBlockVersions.h index e08b6c50..2e7acbd8 100644 --- a/src/blockstore/implementations/versioncounting/KnownBlockVersions.h +++ b/src/blockstore/implementations/versioncounting/KnownBlockVersions.h @@ -26,7 +26,6 @@ namespace blockstore { uint64_t incrementVersion(const Key &key, uint64_t lastVersion); uint64_t getBlockVersion(uint32_t clientId, const Key &key) const; - void setVersion(uint32_t clientId, const Key &key, uint64_t version); uint32_t myClientId() const; diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlock.h b/src/blockstore/implementations/versioncounting/VersionCountingBlock.h index 8a8e5847..cc5a157d 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlock.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlock.h @@ -41,6 +41,7 @@ public: size_t size() const override; void resize(size_t newSize) override; + uint64_t version() const; cpputils::unique_ref releaseBlock(); private: @@ -59,14 +60,13 @@ private: // This header is prepended to blocks to allow future versions to have compatibility. static constexpr uint16_t FORMAT_VERSION_HEADER = 0; + static constexpr uint64_t VERSION_ZERO = 0; std::mutex _mutex; DISALLOW_COPY_AND_ASSIGN(VersionCountingBlock); public: - static constexpr uint64_t VERSION_ZERO = 0; - static constexpr uint64_t VERSION_DELETED = std::numeric_limits::max(); static constexpr unsigned int CLIENTID_HEADER_OFFSET = sizeof(FORMAT_VERSION_HEADER); static constexpr unsigned int VERSION_HEADER_OFFSET = sizeof(FORMAT_VERSION_HEADER) + sizeof(uint32_t); static constexpr unsigned int HEADER_LENGTH = sizeof(FORMAT_VERSION_HEADER) + sizeof(uint32_t) + sizeof(VERSION_ZERO); @@ -186,11 +186,6 @@ inline void VersionCountingBlock::resize(size_t newSize) { inline void VersionCountingBlock::_storeToBaseBlock() { if (_dataChanged) { _version = _knownBlockVersions->incrementVersion(key(), _version); - if (_version == VERSION_DELETED) { - // It's *very* unlikely we ever run out of version numbers in 64bit...but just to be sure... - throw std::runtime_error("Version overflow"); - static_assert(VERSION_DELETED == std::numeric_limits::max(), "The check above assumes VERSION_DELETE to be an upper bound."); - } uint32_t myClientId = _knownBlockVersions->myClientId(); std::memcpy(_dataWithHeader.dataOffset(CLIENTID_HEADER_OFFSET), &myClientId, sizeof(myClientId)); std::memcpy(_dataWithHeader.dataOffset(VERSION_HEADER_OFFSET), &_version, sizeof(_version)); @@ -212,6 +207,10 @@ inline uint64_t VersionCountingBlock::blockSizeFromPhysicalBlockSize(uint64_t bl return blockSize - HEADER_LENGTH; } +inline uint64_t VersionCountingBlock::version() const { + return _version; +} + } } diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h index ae59e1a9..5a77a401 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h @@ -62,9 +62,9 @@ inline void VersionCountingBlockStore::remove(cpputils::unique_ref block) Key key = block->key(); auto versionCountingBlock = cpputils::dynamic_pointer_move(block); ASSERT(versionCountingBlock != boost::none, "Block is not an VersionCountingBlock"); + _knownBlockVersions.incrementVersion(key, (*versionCountingBlock)->version()); auto baseBlock = (*versionCountingBlock)->releaseBlock(); _baseBlockStore->remove(std::move(baseBlock)); - _knownBlockVersions.setVersion(_knownBlockVersions.myClientId(), key, VersionCountingBlock::VERSION_DELETED); } inline uint64_t VersionCountingBlockStore::numBlocks() const { diff --git a/test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp b/test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp index 045d4fd2..bf2c6f98 100644 --- a/test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp +++ b/test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp @@ -19,6 +19,12 @@ public: TempFile stateFile; KnownBlockVersions testobj; + void setVersion(KnownBlockVersions *testobj, uint32_t clientId, const blockstore::Key &key, uint64_t version) { + if (!testobj->checkAndUpdateVersion(clientId, key, version)) { + throw std::runtime_error("Couldn't increase version"); + } + } + void EXPECT_VERSION_IS(uint64_t version, KnownBlockVersions *testobj, blockstore::Key &key, uint32_t clientId) { EXPECT_FALSE(testobj->checkAndUpdateVersion(clientId, key, version-1)); EXPECT_TRUE(testobj->checkAndUpdateVersion(clientId, key, version+1)); @@ -26,34 +32,34 @@ public: }; TEST_F(KnownBlockVersionsTest, setandget) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, setandget_isPerClientId) { - testobj.setVersion(clientId, key, 5); - testobj.setVersion(clientId2, key, 3); + setVersion(&testobj, clientId, key, 5); + setVersion(&testobj, clientId2, key, 3); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); EXPECT_EQ(3u, testobj.getBlockVersion(clientId2, key)); } TEST_F(KnownBlockVersionsTest, setandget_isPerBlock) { - testobj.setVersion(clientId, key, 5); - testobj.setVersion(clientId, key2, 3); + setVersion(&testobj, clientId, key, 5); + setVersion(&testobj, clientId, key2, 3); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); EXPECT_EQ(3u, testobj.getBlockVersion(clientId, key2)); } TEST_F(KnownBlockVersionsTest, setandget_allowsIncreasing) { - testobj.setVersion(clientId, key, 5); - testobj.setVersion(clientId, key, 6); + setVersion(&testobj, clientId, key, 5); + setVersion(&testobj, clientId, key, 6); EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, setandget_doesntAllowDecreasing) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_ANY_THROW( - testobj.setVersion(clientId, key, 4); + setVersion(&testobj, clientId, key, 4); ); } @@ -74,28 +80,28 @@ TEST_F(KnownBlockVersionsTest, incrementVersion_newentry_versionnotzero) { } TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_sameVersion) { - testobj.setVersion(testobj.myClientId(), key, 5); + setVersion(&testobj, testobj.myClientId(), key, 5); auto version = testobj.incrementVersion(key, 5); EXPECT_EQ(6u, version); EXPECT_EQ(6u, testobj.getBlockVersion(testobj.myClientId(), key)); } TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_lowerVersion1) { - testobj.setVersion(testobj.myClientId(), key, 5); + setVersion(&testobj, testobj.myClientId(), key, 5); auto version = testobj.incrementVersion(key, 4); EXPECT_EQ(6u, version); EXPECT_EQ(6u, testobj.getBlockVersion(testobj.myClientId(), key)); } TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_lowerVersion2) { - testobj.setVersion(testobj.myClientId(), key, 5); + setVersion(&testobj, testobj.myClientId(), key, 5); auto version = testobj.incrementVersion(key, 3); EXPECT_EQ(6u, version); EXPECT_EQ(6u, testobj.getBlockVersion(testobj.myClientId(), key)); } TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_higherVersion) { - testobj.setVersion(testobj.myClientId(), key, 5); + setVersion(&testobj, testobj.myClientId(), key, 5); auto version = testobj.incrementVersion(key, 6); EXPECT_EQ(7u, version); EXPECT_EQ(7u, testobj.getBlockVersion(testobj.myClientId(), key)); @@ -107,46 +113,46 @@ TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_newentry) { } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_sameClientSameVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 5)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_sameClientLowerVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 4)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_sameClientNewerVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 6)); EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_differentClientSameVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 5)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId2, key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_differentClientLowerVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 3)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); EXPECT_EQ(3u, testobj.getBlockVersion(clientId2, key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_differentClientHigherVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientLowerVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 3)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); @@ -154,7 +160,7 @@ TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientLowerVers } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 5)); // Don't allow rollback to old client's newest block, if it was superseded by another client EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); @@ -162,7 +168,7 @@ TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersi } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientHigherVersion) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 6)); EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key)); @@ -194,7 +200,7 @@ TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientHigherVer } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientLowerVersion_newClientIsSelf) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); testobj.incrementVersion(key, 6); EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 3)); EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); @@ -202,7 +208,7 @@ TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientLowerVers } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersion_newClientIsSelf) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); testobj.incrementVersion(key, 6); EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 5)); // Don't allow rollback to old client's newest block, if it was superseded by another client EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); @@ -210,7 +216,7 @@ TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersi } TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientHigherVersion_newClientIsSelf) { - testobj.setVersion(clientId, key, 5); + setVersion(&testobj, clientId, key, 5); testobj.incrementVersion(key, 6); EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 6)); EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key));