From 86257016956ce98d385977aa9ed44e87bf938bbf Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Mon, 17 May 2021 15:47:21 -0400 Subject: [PATCH 1/8] Handle trim_blocklog_front special case --- libraries/chain/block_log.cpp | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 822de1ea8b8..4eee1c39abe 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -1036,18 +1036,40 @@ namespace eosio { namespace chain { const auto num_read = fread(buf, read_size, 1, original_block_log.blk_in); EOS_ASSERT( num_read == 1, block_log_exception, "blocks.log read failed" ); + // handle the case that the distance between original_pos and the end of the buf is less than 8 bytes + uint64_t complete_pos_content; + bool re_write = false; + uint64_t re_write_offset; + // walk this memory section to adjust block position to match the adjusted location // of the block start and store in the new index file while(original_pos >= start_of_blk_buffer_pos) { const auto buffer_index = original_pos - start_of_blk_buffer_pos; - uint64_t& pos_content = *(uint64_t*)(buf + buffer_index); - const auto start_of_this_block = pos_content; - pos_content = start_of_this_block - pos_delta; - index.write(pos_content); - original_pos = start_of_this_block - pos_size; + if (buffer_index + pos_size <= detail::reverse_iterator::_buf_len){ + uint64_t& pos_content = *(uint64_t*)(buf + buffer_index); + const auto start_of_this_block = pos_content; + pos_content = start_of_this_block - pos_delta; + index.write(pos_content); + original_pos = start_of_this_block - pos_size; + }else{ + re_write = true; + re_write_offset = detail::reverse_iterator::_buf_len - buffer_index; + // read pos_content from original log file instead of buf + status = fseek(original_block_log.blk_in, original_pos, SEEK_SET); + const auto num_read = fread(&complete_pos_content, sizeof(uint64_t), 1, original_block_log.blk_in); + EOS_ASSERT( num_read == 1, block_log_exception, "blocks.log read failed" ); + index.write(complete_pos_content - pos_delta); + original_pos = complete_pos_content - pos_size; + } } new_block_file.seek(new_block_file_first_block_pos + to_write_remaining - read_size); new_block_file.write(buf, read_size); + if (re_write){ + re_write = false; + new_block_file.seek((new_block_file_first_block_pos + to_write_remaining - read_size) + read_size - re_write_offset); + uint64_t pos = complete_pos_content - pos_delta; + new_block_file.write((char*)&pos, sizeof(uint64_t)); + } } index.complete(); fclose(original_block_log.blk_in); From 129b0b3722c9b91b67b3d2f28d31f9394bd07b87 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Mon, 17 May 2021 16:15:53 -0400 Subject: [PATCH 2/8] Update comments --- libraries/chain/block_log.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 4eee1c39abe..f902529dd99 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -1036,7 +1036,7 @@ namespace eosio { namespace chain { const auto num_read = fread(buf, read_size, 1, original_block_log.blk_in); EOS_ASSERT( num_read == 1, block_log_exception, "blocks.log read failed" ); - // handle the case that the distance between original_pos and the end of the buf is less than 8 bytes + // handle the case that the distance between `buf+buffer_index` and `the end of the buffer` is less than 8 bytes uint64_t complete_pos_content; bool re_write = false; uint64_t re_write_offset; @@ -1054,7 +1054,7 @@ namespace eosio { namespace chain { }else{ re_write = true; re_write_offset = detail::reverse_iterator::_buf_len - buffer_index; - // read pos_content from original log file instead of buf + // read pos_content from original log file instead of the buffer status = fseek(original_block_log.blk_in, original_pos, SEEK_SET); const auto num_read = fread(&complete_pos_content, sizeof(uint64_t), 1, original_block_log.blk_in); EOS_ASSERT( num_read == 1, block_log_exception, "blocks.log read failed" ); From db07176309fb0843171ba778c6fbf50115ba4581 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Mon, 30 Aug 2021 15:51:31 -0400 Subject: [PATCH 3/8] Simplify the while loop code --- libraries/chain/block_log.cpp | 45 +++++++++++++---------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index f902529dd99..eac04d6eddd 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -1024,7 +1024,8 @@ namespace eosio { namespace chain { detail::index_writer index(new_index_filename, num_blocks); uint64_t read_size = 0; - for(uint64_t to_write_remaining = to_write; to_write_remaining > 0; to_write_remaining -= read_size) { + uint64_t write_size = 0; + for(uint64_t to_write_remaining = to_write; to_write_remaining > 0; to_write_remaining -= write_size) { read_size = to_write_remaining; if (read_size > detail::reverse_iterator::_buf_len) { read_size = detail::reverse_iterator::_buf_len; @@ -1036,41 +1037,27 @@ namespace eosio { namespace chain { const auto num_read = fread(buf, read_size, 1, original_block_log.blk_in); EOS_ASSERT( num_read == 1, block_log_exception, "blocks.log read failed" ); - // handle the case that the distance between `buf+buffer_index` and `the end of the buffer` is less than 8 bytes - uint64_t complete_pos_content; - bool re_write = false; - uint64_t re_write_offset; - // walk this memory section to adjust block position to match the adjusted location // of the block start and store in the new index file + write_size = read_size; while(original_pos >= start_of_blk_buffer_pos) { const auto buffer_index = original_pos - start_of_blk_buffer_pos; - if (buffer_index + pos_size <= detail::reverse_iterator::_buf_len){ - uint64_t& pos_content = *(uint64_t*)(buf + buffer_index); - const auto start_of_this_block = pos_content; - pos_content = start_of_this_block - pos_delta; - index.write(pos_content); - original_pos = start_of_this_block - pos_size; - }else{ - re_write = true; - re_write_offset = detail::reverse_iterator::_buf_len - buffer_index; - // read pos_content from original log file instead of the buffer - status = fseek(original_block_log.blk_in, original_pos, SEEK_SET); - const auto num_read = fread(&complete_pos_content, sizeof(uint64_t), 1, original_block_log.blk_in); - EOS_ASSERT( num_read == 1, block_log_exception, "blocks.log read failed" ); - index.write(complete_pos_content - pos_delta); - original_pos = complete_pos_content - pos_size; + uint64_t& pos_content = *(uint64_t*)(buf + buffer_index); + if ( (pos_content - start_of_blk_buffer_pos) > 0 && (pos_content - start_of_blk_buffer_pos) < pos_size ) { + // avoid the whole 8 bytes that contains a blk pos being split by the buffer + write_size = read_size - buffer_index - pos_size; + break; } + const auto start_of_this_block = pos_content; + pos_content = start_of_this_block - pos_delta; + index.write(pos_content); + original_pos = start_of_this_block - pos_size; } - new_block_file.seek(new_block_file_first_block_pos + to_write_remaining - read_size); - new_block_file.write(buf, read_size); - if (re_write){ - re_write = false; - new_block_file.seek((new_block_file_first_block_pos + to_write_remaining - read_size) + read_size - re_write_offset); - uint64_t pos = complete_pos_content - pos_delta; - new_block_file.write((char*)&pos, sizeof(uint64_t)); - } + new_block_file.seek(new_block_file_first_block_pos + to_write_remaining - write_size); + uint64_t offset = read_size - write_size; + new_block_file.write(buf+offset, write_size); } + index.complete(); fclose(original_block_log.blk_in); original_block_log.blk_in = nullptr; From 42935f180287887541ebb7bfba1b678eacd514d5 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Mon, 30 Aug 2021 16:58:49 -0400 Subject: [PATCH 4/8] Update code to avoid alignment issues --- libraries/chain/block_log.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index eac04d6eddd..44aff7756aa 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -956,6 +956,20 @@ namespace eosio { namespace chain { return std::clamp(version, min_supported_version, max_supported_version) == version; } + namespace { + template + T read_buffer(const char* buf) { + T result; + memcpy(&result, buf, sizeof(T)); + return result; + } + + template + void write_buffer(char* des, const T* src) { + memcpy(des, src, sizeof(T)); + } + } + bool block_log::trim_blocklog_front(const fc::path& block_dir, const fc::path& temp_dir, uint32_t truncate_at_block) { using namespace std; EOS_ASSERT( block_dir != temp_dir, block_log_exception, "block_dir and temp_dir need to be different directories" ); @@ -1042,7 +1056,7 @@ namespace eosio { namespace chain { write_size = read_size; while(original_pos >= start_of_blk_buffer_pos) { const auto buffer_index = original_pos - start_of_blk_buffer_pos; - uint64_t& pos_content = *(uint64_t*)(buf + buffer_index); + uint64_t pos_content = read_buffer(buf + buffer_index); if ( (pos_content - start_of_blk_buffer_pos) > 0 && (pos_content - start_of_blk_buffer_pos) < pos_size ) { // avoid the whole 8 bytes that contains a blk pos being split by the buffer write_size = read_size - buffer_index - pos_size; @@ -1050,6 +1064,7 @@ namespace eosio { namespace chain { } const auto start_of_this_block = pos_content; pos_content = start_of_this_block - pos_delta; + write_buffer(buf + buffer_index, &pos_content); index.write(pos_content); original_pos = start_of_this_block - pos_size; } From a1cc6f9db8308f91d1614b1a7d214f618e6404a5 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Tue, 31 Aug 2021 19:20:16 -0400 Subject: [PATCH 5/8] Add test cases --- libraries/chain/block_log.cpp | 12 +++- .../chain/include/eosio/chain/block_log.hpp | 3 + unittests/restart_chain_tests.cpp | 60 +++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 44aff7756aa..5a5459518dd 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -102,7 +102,7 @@ namespace eosio { namespace chain { uint64_t previous(); uint32_t version() const { return _version; } uint32_t first_block_num() const { return _first_block_num; } - constexpr static uint32_t _buf_len = 1U << 24; + static uint32_t _buf_len; private: void update_buffer(); @@ -759,6 +759,8 @@ namespace eosio { namespace chain { , _buffer_ptr(std::make_unique(_buf_len)) { } + uint32_t detail::reverse_iterator::_buf_len = 1U << 24; + uint32_t detail::reverse_iterator::open(const fc::path& block_file_name) { _block_file_name = block_file_name.generic_string(); _file.reset( FC_FOPEN(_block_file_name.c_str(), "r")); @@ -970,6 +972,10 @@ namespace eosio { namespace chain { } } + void block_log::set_buff_len(uint64_t len){ + detail::reverse_iterator::_buf_len = len; + } + bool block_log::trim_blocklog_front(const fc::path& block_dir, const fc::path& temp_dir, uint32_t truncate_at_block) { using namespace std; EOS_ASSERT( block_dir != temp_dir, block_log_exception, "block_dir and temp_dir need to be different directories" ); @@ -1057,10 +1063,10 @@ namespace eosio { namespace chain { while(original_pos >= start_of_blk_buffer_pos) { const auto buffer_index = original_pos - start_of_blk_buffer_pos; uint64_t pos_content = read_buffer(buf + buffer_index); + if ( (pos_content - start_of_blk_buffer_pos) > 0 && (pos_content - start_of_blk_buffer_pos) < pos_size ) { // avoid the whole 8 bytes that contains a blk pos being split by the buffer - write_size = read_size - buffer_index - pos_size; - break; + write_size = read_size - (pos_content - start_of_blk_buffer_pos); } const auto start_of_this_block = pos_content; pos_content = start_of_this_block - pos_delta; diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index 5fbe3e771a5..c584e5f3879 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -81,6 +81,9 @@ namespace eosio { namespace chain { static bool trim_blocklog_front(const fc::path& block_dir, const fc::path& temp_dir, uint32_t truncate_at_block); + // used only for unit test to adjust the buffer length + static void set_buff_len(uint64_t len); + private: void open(const fc::path& data_dir); void construct_index(); diff --git a/unittests/restart_chain_tests.cpp b/unittests/restart_chain_tests.cpp index 06958d25532..1818bfeccaa 100644 --- a/unittests/restart_chain_tests.cpp +++ b/unittests/restart_chain_tests.cpp @@ -78,4 +78,64 @@ BOOST_AUTO_TEST_CASE(test_restart_with_different_chain_id) BOOST_REQUIRE_EXCEPTION(other.open(chain_id), chain_id_type_exception, fc_exception_message_starts_with("chain ID in state ")); } +namespace{ + struct scoped_temp_path { + boost::filesystem::path path; + scoped_temp_path() { + path = boost::filesystem::unique_path(); + if (boost::unit_test::framework::master_test_suite().argc >= 2) { + path += boost::unit_test::framework::master_test_suite().argv[1]; + } + } + ~scoped_temp_path() { + boost::filesystem::remove_all(path); + } + }; +} + +enum class buf_len_type { small, medium, large }; + +void trim_blocklog_front(uint32_t truncate_at_block, buf_len_type len_type) { + tester chain; + chain.produce_blocks(30); + chain.close(); + + namespace bfs = boost::filesystem; + + auto blocks_dir = chain.get_config().blocks_dir; + scoped_temp_path temp1, temp2; + boost::filesystem::create_directory(temp1.path); + bfs::copy(blocks_dir / "blocks.log", temp1.path / "blocks.log"); + bfs::copy(blocks_dir / "blocks.index", temp1.path / "blocks.index"); + + trim_data td(temp1.path); + uint64_t blk_size = td.block_pos(30) - td.block_pos(29); + uint64_t log_size = td.block_pos(30) + blk_size; + + switch (len_type){ + case buf_len_type::small: + block_log::set_buff_len( blk_size + (sizeof(uint64_t) - 1)); + break; + case buf_len_type::medium: + block_log::set_buff_len( log_size / 3); + break; + case buf_len_type::large: + block_log::set_buff_len(log_size); + break; + default: + return; + } + + BOOST_CHECK( block_log::trim_blocklog_front(temp1.path, temp2.path, truncate_at_block) == true); +} + +BOOST_AUTO_TEST_CASE(test_trim_blocklog_front) { + trim_blocklog_front(5, buf_len_type::small); + trim_blocklog_front(6, buf_len_type::small); + trim_blocklog_front(10, buf_len_type::medium); + trim_blocklog_front(11, buf_len_type::medium); + trim_blocklog_front(15, buf_len_type::large); + trim_blocklog_front(16, buf_len_type::large); +} + BOOST_AUTO_TEST_SUITE_END() From d4eb59c7f25bfd196dd583027795351bc0274236 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Wed, 1 Sep 2021 18:56:35 -0400 Subject: [PATCH 6/8] Update the new test case --- unittests/restart_chain_tests.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/unittests/restart_chain_tests.cpp b/unittests/restart_chain_tests.cpp index 1818bfeccaa..402dda6d69a 100644 --- a/unittests/restart_chain_tests.cpp +++ b/unittests/restart_chain_tests.cpp @@ -102,15 +102,17 @@ void trim_blocklog_front(uint32_t truncate_at_block, buf_len_type len_type) { namespace bfs = boost::filesystem; - auto blocks_dir = chain.get_config().blocks_dir; + auto blocks_dir = chain.get_config().blocks_dir; + auto old_index_size = fc::file_size(blocks_dir / "blocks.index"); + scoped_temp_path temp1, temp2; boost::filesystem::create_directory(temp1.path); bfs::copy(blocks_dir / "blocks.log", temp1.path / "blocks.log"); bfs::copy(blocks_dir / "blocks.index", temp1.path / "blocks.index"); - trim_data td(temp1.path); - uint64_t blk_size = td.block_pos(30) - td.block_pos(29); - uint64_t log_size = td.block_pos(30) + blk_size; + trim_data old_log(temp1.path); + uint64_t blk_size = old_log.block_pos(30) - old_log.block_pos(29); + uint64_t log_size = old_log.block_pos(30) + blk_size; switch (len_type){ case buf_len_type::small: @@ -127,6 +129,13 @@ void trim_blocklog_front(uint32_t truncate_at_block, buf_len_type len_type) { } BOOST_CHECK( block_log::trim_blocklog_front(temp1.path, temp2.path, truncate_at_block) == true); + trim_data new_log(temp1.path); + BOOST_CHECK(new_log.first_block == truncate_at_block); + BOOST_CHECK(new_log.last_block == old_log.last_block); + BOOST_CHECK(old_log.version == new_log.version); + + int num_blocks_trimmed = truncate_at_block - 1; + BOOST_CHECK(fc::file_size(temp1.path / "blocks.index") == old_index_size - sizeof(uint64_t) * num_blocks_trimmed); } BOOST_AUTO_TEST_CASE(test_trim_blocklog_front) { From 7baa80e5dcdcd1d1d7f7624d5479be0d34d8ea74 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Wed, 1 Sep 2021 19:52:26 -0400 Subject: [PATCH 7/8] Move a method out of header file as the method is created for unit test --- libraries/chain/block_log.cpp | 9 +++++---- libraries/chain/include/eosio/chain/block_log.hpp | 3 --- unittests/restart_chain_tests.cpp | 8 +++++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 5a5459518dd..90677f4105d 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -972,10 +972,6 @@ namespace eosio { namespace chain { } } - void block_log::set_buff_len(uint64_t len){ - detail::reverse_iterator::_buf_len = len; - } - bool block_log::trim_blocklog_front(const fc::path& block_dir, const fc::path& temp_dir, uint32_t truncate_at_block) { using namespace std; EOS_ASSERT( block_dir != temp_dir, block_log_exception, "block_dir and temp_dir need to be different directories" ); @@ -1215,3 +1211,8 @@ namespace eosio { namespace chain { } } } /// eosio::chain + +// used only for unit test to adjust the buffer length +void set_buff_len(uint64_t len){ + eosio::chain::detail::reverse_iterator::_buf_len = len; +} \ No newline at end of file diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index c584e5f3879..5fbe3e771a5 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -81,9 +81,6 @@ namespace eosio { namespace chain { static bool trim_blocklog_front(const fc::path& block_dir, const fc::path& temp_dir, uint32_t truncate_at_block); - // used only for unit test to adjust the buffer length - static void set_buff_len(uint64_t len); - private: void open(const fc::path& data_dir); void construct_index(); diff --git a/unittests/restart_chain_tests.cpp b/unittests/restart_chain_tests.cpp index 402dda6d69a..197a1ac30f1 100644 --- a/unittests/restart_chain_tests.cpp +++ b/unittests/restart_chain_tests.cpp @@ -22,6 +22,8 @@ void remove_existing_blocks(controller::config& config) { remove(block_index_path); } +void set_buff_len(uint64_t len); + BOOST_AUTO_TEST_SUITE(restart_chain_tests) BOOST_AUTO_TEST_CASE(test_existing_state_without_block_log) @@ -116,13 +118,13 @@ void trim_blocklog_front(uint32_t truncate_at_block, buf_len_type len_type) { switch (len_type){ case buf_len_type::small: - block_log::set_buff_len( blk_size + (sizeof(uint64_t) - 1)); + set_buff_len( blk_size + (sizeof(uint64_t) - 1)); break; case buf_len_type::medium: - block_log::set_buff_len( log_size / 3); + set_buff_len( log_size / 3); break; case buf_len_type::large: - block_log::set_buff_len(log_size); + set_buff_len(log_size); break; default: return; From 90499a44818a8449818f62826ca13ebc6cdf3af1 Mon Sep 17 00:00:00 2001 From: Jingjun Zhao Date: Thu, 2 Sep 2021 10:07:30 -0400 Subject: [PATCH 8/8] Rename a function --- libraries/chain/block_log.cpp | 2 +- unittests/restart_chain_tests.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 90677f4105d..66a26a59d8e 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -1213,6 +1213,6 @@ namespace eosio { namespace chain { } } /// eosio::chain // used only for unit test to adjust the buffer length -void set_buff_len(uint64_t len){ +void block_log_set_buff_len(uint64_t len){ eosio::chain::detail::reverse_iterator::_buf_len = len; } \ No newline at end of file diff --git a/unittests/restart_chain_tests.cpp b/unittests/restart_chain_tests.cpp index 197a1ac30f1..a33d57ce8a5 100644 --- a/unittests/restart_chain_tests.cpp +++ b/unittests/restart_chain_tests.cpp @@ -22,7 +22,7 @@ void remove_existing_blocks(controller::config& config) { remove(block_index_path); } -void set_buff_len(uint64_t len); +void block_log_set_buff_len(uint64_t len); BOOST_AUTO_TEST_SUITE(restart_chain_tests) @@ -118,13 +118,13 @@ void trim_blocklog_front(uint32_t truncate_at_block, buf_len_type len_type) { switch (len_type){ case buf_len_type::small: - set_buff_len( blk_size + (sizeof(uint64_t) - 1)); + block_log_set_buff_len( blk_size + (sizeof(uint64_t) - 1)); break; case buf_len_type::medium: - set_buff_len( log_size / 3); + block_log_set_buff_len( log_size / 3); break; case buf_len_type::large: - set_buff_len(log_size); + block_log_set_buff_len(log_size); break; default: return;