From c3d543619e84b7acc9f35159de09a6c1812050b1 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 11 Dec 2014 00:20:23 +0100 Subject: [PATCH] When shrinking a data node, the old space is overwritten with zeroes --- .../onblocks/impl/DataLeafNode.cpp | 9 +++ .../onblocks/impl/DataLeafNode.h | 2 + .../onblocks/impl/DataLeafNodeTest.cpp | 70 +++++++++++++++---- 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/blobstore/implementations/onblocks/impl/DataLeafNode.cpp b/src/blobstore/implementations/onblocks/impl/DataLeafNode.cpp index 2ec4897d..a26985e2 100644 --- a/src/blobstore/implementations/onblocks/impl/DataLeafNode.cpp +++ b/src/blobstore/implementations/onblocks/impl/DataLeafNode.cpp @@ -30,6 +30,11 @@ void DataLeafNode::write(off_t offset, size_t count, const Data &data) { void DataLeafNode::InitializeNewNode() { *_node.MagicNumber() = _node.magicNumberLeaf; *_node.Size() = 0; + //fillDataWithZeroes(); not needed, because a newly created block will be zeroed out. DataLeafNodeTest.SpaceIsZeroFilledWhenGrowing ensures this. +} + +void DataLeafNode::fillDataWithZeroesFromTo(off_t begin, off_t end) { + std::memset(_node.DataBegin()+begin, 0, end-begin); } uint64_t DataLeafNode::numBytesInThisNode() { @@ -39,6 +44,10 @@ uint64_t DataLeafNode::numBytesInThisNode() { void DataLeafNode::resize(uint64_t newsize_bytes) { assert(newsize_bytes <= MAX_STORED_BYTES); + if (newsize_bytes < *_node.Size()) { + fillDataWithZeroesFromTo(newsize_bytes, *_node.Size()); + } + *_node.Size() = newsize_bytes; } diff --git a/src/blobstore/implementations/onblocks/impl/DataLeafNode.h b/src/blobstore/implementations/onblocks/impl/DataLeafNode.h index 62034b71..e6991255 100644 --- a/src/blobstore/implementations/onblocks/impl/DataLeafNode.h +++ b/src/blobstore/implementations/onblocks/impl/DataLeafNode.h @@ -22,6 +22,8 @@ public: uint64_t numBytesInThisNode() override; void resize(uint64_t newsize_bytes) override; +private: + void fillDataWithZeroesFromTo(off_t begin, off_t end); }; } diff --git a/src/test/blobstore/implementations/onblocks/impl/DataLeafNodeTest.cpp b/src/test/blobstore/implementations/onblocks/impl/DataLeafNodeTest.cpp index 5d912ce2..4fcc9316 100644 --- a/src/test/blobstore/implementations/onblocks/impl/DataLeafNodeTest.cpp +++ b/src/test/blobstore/implementations/onblocks/impl/DataLeafNodeTest.cpp @@ -14,6 +14,7 @@ using std::make_unique; using std::string; using blockstore::BlockStore; +using blockstore::BlockWithKey; using blockstore::Data; using blockstore::inmemory::InMemoryBlockStore; using namespace blobstore; @@ -23,9 +24,18 @@ using namespace blobstore::onblocks; class DataLeafNodeTest: public Test { public: - unique_ptr blockStore = make_unique(); - DataLeafNodeTest(): randomData(DataLeafNode::MAX_STORED_BYTES) { + DataLeafNodeTest(): + ZEROES(DataLeafNode::MAX_STORED_BYTES), + randomData(DataLeafNode::MAX_STORED_BYTES), + blockStore(make_unique()), + block(blockStore->create(DataNodeView::BLOCKSIZE_BYTES)), + leafblock(blockStore->create(DataNodeView::BLOCKSIZE_BYTES)), + leafblockdata((uint8_t*)leafblock.block->data()), + leaf(DataNode::createNewLeafNode(std::move(leafblock.block))) { + + ZEROES.FillWithZeroes(); + DataBlockFixture dataFixture(DataLeafNode::MAX_STORED_BYTES); std::memcpy(randomData.data(), dataFixture.data(), randomData.size()); @@ -39,18 +49,27 @@ public: return block.key; } - void ReadDataFromLeafBlock(Key key, Data *data) { + void FillLeafBlockWithData() { + leaf->resize(randomData.size()); + leaf->write(0, randomData.size(), randomData); + } + + void ReadDataFromLoadedLeafBlock(Key key, Data *data) { auto leaf = DataNode::load(blockStore->load(key)); EXPECT_IS_PTR_TYPE(DataLeafNode, leaf.get()); leaf->read(0, data->size(), data); } + Data ZEROES; Data randomData; + unique_ptr blockStore; + BlockWithKey block; + BlockWithKey leafblock; + const uint8_t *leafblockdata; + unique_ptr leaf; }; TEST_F(DataLeafNodeTest, ReadWrittenDataImmediately) { - auto block = blockStore->create(DataNodeView::BLOCKSIZE_BYTES); - auto leaf = DataNode::createNewLeafNode(std::move(block.block)); leaf->resize(randomData.size()); leaf->write(0, randomData.size(), randomData); @@ -63,19 +82,16 @@ TEST_F(DataLeafNodeTest, ReadWrittenDataAfterReloadingBLock) { Key key = WriteDataToNewLeafBlockAndReturnKey(); Data data(DataLeafNode::MAX_STORED_BYTES); - ReadDataFromLeafBlock(key, &data); + ReadDataFromLoadedLeafBlock(key, &data); EXPECT_EQ(0, std::memcmp(randomData.data(), data.data(), randomData.size())); } TEST_F(DataLeafNodeTest, NewLeafNodeHasSizeZero) { - auto block = blockStore->create(DataNodeView::BLOCKSIZE_BYTES); - auto leaf = DataNode::createNewLeafNode(std::move(block.block)); EXPECT_EQ(0u, leaf->numBytesInThisNode()); } TEST_F(DataLeafNodeTest, NewLeafNodeHasSizeZero_AfterLoading) { - auto block = blockStore->create(DataNodeView::BLOCKSIZE_BYTES); { DataNode::createNewLeafNode(std::move(block.block)); } @@ -88,14 +104,11 @@ class DataLeafNodeSizeTest: public DataLeafNodeTest, public WithParamInterfacecreate(DataNodeView::BLOCKSIZE_BYTES); - auto leaf = DataNode::createNewLeafNode(std::move(block.block)); leaf->resize(GetParam()); EXPECT_EQ(GetParam(), leaf->numBytesInThisNode()); } TEST_P(DataLeafNodeSizeTest, ResizeNode_ReadSizeAfterLoading) { - auto block = blockStore->create(DataNodeView::BLOCKSIZE_BYTES); { auto leaf = DataNode::createNewLeafNode(std::move(block.block)); leaf->resize(GetParam()); @@ -104,5 +117,36 @@ TEST_P(DataLeafNodeSizeTest, ResizeNode_ReadSizeAfterLoading) { EXPECT_EQ(GetParam(), leaf->numBytesInThisNode()); } +TEST_F(DataLeafNodeTest, SpaceIsZeroFilledWhenGrowing) { + leaf->resize(randomData.size()); + + Data read(randomData.size()); + leaf->read(0, read.size(), &read); + EXPECT_EQ(0, std::memcmp(ZEROES.data(), read.data(), read.size())); +} + +TEST_F(DataLeafNodeTest, SpaceGetsZeroFilledWhenShrinkingAndRegrowing) { + FillLeafBlockWithData(); + // resize it smaller and then back to original size + uint32_t smaller_size = randomData.size() - 100; + leaf->resize(smaller_size); + leaf->resize(randomData.size()); + + //Check that the space was filled with zeroes + Data read(100); + leaf->read(smaller_size, read.size(), &read); + EXPECT_EQ(0, std::memcmp(ZEROES.data(), read.data(), read.size())); +} + +TEST_F(DataLeafNodeTest, DataGetsZeroFilledWhenShrinking) { + FillLeafBlockWithData(); + uint32_t smaller_size = randomData.size() - 100; + //At first, we expect there to be random data in the underlying data block + EXPECT_EQ(0, std::memcmp((char*)randomData.data()+smaller_size, leafblockdata+DataNodeView::HEADERSIZE_BYTES+smaller_size, 100)); + + //After shrinking, we expect there to be zeroes in the underlying data block + leaf->resize(smaller_size); + EXPECT_EQ(0, std::memcmp(ZEROES.data(), leafblockdata+DataNodeView::HEADERSIZE_BYTES+smaller_size, 100)); +} + //TODO Write tests that only read part of the data -//TODO Test numBytesInThisNode()