Fix CachingBlockStore2 and add test case for it

This commit is contained in:
Sebastian Messmer 2017-08-21 23:09:43 +01:00
parent 4add7f3d80
commit 509bf6cc26
2 changed files with 28 additions and 9 deletions

View File

@ -54,11 +54,11 @@ CachingBlockStore2::CachingBlockStore2(cpputils::unique_ref<BlockStore2> baseBlo
} }
bool CachingBlockStore2::tryCreate(const Key &key, const Data &data) { bool CachingBlockStore2::tryCreate(const Key &key, const Data &data) {
//TODO Check if block exists in base store //TODO Check if block exists in base store? Performance hit? It's very unlikely it exists.
auto popped = _cache.pop(key); auto popped = _cache.pop(key);
if (popped != boost::none) { if (popped != boost::none) {
// entry already exists in cache // entry already exists in cache
_cache.push(key, std::move(*popped)); _cache.push(key, std::move(*popped)); // push the just popped element back to the cache
return false; return false;
} else { } else {
_cache.push(key, make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, data.copy(), true)); _cache.push(key, make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, data.copy(), true));
@ -72,7 +72,8 @@ bool CachingBlockStore2::remove(const Key &key) {
// TODO Don't write-through but cache remove operations // TODO Don't write-through but cache remove operations
auto popped = _cache.pop(key); auto popped = _cache.pop(key);
if (popped != boost::none) { if (popped != boost::none) {
return std::move(**popped).remove(); std::move(**popped).remove();
return true;
} else { } else {
return _baseBlockStore->remove(key); return _baseBlockStore->remove(key);
} }
@ -107,8 +108,10 @@ void CachingBlockStore2::store(const Key &key, const Data &data) {
if (popped != boost::none) { if (popped != boost::none) {
(*popped)->write(data.copy()); (*popped)->write(data.copy());
} else { } else {
popped = make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, data.copy(), true); popped = make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, data.copy(), false);
// TODO If block doesn't exist in base store, we have to add it to _cachedBlocksNotInBaseStore // TODO Instead of storing it to the base store, we could just keep it dirty in the cache
// and (if it doesn't exist in base store yet) add it to _cachedBlocksNotInBaseStore
_baseBlockStore->store(key, data);
} }
_cache.push(key, std::move(*popped)); _cache.push(key, std::move(*popped));
} }

View File

@ -288,7 +288,7 @@ TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectOnEmptyBlockstore) {
EXPECT_EQ(0u, blockStore->numBlocks()); EXPECT_EQ(0u, blockStore->numBlocks());
} }
TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterAddingOneBlock) { TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterCreatingOneBlock) {
auto blockStore = this->fixture.createBlockStore(); auto blockStore = this->fixture.createBlockStore();
blockStore->create(cpputils::Data(1)); blockStore->create(cpputils::Data(1));
EXPECT_EQ(1u, blockStore->numBlocks()); EXPECT_EQ(1u, blockStore->numBlocks());
@ -301,7 +301,7 @@ TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterRemovingTheLastBlock) {
EXPECT_EQ(0u, blockStore->numBlocks()); EXPECT_EQ(0u, blockStore->numBlocks());
} }
TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterAddingTwoBlocks) { TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterCreatingTwoBlocks) {
auto blockStore = this->fixture.createBlockStore(); auto blockStore = this->fixture.createBlockStore();
blockStore->create(cpputils::Data(1)); blockStore->create(cpputils::Data(1));
blockStore->create(cpputils::Data(0)); blockStore->create(cpputils::Data(0));
@ -316,6 +316,20 @@ TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterRemovingABlock) {
EXPECT_EQ(1u, blockStore->numBlocks()); EXPECT_EQ(1u, blockStore->numBlocks());
} }
TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterStoringANewBlock) {
const blockstore::Key key = blockstore::Key::FromString("1491BB4932A389EE14BC7090AC772972");
auto blockStore = this->fixture.createBlockStore();
blockStore->store(key, cpputils::Data(1));
EXPECT_EQ(1u, blockStore->numBlocks());
}
TYPED_TEST_P(BlockStore2Test, NumBlocksIsCorrectAfterStoringAnExistingBlock) {
auto blockStore = this->fixture.createBlockStore();
blockstore::Key key = blockStore->create(cpputils::Data(1));
blockStore->store(key, cpputils::Data(1));
EXPECT_EQ(1u, blockStore->numBlocks());
}
TYPED_TEST_P(BlockStore2Test, ForEachBlock_zeroblocks) { TYPED_TEST_P(BlockStore2Test, ForEachBlock_zeroblocks) {
auto blockStore = this->fixture.createBlockStore(); auto blockStore = this->fixture.createBlockStore();
MockForEachBlockCallback mockForEachBlockCallback; MockForEachBlockCallback mockForEachBlockCallback;
@ -405,10 +419,12 @@ REGISTER_TYPED_TEST_CASE_P(BlockStore2Test,
givenEmptyBlockStore_whenLoadingNonExistingBlock_thenFails, givenEmptyBlockStore_whenLoadingNonExistingBlock_thenFails,
givenNonEmptyBlockStore_whenLoadingNonExistingBlock_thenFails, givenNonEmptyBlockStore_whenLoadingNonExistingBlock_thenFails,
NumBlocksIsCorrectOnEmptyBlockstore, NumBlocksIsCorrectOnEmptyBlockstore,
NumBlocksIsCorrectAfterAddingOneBlock, NumBlocksIsCorrectAfterCreatingOneBlock,
NumBlocksIsCorrectAfterRemovingTheLastBlock, NumBlocksIsCorrectAfterRemovingTheLastBlock,
NumBlocksIsCorrectAfterAddingTwoBlocks, NumBlocksIsCorrectAfterCreatingTwoBlocks,
NumBlocksIsCorrectAfterRemovingABlock, NumBlocksIsCorrectAfterRemovingABlock,
NumBlocksIsCorrectAfterStoringANewBlock,
NumBlocksIsCorrectAfterStoringAnExistingBlock,
ForEachBlock_zeroblocks, ForEachBlock_zeroblocks,
ForEachBlock_oneblock, ForEachBlock_oneblock,
ForEachBlock_twoblocks, ForEachBlock_twoblocks,