Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

snapshots: index builder refactorings #1922

Merged
merged 10 commits into from
Mar 26, 2024
Merged
Prev Previous commit
Next Next commit
rename IndexBuilder, TransactionIndex1
battlmonstr committed Mar 25, 2024
commit aa5bc0e0390e76005b22eac420fed689e7ce0d20
6 changes: 3 additions & 3 deletions cmd/dev/snapshots.cpp
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@
#include <silkworm/db/snapshots/bittorrent/client.hpp>
#include <silkworm/db/snapshots/body_index.hpp>
#include <silkworm/db/snapshots/header_index.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/repository.hpp>
#include <silkworm/db/snapshots/seg/seg_zip.hpp>
#include <silkworm/db/snapshots/snapshot.hpp>
@@ -297,8 +297,8 @@ void create_index(const SnapSettings& settings, int repetitions) {
break;
}
case SnapshotType::transactions: {
auto bodies_segment_path = TransactionIndex1::bodies_segment_path(*snap_file);
auto index = TransactionIndex1::make(bodies_segment_path, *snap_file);
auto bodies_segment_path = TransactionIndex::bodies_segment_path(*snap_file);
auto index = TransactionIndex::make(bodies_segment_path, *snap_file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could merge these two lines into just one:

auto index = TransactionIndex::make(*snap_file);

looking exactly similar to the HeaderIndex and BodyIndex above, either by refactoring or overloading the current TransactionIndex::make. Next PR is fine

index.build();

auto index_hash_to_block = TransactionToBlockIndex::make(bodies_segment_path, *snap_file);
14 changes: 7 additions & 7 deletions silkworm/capi/silkworm.cpp
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@
#include <silkworm/db/buffer.hpp>
#include <silkworm/db/snapshots/body_index.hpp>
#include <silkworm/db/snapshots/header_index.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/txn_index.hpp>
#include <silkworm/db/snapshots/txn_to_block_index.hpp>
#include <silkworm/db/stages.hpp>
@@ -236,7 +236,7 @@ SILKWORM_EXPORT int silkworm_build_recsplit_indexes(SilkwormHandle handle, struc
return SILKWORM_INVALID_HANDLE;
}

std::vector<std::shared_ptr<snapshots::Index>> needed_indexes;
std::vector<std::shared_ptr<snapshots::IndexBuilder>> needed_indexes;
for (size_t i = 0; i < len; i++) {
struct SilkwormMemoryMappedFile* snapshot = snapshots[i];
if (!snapshot) {
@@ -249,26 +249,26 @@ SILKWORM_EXPORT int silkworm_build_recsplit_indexes(SilkwormHandle handle, struc
return SILKWORM_INVALID_PATH;
}

std::shared_ptr<snapshots::Index> index;
std::shared_ptr<snapshots::IndexBuilder> index;
switch (snapshot_path->type()) {
case snapshots::SnapshotType::headers: {
index = std::make_shared<snapshots::Index>(snapshots::HeaderIndex::make(*snapshot_path, snapshot_region));
index = std::make_shared<snapshots::IndexBuilder>(snapshots::HeaderIndex::make(*snapshot_path, snapshot_region));
needed_indexes.push_back(index);
break;
}
case snapshots::SnapshotType::bodies: {
index = std::make_shared<snapshots::Index>(snapshots::BodyIndex::make(*snapshot_path, snapshot_region));
index = std::make_shared<snapshots::IndexBuilder>(snapshots::BodyIndex::make(*snapshot_path, snapshot_region));
needed_indexes.push_back(index);
break;
}
case snapshots::SnapshotType::transactions: {
auto bodies_segment_path = snapshots::TransactionIndex1::bodies_segment_path(*snapshot_path);
auto bodies_segment_path = snapshots::TransactionIndex::bodies_segment_path(*snapshot_path);
auto bodies_file = std::find_if(snapshots, snapshots + len, [&](SilkwormMemoryMappedFile* file) -> bool {
return snapshots::SnapshotPath::parse(file->file_path) == bodies_segment_path;
});

if (bodies_file < snapshots + len) {
index = std::make_shared<snapshots::Index>(snapshots::TransactionIndex1::make(bodies_segment_path, *snapshot_path));
index = std::make_shared<snapshots::IndexBuilder>(snapshots::TransactionIndex::make(bodies_segment_path, *snapshot_path));
needed_indexes.push_back(index);

index = std::make_shared<snapshots::IndexBuilder>(snapshots::TransactionToBlockIndex::make(bodies_segment_path, *snapshot_path));
4 changes: 2 additions & 2 deletions silkworm/capi/silkworm_test.cpp
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@
#include <silkworm/db/mdbx/mdbx.hpp>
#include <silkworm/db/snapshots/body_index.hpp>
#include <silkworm/db/snapshots/header_index.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/snapshot.hpp>
#include <silkworm/db/snapshots/test_util/common.hpp>
#include <silkworm/db/snapshots/txn_index.hpp>
@@ -658,7 +658,7 @@ TEST_CASE_METHOD(CApiTest, "CAPI silkworm_add_snapshot", "[silkworm][capi]") {
body_snapshot.reopen_segment();
body_snapshot.reopen_index();

auto tx_index = snapshots::TransactionIndex1::make(body_snapshot_path, tx_snapshot_path);
auto tx_index = snapshots::TransactionIndex::make(body_snapshot_path, tx_snapshot_path);
tx_index.build();
auto tx_index_hash_to_block = snapshots::TransactionToBlockIndex::make(body_snapshot_path, tx_snapshot_path);
tx_index_hash_to_block.build();
2 changes: 1 addition & 1 deletion silkworm/db/snapshot_sync.cpp
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@
#include <silkworm/core/types/hash.hpp>
#include <silkworm/db/mdbx/etl_mdbx_collector.hpp>
#include <silkworm/db/snapshots/config.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/path.hpp>
#include <silkworm/db/stages.hpp>
#include <silkworm/infra/common/ensure.hpp>
2 changes: 1 addition & 1 deletion silkworm/db/snapshots/body_index.hpp
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@
#include <silkworm/core/common/bytes.hpp>
#include <silkworm/infra/common/memory_mapped_file.hpp>

#include "index.hpp"
#include "index_builder.hpp"
#include "path.hpp"

namespace silkworm::snapshots {
2 changes: 1 addition & 1 deletion silkworm/db/snapshots/header_index.hpp
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@
#include <silkworm/core/common/bytes.hpp>
#include <silkworm/infra/common/memory_mapped_file.hpp>

#include "index.hpp"
#include "index_builder.hpp"
#include "path.hpp"

namespace silkworm::snapshots {
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
limitations under the License.
*/

#include "index.hpp"
#include "index_builder.hpp"

#include <silkworm/db/snapshots/rec_split/rec_split.hpp>
#include <silkworm/db/snapshots/rec_split/rec_split_seq.hpp>
Original file line number Diff line number Diff line change
@@ -130,7 +130,4 @@ struct IndexBuilder {
std::unique_ptr<IndexInputDataQuery> query_;
};

// TODO: remove
using Index = IndexBuilder;

} // namespace silkworm::snapshots
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
limitations under the License.
*/

#include "index.hpp"
#include "index_builder.hpp"

#include <catch2/catch.hpp>

@@ -60,7 +60,7 @@ TEST_CASE("TransactionIndex::build KO: empty snapshot", "[silkworm][snapshot][in
auto txs_snapshot_path = *SnapshotPath::parse(txs_snapshot_file.path());
auto bodies_snapshot_path = *SnapshotPath::parse(bodies_snapshot_file.path());

CHECK_THROWS_WITH(TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path).build(), StartsWith("empty body snapshot"));
CHECK_THROWS_WITH(TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), StartsWith("empty body snapshot"));
CHECK_THROWS_WITH(TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), StartsWith("empty body snapshot"));
}
}
@@ -84,7 +84,7 @@ TEST_CASE("TransactionIndex::build KO: invalid snapshot", "[silkworm][snapshot][
auto txs_snapshot_path = *SnapshotPath::parse(txs_snapshot_file.path());
auto bodies_snapshot_path = *SnapshotPath::parse(bodies_snapshot_file.path());

CHECK_THROWS_WITH(TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path).build(), StartsWith("invalid zero word length"));
CHECK_THROWS_WITH(TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), StartsWith("invalid zero word length"));
CHECK_THROWS_WITH(TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), StartsWith("invalid zero word length"));
}

@@ -100,7 +100,7 @@ TEST_CASE("TransactionIndex::build KO: invalid snapshot", "[silkworm][snapshot][
test::SampleTransactionSnapshotFile valid_txs_snapshot{};
test::SampleTransactionSnapshotPath txs_snapshot_path{valid_txs_snapshot.path()}; // necessary to tweak the block numbers

CHECK_THROWS_WITH(TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position depth"));
CHECK_THROWS_WITH(TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position depth"));
CHECK_THROWS_WITH(TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position depth"));
}

@@ -116,7 +116,7 @@ TEST_CASE("TransactionIndex::build KO: invalid snapshot", "[silkworm][snapshot][
test::SampleTransactionSnapshotFile valid_txs_snapshot{};
test::SampleTransactionSnapshotPath txs_snapshot_path{valid_txs_snapshot.path()}; // necessary to tweak the block numbers

CHECK_THROWS_WITH(TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position read"));
CHECK_THROWS_WITH(TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position read"));
CHECK_THROWS_WITH(TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position read"));
}

@@ -132,7 +132,7 @@ TEST_CASE("TransactionIndex::build KO: invalid snapshot", "[silkworm][snapshot][
test::SampleTransactionSnapshotFile valid_txs_snapshot{};
test::SampleTransactionSnapshotPath txs_snapshot_path{valid_txs_snapshot.path()}; // necessary to tweak the block numbers

CHECK_THROWS_WITH(TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position read"));
CHECK_THROWS_WITH(TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position read"));
CHECK_THROWS_WITH(TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), Contains("invalid: position read"));
}

@@ -148,7 +148,7 @@ TEST_CASE("TransactionIndex::build KO: invalid snapshot", "[silkworm][snapshot][
test::SampleTransactionSnapshotFile valid_txs_snapshot{};
test::SampleTransactionSnapshotPath txs_snapshot_path{valid_txs_snapshot.path()}; // necessary to tweak the block numbers

CHECK_THROWS_AS(TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path).build(), DecodingException);
CHECK_THROWS_AS(TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), DecodingException);
CHECK_THROWS_AS(TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path).build(), DecodingException);
}

@@ -172,7 +172,7 @@ TEST_CASE("TransactionIndex::build KO: invalid snapshot", "[silkworm][snapshot][
};
test::SampleTransactionSnapshotPath txs_snapshot_path{invalid_txs_snapshot.path()}; // necessary to tweak the block numbers

auto tx_index = TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path);
auto tx_index = TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path);
CHECK_THROWS_WITH(tx_index.build(), StartsWith("keys expected"));
auto tx_index_hash_to_block = TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path);
CHECK_THROWS_WITH(tx_index_hash_to_block.build(), Contains("tx count mismatch"));
@@ -186,7 +186,7 @@ TEST_CASE("TransactionIndex::build OK", "[silkworm][snapshot][index]") {
test::SampleTransactionSnapshotFile valid_txs_snapshot{};
test::SampleTransactionSnapshotPath txs_snapshot_path{valid_txs_snapshot.path()}; // necessary to tweak the block numbers

auto tx_index = TransactionIndex1::make(bodies_snapshot_path, txs_snapshot_path);
auto tx_index = TransactionIndex::make(bodies_snapshot_path, txs_snapshot_path);
tx_index.build();
auto tx_index_hash_to_block = TransactionToBlockIndex::make(bodies_snapshot_path, txs_snapshot_path);
tx_index_hash_to_block.build();
10 changes: 5 additions & 5 deletions silkworm/db/snapshots/repository.cpp
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@
#include <silkworm/core/common/assert.hpp>
#include <silkworm/db/snapshots/body_index.hpp>
#include <silkworm/db/snapshots/header_index.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/txn_index.hpp>
#include <silkworm/db/snapshots/txn_to_block_index.hpp>
#include <silkworm/infra/common/ensure.hpp>
@@ -203,13 +203,13 @@ std::optional<BlockNum> SnapshotRepository::find_block_number(Hash txn_hash) con

std::vector<std::shared_ptr<IndexBuilder>> SnapshotRepository::missing_indexes() const {
SnapshotPathList segment_files = get_segment_files();
std::vector<std::shared_ptr<Index>> missing_index_list;
std::vector<std::shared_ptr<IndexBuilder>> missing_index_list;
missing_index_list.reserve(segment_files.size());
for (const auto& seg_file : segment_files) {
const auto index_file = seg_file.index_file();
SILK_TRACE << "Segment file: " << seg_file.filename() << " has index: " << index_file.filename();
if (!std::filesystem::exists(index_file.path())) {
std::shared_ptr<Index> index;
std::shared_ptr<IndexBuilder> index;
switch (seg_file.type()) {
case SnapshotType::headers: {
index = std::make_shared<IndexBuilder>(HeaderIndex::make(seg_file));
@@ -222,9 +222,9 @@ std::vector<std::shared_ptr<IndexBuilder>> SnapshotRepository::missing_indexes()
break;
}
case SnapshotType::transactions: {
auto bodies_segment_path = TransactionIndex1::bodies_segment_path(seg_file);
auto bodies_segment_path = TransactionIndex::bodies_segment_path(seg_file);
if (std::find(segment_files.begin(), segment_files.end(), bodies_segment_path) != segment_files.end()) {
index = std::make_shared<IndexBuilder>(TransactionIndex1::make(bodies_segment_path, seg_file));
index = std::make_shared<IndexBuilder>(TransactionIndex::make(bodies_segment_path, seg_file));
missing_index_list.push_back(index);

index = std::make_shared<IndexBuilder>(TransactionToBlockIndex::make(bodies_segment_path, seg_file));
6 changes: 3 additions & 3 deletions silkworm/db/snapshots/repository_test.cpp
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@

#include <silkworm/db/snapshots/body_index.hpp>
#include <silkworm/db/snapshots/header_index.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/test_util/common.hpp>
#include <silkworm/db/snapshots/txn_index.hpp>
#include <silkworm/db/snapshots/txn_to_block_index.hpp>
@@ -181,7 +181,7 @@ TEST_CASE("SnapshotRepository::find_segment", "[silkworm][node][snapshot]") {
auto body_index = BodyIndex::make(body_snapshot_path);
REQUIRE_NOTHROW(body_index.build());
test::SampleTransactionSnapshotPath txn_snapshot_path{txn_snapshot.path()}; // necessary to tweak the block numbers
REQUIRE_NOTHROW(TransactionIndex1::make(body_snapshot_path, txn_snapshot_path).build());
REQUIRE_NOTHROW(TransactionIndex::make(body_snapshot_path, txn_snapshot_path).build());
REQUIRE_NOTHROW(TransactionToBlockIndex::make(body_snapshot_path, txn_snapshot_path).build());

REQUIRE_NOTHROW(repository.reopen_folder());
@@ -229,7 +229,7 @@ TEST_CASE("SnapshotRepository::find_block_number", "[silkworm][node][snapshot]")
auto body_index = BodyIndex::make(body_snapshot_path);
REQUIRE_NOTHROW(body_index.build());
test::SampleTransactionSnapshotPath txn_snapshot_path{txn_snapshot.path()}; // necessary to tweak the block numbers
REQUIRE_NOTHROW(TransactionIndex1::make(body_snapshot_path, txn_snapshot_path).build());
REQUIRE_NOTHROW(TransactionIndex::make(body_snapshot_path, txn_snapshot_path).build());
REQUIRE_NOTHROW(TransactionToBlockIndex::make(body_snapshot_path, txn_snapshot_path).build());

REQUIRE_NOTHROW(repository.reopen_folder());
6 changes: 3 additions & 3 deletions silkworm/db/snapshots/snapshot_benchmark.cpp
Original file line number Diff line number Diff line change
@@ -19,7 +19,7 @@
#include <silkworm/core/common/util.hpp>
#include <silkworm/db/snapshots/body_index.hpp>
#include <silkworm/db/snapshots/header_index.hpp>
#include <silkworm/db/snapshots/index.hpp>
#include <silkworm/db/snapshots/index_builder.hpp>
#include <silkworm/db/snapshots/seg/decompressor.hpp>
#include <silkworm/db/snapshots/test_util/common.hpp>
#include <silkworm/db/snapshots/txn_index.hpp>
@@ -123,7 +123,7 @@ static void build_tx_index(benchmark::State& state) {
body_index.build();

test::SampleTransactionSnapshotPath txn_snapshot_path{txn_snapshot.path()}; // necessary to tweak the block numbers
auto tx_index = TransactionIndex1::make(body_snapshot_path, txn_snapshot_path);
auto tx_index = TransactionIndex::make(body_snapshot_path, txn_snapshot_path);
tx_index.build();
auto tx_index_hash_to_block = TransactionToBlockIndex::make(body_snapshot_path, txn_snapshot_path);
tx_index_hash_to_block.build();
@@ -153,7 +153,7 @@ static void reopen_folder(benchmark::State& state) {
body_index.build();

test::SampleTransactionSnapshotPath txn_snapshot_path{txn_snapshot.path()}; // necessary to tweak the block numbers
auto tx_index = TransactionIndex1::make(body_snapshot_path, txn_snapshot_path);
auto tx_index = TransactionIndex::make(body_snapshot_path, txn_snapshot_path);
tx_index.build();
auto tx_index_hash_to_block = TransactionToBlockIndex::make(body_snapshot_path, txn_snapshot_path);
tx_index_hash_to_block.build();
Loading