Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Optimize block log usage #7785

Merged
merged 5 commits into from
Aug 27, 2019
Merged

Optimize block log usage #7785

merged 5 commits into from
Aug 27, 2019

Conversation

heifner
Copy link
Contributor

@heifner heifner commented Aug 22, 2019

Change Description

  • Emit accepted block signal before updating block log since accepted block does not depend on lib. This allows the net_plugin to start sending out the block sooner.
  • Optimize getting block_id via block_num out of block log by reading only the block_header instead of the entire block.
  • Provide access to head_id of block log instead of calculating it from the head block.

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@@ -2697,12 +2697,12 @@ block_id_type controller::last_irreversible_block_id() const {
if( block_header::num_from_id(tapos_block_summary.block_id) == lib_num )
return tapos_block_summary.block_id;

auto signed_blk = my->blog.read_block_by_num( lib_num );
auto id = my->blog.read_block_id_by_num( lib_num );
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we change the contents of this method to just call get_block_id_for_num?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I agree with Brian, It was fixed IIRC in 1.8.x but we used to have a situation where the LIB was not guaranteed to be in the block-log (it was still in the fork DB due to a slow pruning process). Even with that fixed, this method (as-spoke) tightly couples to the expectation that the block-log always contains the LIB and I can think of several reasons why we'd want to break that assumption in the future. (like throwing block.log writes to a background thread).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the check for id in tapos block summary to get_block_id_for_num since might as well have that optimization there and modified last_irreversiable_block_id to call get_block_id_for_num.

@@ -2697,12 +2697,12 @@ block_id_type controller::last_irreversible_block_id() const {
if( block_header::num_from_id(tapos_block_summary.block_id) == lib_num )
return tapos_block_summary.block_id;

auto signed_blk = my->blog.read_block_by_num( lib_num );
auto id = my->blog.read_block_id_by_num( lib_num );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I agree with Brian, It was fixed IIRC in 1.8.x but we used to have a situation where the LIB was not guaranteed to be in the block-log (it was still in the fork DB due to a slow pruning process). Even with that fixed, this method (as-spoke) tightly couples to the expectation that the block-log always contains the LIB and I can think of several reasons why we'd want to break that assumption in the future. (like throwing block.log writes to a background thread).

@heifner heifner requested a review from b1bart August 27, 2019 13:19
@heifner heifner merged commit bfc0845 into develop Aug 27, 2019
@heifner heifner deleted the opt-block-log branch August 27, 2019 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants