From 5534b56ce3f144e31b6955d8f50c9874b27262df Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 23 Jun 2016 14:47:27 -0700 Subject: [PATCH] When increasing the version number, don't only look at the version number in the block, but also at the last version number given to it by the current client. Ensure, the new version number is higher than both. --- .../versioncounting/KnownBlockVersions.cpp | 17 +- .../versioncounting/KnownBlockVersions.h | 3 +- .../versioncounting/VersionCountingBlock.h | 10 +- .../VersionCountingBlockStore.h | 2 +- .../KnownBlockVersionsTest.cpp | 218 ++++++++++++++---- 5 files changed, 193 insertions(+), 57 deletions(-) diff --git a/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp b/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp index 6cada5f5..46c6d01c 100644 --- a/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp +++ b/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp @@ -68,10 +68,21 @@ bool KnownBlockVersions::checkAndUpdateVersion(uint32_t clientId, const Key &key return true; } -void KnownBlockVersions::updateVersion(const Key &key, uint64_t version) { - if (!checkAndUpdateVersion(_myClientId, key, version)) { - throw std::logic_error("Tried to decrease block version"); +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); + _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() { diff --git a/src/blockstore/implementations/versioncounting/KnownBlockVersions.h b/src/blockstore/implementations/versioncounting/KnownBlockVersions.h index b97693c8..e08b6c50 100644 --- a/src/blockstore/implementations/versioncounting/KnownBlockVersions.h +++ b/src/blockstore/implementations/versioncounting/KnownBlockVersions.h @@ -23,9 +23,10 @@ namespace blockstore { __attribute__((warn_unused_result)) bool checkAndUpdateVersion(uint32_t clientId, const Key &key, uint64_t version); - void updateVersion(const Key &key, uint64_t version); + 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 7ebdd8a7..8a8e5847 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlock.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlock.h @@ -65,7 +65,7 @@ private: DISALLOW_COPY_AND_ASSIGN(VersionCountingBlock); public: - static constexpr uint64_t VERSION_ZERO = 1; // lowest block version is '1', because that is required by class KnownBlockVersions. + 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); @@ -74,14 +74,15 @@ public: inline boost::optional> VersionCountingBlock::TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, KnownBlockVersions *knownBlockVersions) { - cpputils::Data dataWithHeader = _prependHeaderToData(knownBlockVersions->myClientId(), VERSION_ZERO, std::move(data)); + uint64_t version = knownBlockVersions->incrementVersion(key, VERSION_ZERO); + + cpputils::Data dataWithHeader = _prependHeaderToData(knownBlockVersions->myClientId(), version, std::move(data)); auto baseBlock = baseBlockStore->tryCreate(key, dataWithHeader.copy()); // TODO Copy necessary? if (baseBlock == boost::none) { //TODO Test this code branch return boost::none; } - knownBlockVersions->updateVersion(key, VERSION_ZERO); return cpputils::make_unique_ref(std::move(*baseBlock), std::move(dataWithHeader), knownBlockVersions); } @@ -184,7 +185,7 @@ inline void VersionCountingBlock::resize(size_t newSize) { inline void VersionCountingBlock::_storeToBaseBlock() { if (_dataChanged) { - ++_version; + _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"); @@ -194,7 +195,6 @@ inline void VersionCountingBlock::_storeToBaseBlock() { std::memcpy(_dataWithHeader.dataOffset(CLIENTID_HEADER_OFFSET), &myClientId, sizeof(myClientId)); std::memcpy(_dataWithHeader.dataOffset(VERSION_HEADER_OFFSET), &_version, sizeof(_version)); _baseBlock->write(_dataWithHeader.data(), 0, _dataWithHeader.size()); - _knownBlockVersions->updateVersion(key(), _version); _dataChanged = false; } } diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h index 13018c7b..ae59e1a9 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h @@ -64,7 +64,7 @@ inline void VersionCountingBlockStore::remove(cpputils::unique_ref block) ASSERT(versionCountingBlock != boost::none, "Block is not an VersionCountingBlock"); auto baseBlock = (*versionCountingBlock)->releaseBlock(); _baseBlockStore->remove(std::move(baseBlock)); - _knownBlockVersions.updateVersion(key, VersionCountingBlock::VERSION_DELETED); + _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 9a4a144e..045d4fd2 100644 --- a/test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp +++ b/test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp @@ -1,8 +1,10 @@ #include #include #include +#include using blockstore::versioncounting::KnownBlockVersions; +using blockstore::versioncounting::VersionCountingBlock; using cpputils::TempFile; class KnownBlockVersionsTest : public ::testing::Test { @@ -23,74 +25,196 @@ public: } }; -TEST_F(KnownBlockVersionsTest, update_newEntry_lowversion) { - testobj.updateVersion(key, 1); +TEST_F(KnownBlockVersionsTest, setandget) { + testobj.setVersion(clientId, key, 5); + EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); } -TEST_F(KnownBlockVersionsTest, update_newEntry_highversion) { - testobj.updateVersion(key, 100); +TEST_F(KnownBlockVersionsTest, setandget_isPerClientId) { + testobj.setVersion(clientId, key, 5); + testobj.setVersion(clientId2, key, 3); + EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); + EXPECT_EQ(3u, testobj.getBlockVersion(clientId2, key)); } -TEST_F(KnownBlockVersionsTest, update_existingEntry_equal_lowversion) { - testobj.updateVersion(key, 1); - testobj.updateVersion(key, 1); +TEST_F(KnownBlockVersionsTest, setandget_isPerBlock) { + testobj.setVersion(clientId, key, 5); + testobj.setVersion(clientId, key2, 3); + EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); + EXPECT_EQ(3u, testobj.getBlockVersion(clientId, key2)); } -TEST_F(KnownBlockVersionsTest, update_existingEntry_equal_highversion) { - testobj.updateVersion(key, 100); - testobj.updateVersion(key, 100); +TEST_F(KnownBlockVersionsTest, setandget_allowsIncreasing) { + testobj.setVersion(clientId, key, 5); + testobj.setVersion(clientId, key, 6); + EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key)); } -TEST_F(KnownBlockVersionsTest, update_existingEntry_nonequal) { - testobj.updateVersion(key, 100); - testobj.updateVersion(key, 101); -} - -TEST_F(KnownBlockVersionsTest, update_existingEntry_invalid) { - testobj.updateVersion(key, 100); +TEST_F(KnownBlockVersionsTest, setandget_doesntAllowDecreasing) { + testobj.setVersion(clientId, key, 5); EXPECT_ANY_THROW( - testobj.updateVersion(key, 99); + testobj.setVersion(clientId, key, 4); ); } -TEST_F(KnownBlockVersionsTest, update_updatesOwnClientId) { - testobj.updateVersion(key, 100); - EXPECT_VERSION_IS(100, &testobj, key, testobj.myClientId()); +TEST_F(KnownBlockVersionsTest, myClientId_isConsistent) { + EXPECT_EQ(testobj.myClientId(), testobj.myClientId()); } -TEST_F(KnownBlockVersionsTest, checkAndUpdate_newEntry_lowversion) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 1)); +TEST_F(KnownBlockVersionsTest, incrementVersion_newentry_versionzero) { + auto version = testobj.incrementVersion(key, VersionCountingBlock::VERSION_ZERO); + EXPECT_EQ(1u, version); + EXPECT_EQ(1u, testobj.getBlockVersion(testobj.myClientId(), key)); } -TEST_F(KnownBlockVersionsTest, checkAndUpdate_newEntry_highversion) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 100)); +TEST_F(KnownBlockVersionsTest, incrementVersion_newentry_versionnotzero) { + auto version = testobj.incrementVersion(key, 5); + EXPECT_EQ(6u, version); + EXPECT_EQ(6u, testobj.getBlockVersion(testobj.myClientId(), key)); } -TEST_F(KnownBlockVersionsTest, checkAndUpdate_existingEntry_equal_lowversion) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 1)); - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 1)); +TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_sameVersion) { + testobj.setVersion(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, checkAndUpdate_existingEntry_equal_highversion) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 100)); - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 100)); +TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_lowerVersion1) { + testobj.setVersion(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, checkAndUpdate_existingEntry_nonequal) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 100)); - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 101)); +TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_lowerVersion2) { + testobj.setVersion(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, checkAndUpdate_existingEntry_invalid) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 100)); - EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 99)); +TEST_F(KnownBlockVersionsTest, incrementVersion_oldentry_higherVersion) { + testobj.setVersion(testobj.myClientId(), key, 5); + auto version = testobj.incrementVersion(key, 6); + EXPECT_EQ(7u, version); + EXPECT_EQ(7u, testobj.getBlockVersion(testobj.myClientId(), key)); } -TEST_F(KnownBlockVersionsTest, checkAndUpdate_existingEntry_invalidDoesntModifyEntry) { - EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 100)); - EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 99)); +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_newentry) { + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 5)); + EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); +} - EXPECT_VERSION_IS(100, &testobj, key, clientId); +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_sameClientSameVersion) { + testobj.setVersion(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); + 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); + 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); + 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); + 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); + 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); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); + EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 3)); + EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); + EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersion) { + testobj.setVersion(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)); + EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientHigherVersion) { + testobj.setVersion(clientId, key, 5); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 6)); + EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key)); + EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientLowerVersion_oldClientIsSelf) { + testobj.incrementVersion(key, 4); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); + EXPECT_FALSE(testobj.checkAndUpdateVersion(testobj.myClientId(), key, 3)); + EXPECT_EQ(5u, testobj.getBlockVersion(testobj.myClientId(), key)); + EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersion_oldClientIsSelf) { + testobj.incrementVersion(key, 4); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); + EXPECT_FALSE(testobj.checkAndUpdateVersion(testobj.myClientId(), key, 5)); // Don't allow rollback to old client's newest block, if it was superseded by another client + EXPECT_EQ(5u, testobj.getBlockVersion(testobj.myClientId(), key)); + EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientHigherVersion_oldClientIsSelf) { + testobj.incrementVersion(key, 4); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId2, key, 7)); + EXPECT_TRUE(testobj.checkAndUpdateVersion(testobj.myClientId(), key, 6)); + EXPECT_EQ(6u, testobj.getBlockVersion(testobj.myClientId(), key)); + EXPECT_EQ(7u, testobj.getBlockVersion(clientId2, key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientLowerVersion_newClientIsSelf) { + testobj.setVersion(clientId, key, 5); + testobj.incrementVersion(key, 6); + EXPECT_FALSE(testobj.checkAndUpdateVersion(clientId, key, 3)); + EXPECT_EQ(5u, testobj.getBlockVersion(clientId, key)); + EXPECT_EQ(7u, testobj.getBlockVersion(testobj.myClientId(), key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientSameVersion_newClientIsSelf) { + testobj.setVersion(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)); + EXPECT_EQ(7u, testobj.getBlockVersion(testobj.myClientId(), key)); +} + +TEST_F(KnownBlockVersionsTest, checkAndUpdateVersion_oldentry_oldClientHigherVersion_newClientIsSelf) { + testobj.setVersion(clientId, key, 5); + testobj.incrementVersion(key, 6); + EXPECT_TRUE(testobj.checkAndUpdateVersion(clientId, key, 6)); + EXPECT_EQ(6u, testobj.getBlockVersion(clientId, key)); + EXPECT_EQ(7u, testobj.getBlockVersion(testobj.myClientId(), key)); } TEST_F(KnownBlockVersionsTest, checkAndUpdate_twoEntriesDontInfluenceEachOther_differentKeys) { @@ -137,22 +261,22 @@ TEST_F(KnownBlockVersionsTest, saveAndLoad_oneentry) { EXPECT_TRUE(KnownBlockVersions(stateFile.path()).checkAndUpdateVersion(clientId, key, 100)); KnownBlockVersions obj(stateFile.path()); - EXPECT_VERSION_IS(100, &obj, key, clientId); + EXPECT_EQ(100u, obj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, saveAndLoad_threeentries) { TempFile stateFile(false); { KnownBlockVersions obj(stateFile.path()); - obj.updateVersion(key, 100); - obj.updateVersion(key2, 50); + EXPECT_TRUE(obj.checkAndUpdateVersion(obj.myClientId(), key, 100)); + EXPECT_TRUE(obj.checkAndUpdateVersion(obj.myClientId(), key2, 50)); EXPECT_TRUE(obj.checkAndUpdateVersion(clientId, key, 150)); } KnownBlockVersions obj(stateFile.path()); - EXPECT_VERSION_IS(100, &obj, key, obj.myClientId()); - EXPECT_VERSION_IS(50, &obj, key2, obj.myClientId()); - EXPECT_VERSION_IS(150, &obj, key, clientId); + EXPECT_EQ(100u, obj.getBlockVersion(obj.myClientId(), key)); + EXPECT_EQ(50u, obj.getBlockVersion(obj.myClientId(), key2)); + EXPECT_EQ(150u, obj.getBlockVersion(clientId, key)); } TEST_F(KnownBlockVersionsTest, saveAndLoad_lastUpdateClientIdIsStored) {