diff --git a/src/blockstore/implementations/caching/CachingBlockStore.cpp b/src/blockstore/implementations/caching/CachingBlockStore.cpp index c0de01a3..b9b96bef 100644 --- a/src/blockstore/implementations/caching/CachingBlockStore.cpp +++ b/src/blockstore/implementations/caching/CachingBlockStore.cpp @@ -18,7 +18,7 @@ namespace blockstore { namespace caching { CachingBlockStore::CachingBlockStore(cpputils::unique_ref baseBlockStore) - :_baseBlockStore(std::move(baseBlockStore)), _cache(), _numNewBlocks(0) { + :_baseBlockStore(std::move(baseBlockStore)), _newBlocks(), _cache() { } Key CachingBlockStore::createKey() { @@ -29,7 +29,6 @@ optional> CachingBlockStore::tryCreate(const Key &key, Data da ASSERT(_cache.pop(key) == none, "Key already exists in cache"); //TODO Shouldn't we return boost::none if the key already exists? //TODO Key can also already exist but not be in the cache right now. - ++_numNewBlocks; return unique_ref(make_unique_ref(make_unique_ref(key, std::move(data), this), this)); } @@ -54,9 +53,6 @@ void CachingBlockStore::remove(cpputils::unique_ref block) { auto baseBlock = (*cached_block)->releaseBlock(); auto baseNewBlock = dynamic_pointer_move(baseBlock); if (baseNewBlock != none) { - if(!(*baseNewBlock)->alreadyExistsInBaseStore()) { - --_numNewBlocks; - } (*baseNewBlock)->remove(); } else { _baseBlockStore->remove(std::move(baseBlock)); @@ -64,7 +60,7 @@ void CachingBlockStore::remove(cpputils::unique_ref block) { } uint64_t CachingBlockStore::numBlocks() const { - return _baseBlockStore->numBlocks() + _numNewBlocks; + return _baseBlockStore->numBlocks() + _newBlocks.size(); } uint64_t CachingBlockStore::estimateNumFreeBytes() const { @@ -77,11 +73,7 @@ void CachingBlockStore::release(unique_ref block) { } optional> CachingBlockStore::tryCreateInBaseStore(const Key &key, Data data) { - auto block = _baseBlockStore->tryCreate(key, std::move(data)); - if (block != none) { - --_numNewBlocks; - } - return block; + return _baseBlockStore->tryCreate(key, std::move(data)); } void CachingBlockStore::removeFromBaseStore(cpputils::unique_ref block) { @@ -96,5 +88,20 @@ uint64_t CachingBlockStore::blockSizeFromPhysicalBlockSize(uint64_t blockSize) c return _baseBlockStore->blockSizeFromPhysicalBlockSize(blockSize); } +void CachingBlockStore::forEachBlock(std::function callback) const { + _baseBlockStore->forEachBlock(callback); + for (NewBlock *newBlock : _newBlocks) { + callback(newBlock->key()); + } +} + +void CachingBlockStore::registerNewBlock(NewBlock *newBlock) { + _newBlocks.insert(newBlock); +} + +void CachingBlockStore::unregisterNewBlock(NewBlock *newBlock) { + _newBlocks.erase(newBlock); +} + } } diff --git a/src/blockstore/implementations/caching/CachingBlockStore.h b/src/blockstore/implementations/caching/CachingBlockStore.h index b6bc5d43..ef5febe7 100644 --- a/src/blockstore/implementations/caching/CachingBlockStore.h +++ b/src/blockstore/implementations/caching/CachingBlockStore.h @@ -4,10 +4,13 @@ #include "cache/Cache.h" #include "../../interface/BlockStore.h" +#include namespace blockstore { namespace caching { +class NewBlock; + //TODO Check that this blockstore allows parallel destructing of blocks (otherwise we won't encrypt blocks in parallel) class CachingBlockStore final: public BlockStore { public: @@ -20,18 +23,21 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; void release(cpputils::unique_ref block); boost::optional> tryCreateInBaseStore(const Key &key, cpputils::Data data); void removeFromBaseStore(cpputils::unique_ref block); + void registerNewBlock(NewBlock *newBlock); + void unregisterNewBlock(NewBlock *newBlock); void flush(); private: cpputils::unique_ref _baseBlockStore; + std::unordered_set _newBlocks; // List of all new blocks that aren't in the base store yet. Cache, 1000> _cache; - uint32_t _numNewBlocks; DISALLOW_COPY_AND_ASSIGN(CachingBlockStore); }; diff --git a/src/blockstore/implementations/caching/NewBlock.cpp b/src/blockstore/implementations/caching/NewBlock.cpp index 8e051935..a886666f 100644 --- a/src/blockstore/implementations/caching/NewBlock.cpp +++ b/src/blockstore/implementations/caching/NewBlock.cpp @@ -15,6 +15,7 @@ NewBlock::NewBlock(const Key &key, Data data, CachingBlockStore *blockStore) _data(std::move(data)), _baseBlock(none), _dataChanged(true) { + _blockStore->registerNewBlock(this); } NewBlock::~NewBlock() { @@ -37,6 +38,7 @@ void NewBlock::writeToBaseBlockIfChanged() { //TODO _data.copy() necessary? auto newBase = _blockStore->tryCreateInBaseStore(key(), _data.copy()); ASSERT(newBase != boost::none, "Couldn't create base block"); //TODO What if tryCreate fails due to a duplicate key? We should ensure we don't use duplicate keys. + _blockStore->unregisterNewBlock(this); // block now exists in base store, we have to remove it from the list of blocks not in the base store. _baseBlock = std::move(*newBase); } else { (*_baseBlock)->write(_data.data(), 0, _data.size()); @@ -48,6 +50,9 @@ void NewBlock::writeToBaseBlockIfChanged() { void NewBlock::remove() { if (_baseBlock != none) { _blockStore->removeFromBaseStore(std::move(*_baseBlock)); + } else { + // block doesn't exists in base store, we have to remove it from the list of blocks not in the base store. + _blockStore->unregisterNewBlock(this); } _dataChanged = false; } @@ -67,9 +72,5 @@ void NewBlock::resize(size_t newSize) { _dataChanged = true; } -bool NewBlock::alreadyExistsInBaseStore() const { - return _baseBlock != none; -} - } } diff --git a/src/blockstore/implementations/caching/NewBlock.h b/src/blockstore/implementations/caching/NewBlock.h index bdaa5172..4a9f5ddd 100644 --- a/src/blockstore/implementations/caching/NewBlock.h +++ b/src/blockstore/implementations/caching/NewBlock.h @@ -33,8 +33,6 @@ public: void remove(); - bool alreadyExistsInBaseStore() const; - private: CachingBlockStore *_blockStore; cpputils::Data _data; diff --git a/src/blockstore/implementations/compressing/CompressingBlockStore.h b/src/blockstore/implementations/compressing/CompressingBlockStore.h index 1e42cb9e..c9bfff90 100644 --- a/src/blockstore/implementations/compressing/CompressingBlockStore.h +++ b/src/blockstore/implementations/compressing/CompressingBlockStore.h @@ -21,6 +21,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; private: cpputils::unique_ref _baseBlockStore; @@ -78,6 +79,11 @@ uint64_t CompressingBlockStore::estimateNumFreeBytes() const { return _baseBlockStore->estimateNumFreeBytes(); } +template +void CompressingBlockStore::forEachBlock(std::function callback) const { + return _baseBlockStore->forEachBlock(callback); +} + template uint64_t CompressingBlockStore::blockSizeFromPhysicalBlockSize(uint64_t blockSize) const { //We probably have more since we're compressing, but we don't know exactly how much. diff --git a/src/blockstore/implementations/encrypted/EncryptedBlockStore.h b/src/blockstore/implementations/encrypted/EncryptedBlockStore.h index 5ab83327..01dd82a6 100644 --- a/src/blockstore/implementations/encrypted/EncryptedBlockStore.h +++ b/src/blockstore/implementations/encrypted/EncryptedBlockStore.h @@ -24,6 +24,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; //This function should only be used by test cases void __setKey(const typename Cipher::EncryptionKey &encKey); @@ -96,6 +97,11 @@ uint64_t EncryptedBlockStore::blockSizeFromPhysicalBlockSize(uint64_t bl return EncryptedBlock::blockSizeFromPhysicalBlockSize(_baseBlockStore->blockSizeFromPhysicalBlockSize(blockSize)); } +template +void EncryptedBlockStore::forEachBlock(std::function callback) const { + return _baseBlockStore->forEachBlock(callback); +} + } } diff --git a/src/blockstore/implementations/inmemory/InMemoryBlockStore.cpp b/src/blockstore/implementations/inmemory/InMemoryBlockStore.cpp index 18e66158..ff2a0a2e 100644 --- a/src/blockstore/implementations/inmemory/InMemoryBlockStore.cpp +++ b/src/blockstore/implementations/inmemory/InMemoryBlockStore.cpp @@ -61,5 +61,11 @@ uint64_t InMemoryBlockStore::blockSizeFromPhysicalBlockSize(uint64_t blockSize) return blockSize; } +void InMemoryBlockStore::forEachBlock(std::function callback) const { + for (const auto &entry : _blocks) { + callback(Key::FromString(entry.first)); + } +} + } } diff --git a/src/blockstore/implementations/inmemory/InMemoryBlockStore.h b/src/blockstore/implementations/inmemory/InMemoryBlockStore.h index a3334e52..63eb40b1 100644 --- a/src/blockstore/implementations/inmemory/InMemoryBlockStore.h +++ b/src/blockstore/implementations/inmemory/InMemoryBlockStore.h @@ -22,6 +22,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; private: std::unordered_map _blocks; diff --git a/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp b/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp index a3ea3bb6..1384be25 100644 --- a/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp +++ b/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp @@ -77,9 +77,9 @@ void OnDiskBlockStore::remove(unique_ref block) { uint64_t OnDiskBlockStore::numBlocks() const { uint64_t count = 0; - for (auto entry = bf::directory_iterator(_rootdir); entry != bf::directory_iterator(); ++entry) { - if (bf::is_directory(entry->path())) { - count += std::distance(bf::directory_iterator(entry->path()), bf::directory_iterator()); + for (auto prefixDir = bf::directory_iterator(_rootdir); prefixDir != bf::directory_iterator(); ++prefixDir) { + if (bf::is_directory(prefixDir->path())) { + count += std::distance(bf::directory_iterator(prefixDir->path()), bf::directory_iterator()); } } return count; @@ -95,5 +95,17 @@ uint64_t OnDiskBlockStore::blockSizeFromPhysicalBlockSize(uint64_t blockSize) co return OnDiskBlock::blockSizeFromPhysicalBlockSize(blockSize); } +void OnDiskBlockStore::forEachBlock(std::function callback) const { + for (auto prefixDir = bf::directory_iterator(_rootdir); prefixDir != bf::directory_iterator(); ++prefixDir) { + if (bf::is_directory(prefixDir->path())) { + string blockKeyPrefix = prefixDir->path().filename().native(); + for (auto block = bf::directory_iterator(prefixDir->path()); block != bf::directory_iterator(); ++block) { + string blockKeyPostfix = block->path().filename().native(); + callback(Key::FromString(blockKeyPrefix + blockKeyPostfix)); + } + } + } +} + } } diff --git a/src/blockstore/implementations/ondisk/OnDiskBlockStore.h b/src/blockstore/implementations/ondisk/OnDiskBlockStore.h index 42325d19..c59a3ae2 100644 --- a/src/blockstore/implementations/ondisk/OnDiskBlockStore.h +++ b/src/blockstore/implementations/ondisk/OnDiskBlockStore.h @@ -21,6 +21,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; private: const boost::filesystem::path _rootdir; diff --git a/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp b/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp index 53e40901..b85ec31a 100644 --- a/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp +++ b/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.cpp @@ -64,5 +64,9 @@ uint64_t ParallelAccessBlockStore::blockSizeFromPhysicalBlockSize(uint64_t block return _baseBlockStore->blockSizeFromPhysicalBlockSize(blockSize); } +void ParallelAccessBlockStore::forEachBlock(std::function callback) const { + return _baseBlockStore->forEachBlock(callback); +} + } } diff --git a/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.h b/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.h index cf23481a..6574ca9b 100644 --- a/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.h +++ b/src/blockstore/implementations/parallelaccess/ParallelAccessBlockStore.h @@ -22,6 +22,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; private: cpputils::unique_ref _baseBlockStore; diff --git a/src/blockstore/implementations/testfake/FakeBlockStore.cpp b/src/blockstore/implementations/testfake/FakeBlockStore.cpp index b001c25e..81601da6 100644 --- a/src/blockstore/implementations/testfake/FakeBlockStore.cpp +++ b/src/blockstore/implementations/testfake/FakeBlockStore.cpp @@ -84,5 +84,11 @@ uint64_t FakeBlockStore::blockSizeFromPhysicalBlockSize(uint64_t blockSize) cons return blockSize; } +void FakeBlockStore::forEachBlock(std::function callback) const { + for (const auto &entry : _blocks) { + callback(Key::FromString(entry.first)); + } +} + } } diff --git a/src/blockstore/implementations/testfake/FakeBlockStore.h b/src/blockstore/implementations/testfake/FakeBlockStore.h index 1e55b812..26991089 100644 --- a/src/blockstore/implementations/testfake/FakeBlockStore.h +++ b/src/blockstore/implementations/testfake/FakeBlockStore.h @@ -37,6 +37,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; void updateData(const Key &key, const cpputils::Data &data); diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h index f9ce7f57..317243c4 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h @@ -23,6 +23,7 @@ public: uint64_t numBlocks() const override; uint64_t estimateNumFreeBytes() const override; uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override; + void forEachBlock(std::function callback) const override; private: cpputils::unique_ref _baseBlockStore; @@ -76,6 +77,10 @@ inline uint64_t VersionCountingBlockStore::blockSizeFromPhysicalBlockSize(uint64 return VersionCountingBlock::blockSizeFromPhysicalBlockSize(_baseBlockStore->blockSizeFromPhysicalBlockSize(blockSize)); } +inline void VersionCountingBlockStore::forEachBlock(std::function callback) const { + return _baseBlockStore->forEachBlock(callback); +} + } } diff --git a/src/blockstore/interface/BlockStore.h b/src/blockstore/interface/BlockStore.h index ba5da0f9..56361579 100644 --- a/src/blockstore/interface/BlockStore.h +++ b/src/blockstore/interface/BlockStore.h @@ -29,6 +29,8 @@ public: // This can be used to create blocks with a certain physical block size. virtual uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const = 0; + virtual void forEachBlock(std::function callback) const = 0; + cpputils::unique_ref create(const cpputils::Data &data) { while(true) { //TODO Copy (data.copy()) necessary? diff --git a/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp b/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp index 2dd4cf85..a34ccf45 100644 --- a/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp +++ b/test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp @@ -32,6 +32,7 @@ public: MOCK_CONST_METHOD0(numBlocks, uint64_t()); MOCK_CONST_METHOD0(estimateNumFreeBytes, uint64_t()); MOCK_CONST_METHOD1(blockSizeFromPhysicalBlockSize, uint64_t(uint64_t)); + MOCK_CONST_METHOD1(forEachBlock, void(std::function)); }; class BlockMock: public Block { diff --git a/test/blockstore/testutils/BlockStoreTest.h b/test/blockstore/testutils/BlockStoreTest.h index abf8d5a7..de5672b5 100644 --- a/test/blockstore/testutils/BlockStoreTest.h +++ b/test/blockstore/testutils/BlockStoreTest.h @@ -3,9 +3,22 @@ #define MESSMER_BLOCKSTORE_TEST_IMPLEMENTATIONS_TESTUTILS_BLOCKSTORETEST_H_ #include +#include #include + #include "blockstore/interface/BlockStore.h" +class MockForEachBlockCallback final { +public: + std::function callback() { + return [this] (const blockstore::Key &key) { + called_with.push_back(key); + }; + } + + std::vector called_with; +}; + class BlockStoreTestFixture { public: virtual ~BlockStoreTestFixture() {} @@ -36,6 +49,24 @@ public: block = blockStore->load(key).value(); EXPECT_EQ(0, std::memcmp(fixture.data(), block->data(), fixture.size())); } + + template + void EXPECT_UNORDERED_EQ(const std::vector &expected, std::vector actual) { + EXPECT_EQ(expected.size(), actual.size()); + for (const Entry &expectedEntry : expected) { + removeOne(&actual, expectedEntry); + } + } + + template + void removeOne(std::vector *entries, const Entry &toRemove) { + auto found = std::find(entries->begin(), entries->end(), toRemove); + if (found != entries->end()) { + entries->erase(found); + return; + } + EXPECT_TRUE(false); + } }; TYPED_TEST_CASE_P(BlockStoreTest); @@ -117,11 +148,64 @@ TYPED_TEST_P(BlockStoreTest, NumBlocksIsCorrectAfterRemovingABlock) { } TYPED_TEST_P(BlockStoreTest, CanRemoveModifiedBlock) { - auto blockStore = this->fixture.createBlockStore(); - auto block = blockStore->create(cpputils::Data(5)); - block->write("data", 0, 4); - blockStore->remove(std::move(block)); - EXPECT_EQ(0u, blockStore->numBlocks()); + auto blockStore = this->fixture.createBlockStore(); + auto block = blockStore->create(cpputils::Data(5)); + block->write("data", 0, 4); + blockStore->remove(std::move(block)); + EXPECT_EQ(0u, blockStore->numBlocks()); +} + +TYPED_TEST_P(BlockStoreTest, ForEachBlock_zeroblocks) { + auto blockStore = this->fixture.createBlockStore(); + MockForEachBlockCallback mockForEachBlockCallback; + blockStore->forEachBlock(mockForEachBlockCallback.callback()); + this->EXPECT_UNORDERED_EQ({}, mockForEachBlockCallback.called_with); +} + +TYPED_TEST_P(BlockStoreTest, ForEachBlock_oneblock) { + auto blockStore = this->fixture.createBlockStore(); + auto block = blockStore->create(cpputils::Data(1)); + MockForEachBlockCallback mockForEachBlockCallback; + blockStore->forEachBlock(mockForEachBlockCallback.callback()); + this->EXPECT_UNORDERED_EQ({block->key()}, mockForEachBlockCallback.called_with); +} + +TYPED_TEST_P(BlockStoreTest, ForEachBlock_twoblocks) { + auto blockStore = this->fixture.createBlockStore(); + auto block1 = blockStore->create(cpputils::Data(1)); + auto block2 = blockStore->create(cpputils::Data(1)); + MockForEachBlockCallback mockForEachBlockCallback; + blockStore->forEachBlock(mockForEachBlockCallback.callback()); + this->EXPECT_UNORDERED_EQ({block1->key(), block2->key()}, mockForEachBlockCallback.called_with); +} + +TYPED_TEST_P(BlockStoreTest, ForEachBlock_threeblocks) { + auto blockStore = this->fixture.createBlockStore(); + auto block1 = blockStore->create(cpputils::Data(1)); + auto block2 = blockStore->create(cpputils::Data(1)); + auto block3 = blockStore->create(cpputils::Data(1)); + MockForEachBlockCallback mockForEachBlockCallback; + blockStore->forEachBlock(mockForEachBlockCallback.callback()); + this->EXPECT_UNORDERED_EQ({block1->key(), block2->key(), block3->key()}, mockForEachBlockCallback.called_with); +} + +TYPED_TEST_P(BlockStoreTest, ForEachBlock_doesntListRemovedBlocks_oneblock) { + auto blockStore = this->fixture.createBlockStore(); + auto block1 = blockStore->create(cpputils::Data(1)); + blockStore->remove(std::move(block1)); + MockForEachBlockCallback mockForEachBlockCallback; + blockStore->forEachBlock(mockForEachBlockCallback.callback()); + this->EXPECT_UNORDERED_EQ({}, mockForEachBlockCallback.called_with); +} + +TYPED_TEST_P(BlockStoreTest, ForEachBlock_doesntListRemovedBlocks_twoblocks) { + auto blockStore = this->fixture.createBlockStore(); + auto block1 = blockStore->create(cpputils::Data(1)); + auto block2 = blockStore->create(cpputils::Data(1)); + blockStore->remove(std::move(block1)); + MockForEachBlockCallback mockForEachBlockCallback; + blockStore->forEachBlock(mockForEachBlockCallback.callback()); + this->EXPECT_UNORDERED_EQ({block2->key()}, mockForEachBlockCallback.called_with); } TYPED_TEST_P(BlockStoreTest, Resize_Larger_FromZero) { @@ -211,6 +295,12 @@ REGISTER_TYPED_TEST_CASE_P(BlockStoreTest, WriteAndReadAfterLoading, OverwriteAndRead, CanRemoveModifiedBlock, + ForEachBlock_zeroblocks, + ForEachBlock_oneblock, + ForEachBlock_twoblocks, + ForEachBlock_threeblocks, + ForEachBlock_doesntListRemovedBlocks_oneblock, + ForEachBlock_doesntListRemovedBlocks_twoblocks, Resize_Larger_FromZero, Resize_Larger_FromZero_BlockIsStillUsable, Resize_Larger,