Skip to content

Commit

Permalink
feat: Don't store every block number in block indices DB (#10658)
Browse files Browse the repository at this point in the history
The block indices DB is what enables clients to query the block number
for a given index. Where there were blocks of zero size (so multiple
block numbers against an index) it was storing every block number
against that index. This could result in an unbounded values size in the
DB. We now just store the block range in this scenario.
  • Loading branch information
PhilWindle authored Dec 17, 2024
1 parent f540fbe commit a3fba84
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,7 @@ void LMDBTreeStore::write_block_index_data(const block_number_t& blockNumber,
msgpack::unpack((const char*)data.data(), data.size()).get().convert(payload);
}

// Double check it's not already present (it shouldn't be)
// We then add the block number and sort
// Sorting shouldn't be necessary as we add blocks in ascending order, but we will make sure
// Sorting here and when we unwind blocks means that looking up the block number for an index becomes O(1)
// These lookups are much more frequent than adds or deletes so we take the hit here
if (!payload.contains(blockNumber)) {
payload.blockNumbers.emplace_back(blockNumber);
payload.sort();
}
payload.add_block(blockNumber);

// Write the new payload back down
msgpack::sbuffer buffer;
Expand All @@ -189,11 +181,11 @@ bool LMDBTreeStore::find_block_for_index(const index_t& index, block_number_t& b
}
BlockIndexPayload payload;
msgpack::unpack((const char*)data.data(), data.size()).get().convert(payload);
if (payload.blockNumbers.empty()) {
if (payload.is_empty()) {
return false;
}
// The block numbers are sorted so we simply return the lowest
blockNumber = payload.blockNumbers[0];
blockNumber = payload.get_min_block_number();
return true;
}

Expand All @@ -217,7 +209,7 @@ void LMDBTreeStore::delete_block_index(const index_t& sizeAtBlock,
payload.delete_block(blockNumber);

// if it's now empty, delete it
if (payload.blockNumbers.empty()) {
if (payload.is_empty()) {
tx.delete_value(key, *_indexToBlockDatabase);
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#pragma once
#include "barretenberg/common/log.hpp"
#include "barretenberg/common/serialize.hpp"
#include "barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp"
#include "barretenberg/crypto/merkle_tree/lmdb_store/callbacks.hpp"
Expand All @@ -15,6 +16,7 @@
#include <cstdint>
#include <optional>
#include <ostream>
#include <stdexcept>
#include <string>
#include <typeinfo>
#include <unordered_map>
Expand Down Expand Up @@ -63,39 +65,90 @@ struct BlockIndexPayload {

bool operator==(const BlockIndexPayload& other) const { return blockNumbers == other.blockNumbers; }

void sort() { std::sort(blockNumbers.begin(), blockNumbers.end()); }
bool is_empty() const { return blockNumbers.empty(); }

block_number_t get_min_block_number() { return blockNumbers[0]; }

bool contains(const block_number_t& blockNumber)
{
auto it = std::lower_bound(blockNumbers.begin(), blockNumbers.end(), blockNumber);
if (it == blockNumbers.end()) {
// The block was not found, we can return
if (is_empty()) {
return false;
}
return *it == blockNumber;
if (blockNumbers.size() == 1) {
return blockNumbers[0] == blockNumber;
}
return blockNumber >= blockNumbers[0] && blockNumber <= blockNumbers[1];
}

void delete_block(const block_number_t& blockNumber)
{
// Shouldn't be possible, but no need to do anything here
if (blockNumbers.empty()) {
return;
}
// shuffle the block number down, removing the one we want to remove and then pop the end item
auto it = std::lower_bound(blockNumbers.begin(), blockNumbers.end(), blockNumber);
if (it == blockNumbers.end()) {
// The block was not found, we can return

// If the size is 1, the blocknumber must match that in index 0, if it does remove it
if (blockNumbers.size() == 1) {
if (blockNumbers[0] == blockNumber) {
blockNumbers.pop_back();
}
return;
}
// It could be a block higher than the one we are looking for
if (*it != blockNumber) {

// we have 2 entries, we must verify that the block number is equal to the item in index 1
if (blockNumbers[1] != blockNumber) {
throw std::runtime_error(format("Unable to delete block number ",
blockNumber,
" for retrieval by index, current max block number at that index: ",
blockNumbers[1]));
}
// It is equal, decrement it, we know that the new block number must have been added previously
--blockNumbers[1];

// We have modified the high block. If it is now equal to the low block then pop it
if (blockNumbers[0] == blockNumbers[1]) {
blockNumbers.pop_back();
}
}

void add_block(const block_number_t& blockNumber)
{
// If empty, just add the block number
if (blockNumbers.empty()) {
blockNumbers.emplace_back(blockNumber);
return;
}

// If the size is 1, then we must be adding the block 1 larger than that in index 0
if (blockNumbers.size() == 1) {
if (blockNumber != blockNumbers[0] + 1) {
// We can't accept a block number for this index that does not immediately follow the block before
throw std::runtime_error(format("Unable to store block number ",
blockNumber,
" for retrieval by index, current max block number at that index: ",
blockNumbers[0]));
}
blockNumbers.emplace_back(blockNumber);
return;
}
// we have found our block, shuffle blocks after this one down
auto readIt = it + 1;
while (readIt != blockNumbers.end()) {
*it++ = *readIt++;

// Size must be 2 here, if larger, this is an error
if (blockNumbers.size() != 2) {
throw std::runtime_error(format("Unable to store block number ",
blockNumber,
" for retrieval by index, block numbers is of invalid size: ",
blockNumbers.size()));
}

// If the size is 2, then we must be adding the block 1 larger than that in index 1
if (blockNumber != blockNumbers[1] + 1) {
// We can't accept a block number for this index that does not immediately follow the block before
throw std::runtime_error(format("Unable to store block number ",
blockNumber,
" for retrieval by index, current max block number at that index: ",
blockNumbers[1]));
}
blockNumbers.pop_back();
blockNumbers[1] = blockNumber;
}
};
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,10 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_by_index)
BlockAndIndex{ .blockNumber = 5, .index = 130 } };
LMDBTreeStore store(_directory, "DB1", _mapSize, _maxReaders);
{
// write all of the blocks. we will write them in reverse order
// write all of the blocks.
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
for (int i = int(blocks.size()) - 1; i >= 0; i--) {
for (auto block : blocks) {
// the arg is block size so add 1
const BlockAndIndex& block = blocks[size_t(i)];
store.write_block_index_data(block.blockNumber, block.index + 1, *transaction);
}
transaction->commit();
Expand Down Expand Up @@ -409,14 +408,24 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
{
// write all of the blocks. we will write them in reverse order
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
for (int i = int(blocks.size()) - 1; i >= 0; i--) {
for (auto block : blocks) {
// the arg is block size so add 1
const BlockAndIndex& block = blocks[size_t(i)];
store.write_block_index_data(block.blockNumber, block.index + 1, *transaction);
}
transaction->commit();
}

{
// we can't add a duplicate block at an index if it is not the next block number
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
// the arg is block size so add 1
EXPECT_THROW(store.write_block_index_data(3, 60 + 1, *transaction), std::runtime_error);
EXPECT_THROW(store.write_block_index_data(6, 60 + 1, *transaction), std::runtime_error);
EXPECT_THROW(store.write_block_index_data(1, 25 + 1, *transaction), std::runtime_error);
EXPECT_THROW(store.write_block_index_data(3, 25 + 1, *transaction), std::runtime_error);
transaction->abort();
}

{
// read back some blocks and check them
LMDBTreeReadTransaction::Ptr transaction = store.create_read_transaction();
Expand All @@ -435,11 +444,12 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
}

{
// delete block 2 at index 60
// attempting to delete block 2 at index 60 should fail as it is not the last block in the series at index 60
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
// the arg is block size so add 1
store.delete_block_index(blocks[1].index + 1, blocks[1].blockNumber, *transaction);
transaction->commit();
EXPECT_THROW(store.delete_block_index(blocks[1].index + 1, blocks[1].blockNumber, *transaction),
std::runtime_error);
transaction->abort();
}

{
Expand All @@ -449,9 +459,9 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
EXPECT_TRUE(store.find_block_for_index(5, readBack, *transaction));
EXPECT_EQ(readBack, 1);

// should be the new lowest block at this index
// should still be the lowest block at this index
EXPECT_TRUE(store.find_block_for_index(30, readBack, *transaction));
EXPECT_EQ(readBack, 3);
EXPECT_EQ(readBack, 2);

EXPECT_TRUE(store.find_block_for_index(82, readBack, *transaction));
EXPECT_EQ(readBack, 5);
Expand All @@ -463,9 +473,9 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
// try and delete blocks that don't exist at index 60
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
// the arg is block size so add 1
store.delete_block_index(blocks[1].index + 1, 2, *transaction);
store.delete_block_index(blocks[1].index + 1, 5, *transaction);
transaction->commit();
EXPECT_THROW(store.delete_block_index(blocks[1].index + 1, 2, *transaction), std::runtime_error);
EXPECT_THROW(store.delete_block_index(blocks[1].index + 1, 5, *transaction), std::runtime_error);
transaction->abort();
}

{
Expand All @@ -475,9 +485,8 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
EXPECT_TRUE(store.find_block_for_index(5, readBack, *transaction));
EXPECT_EQ(readBack, 1);

// should be the new lowest block at this index
EXPECT_TRUE(store.find_block_for_index(30, readBack, *transaction));
EXPECT_EQ(readBack, 3);
EXPECT_EQ(readBack, 2);

EXPECT_TRUE(store.find_block_for_index(82, readBack, *transaction));
EXPECT_EQ(readBack, 5);
Expand All @@ -486,14 +495,40 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
}

{
// delete 2 more blocks
// delete the last 2 blocks at index 60
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
// the arg is block size so add 1
store.delete_block_index(blocks[3].index + 1, blocks[3].blockNumber, *transaction);
store.delete_block_index(blocks[2].index + 1, blocks[2].blockNumber, *transaction);
transaction->commit();
}

{
// check the blocks again
LMDBTreeReadTransaction::Ptr transaction = store.create_read_transaction();
block_number_t readBack = 0;
EXPECT_TRUE(store.find_block_for_index(5, readBack, *transaction));
EXPECT_EQ(readBack, 1);

EXPECT_TRUE(store.find_block_for_index(30, readBack, *transaction));
EXPECT_EQ(readBack, 2);

EXPECT_TRUE(store.find_block_for_index(82, readBack, *transaction));
EXPECT_EQ(readBack, 5);
}

{
// delete the last final block at index 60
LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
// the arg is block size so add 1
// Only one block remains at index 60, try and delete one that doesn't exist, it should do nothing
store.delete_block_index(blocks[3].index + 1, blocks[3].blockNumber, *transaction);

// Now delete the last block
store.delete_block_index(blocks[1].index + 1, blocks[1].blockNumber, *transaction);
transaction->commit();
}

{
// check the blocks again
LMDBTreeReadTransaction::Ptr transaction = store.create_read_transaction();
Expand Down

0 comments on commit a3fba84

Please sign in to comment.