-
Notifications
You must be signed in to change notification settings - Fork 73
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
Propagate block after header validation #590
Conversation
…uld restart block because speculative block may have been interrupted.
… full validation of the block.
…received so it can be processed when valid later.
…k received during block production
…read is doing block header validation at a time.
fc_dlog( logger, "signaled pre_accepted_block, blk num = ${num}, id = ${id}", ("num", block->block_num())("id", id) ); | ||
|
||
auto blk_trace = fc_create_trace_with_id("Block", id); | ||
auto blk_span = fc_create_span(blk_trace, "PreAccepted"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should FC zipkin related files be removed all together?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that fc is no longer a submodule, I would think yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
…OGH-590-shutdown-race [3.1] Fix race condition on trace_api_plugin shutdown
…OGH-590-into-main [3.1 -> main] Fix race condition on trace_api_plugin shutdown
@@ -38,7 +38,7 @@ namespace eosio { namespace chain { namespace plugin_interface { | |||
|
|||
namespace incoming { | |||
namespace methods { | |||
// synchronously push a block/trx to a single provider, block_state_ptr may be null | |||
// synchronously push a block/trx to a single provider, block_state_ptr may be null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this comment is accurate about synchronously push a transaction, as its name is transaction_async
. Maybe split block/trx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a somewhat confusing use of async
used in our codebase. async
here means that there is a next_function
provided which will continue processing. The comment is meant to convey, that regardless of the name, these methods
are called synchronously unlike channels
which are implemented via post()
calls.
@@ -1750,8 +1751,6 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { | |||
} | |||
|
|||
try { | |||
_account_fails.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is called in abort_block()
, no need to call it here.
@heifner As much as I want to, I don't feel I understand enough to approve the PR, sorry about that! |
general comment for future: |
…index has to be maintained.
…it can just be calculated quickly from id
Validate block header and propagate blocks without touching the main thread. This should greatly improve block propagation times. This allows a block header to be validated and a block sent to peers without touching the main thread as long as the previous block has been processed. If the previous block has not been fully processed then it is queued for processing on the main thread.
Includes:
peer_block_state_index
Resolves #568
Built on #543