Refactor in Block producer #16181
Refactor in Block producer #16181
Conversation
|
!ci-build-me |
|
!ci-nightly-me |
georgeee
force-pushed
the
al/header
branch
5 times, most recently
from
bba3a3d
to
a4f45cf
Compare
Co-authored-by: georgeee <[email protected]>
Co-authored-by: georgeee <[email protected]>
Co-authored-by: georgeee <[email protected]>
Co-authored-by: georgeee <[email protected]>
Co-authored-by: georgeee <[email protected]>
georgeee
force-pushed
the
al/block-producer
branch
from
38e78ff
to
071cb48
Compare
Committed separately for cleaner diff in Github interface.
georgeee
force-pushed
the
al/block-producer
branch
from
071cb48
to
a2be0e4
Compare
| (* Commented out because for large blocks it's an oversized log *) | ||
| (* ; ( "internal_transition" | ||
| , Internal_transition.to_yojson internal_transition ) *) | ||
| ; ( "internal_transition" |
There was a problem hiding this comment.
This change was probably made by accident
|
Suggest splitting this PR: commits before Refactor block production control flow are extremely trivial movements of code and could be merged as such (we don't even need to test these). Everything further actually changes control flow, so it's probably better to consider them separately. |
|
Commit Debugging block creation schedule has to be removed from the PR |
| in | ||
| check_next_block_timing slot i () ) | ||
| let%map () = | ||
| Broadcast_pipe.Reader.iter_until frontier_reader |
There was a problem hiding this comment.
Add comment: iterates until there is some frontier
| x | ||
| in | ||
| let%map _ = Interruptible.force produce_intr in | ||
| (* TODO consider uncommenting below or removing interruptible usage completely *) |
There was a problem hiding this comment.
Forcing produce_intr makes sure that the next iteration won't start before the block is produced. Hence interruptible will not ever be actually interrupted.
It could make sense if we replaced the Interruptible.force above with Interruptible.don't_wait_for, but this wasn't the case in the original code.
| handle_block_production_errors ~logger ~rejected_blocks_logger | ||
| ~time_taken:span ~previous_protocol_state ~protocol_state res | ||
|
|
||
| let iteration ~schedule_next_check ~dispatch_now ~schedule_dispatch |
There was a problem hiding this comment.
Rename:
~schedule_next_checkto~schedule_next_vrf_check~dispatch_nowto~produce_block_now~next_check_nowto~next_vrf_check_now~schedule_dispatchto~schedule_block_production
| ~next_check_now ~context:(module Context : CONTEXT) ~vrf_evaluator | ||
| ~time_controller ~coinbase_receiver ~set_next_producer_timing ~frontier | ||
| ~vrf_evaluation_state ~epoch_data_for_vrf ~ledger_snapshot i slot = | ||
| O1trace.thread |
There was a problem hiding this comment.
Remove this O1trace.thread, it was introduced for debugging
| @@ -1241,6 +1225,9 @@ let run ~context:(module Context : CONTEXT) ~vrf_evaluator ~prover ~verifier | |||
| let run_precomputed ~context:(module Context : CONTEXT) ~verifier ~trust_system | |||
| ~time_controller ~frontier_reader ~transition_writer ~precomputed_blocks = | |||
| let open Context in | |||
| let log_bootstrap_mode () = | |||
There was a problem hiding this comment.
why it was re-introduced locally?
|
Last few commits are better be split to standalone PRs. Point of commit Refactor block production control flow is that this is only a rewrite of previous code: singleton scheduler is only used to schedule one routine to happen strictly after previous scheduled routine was completed, and Commit Remove Interruptible usage from block production removes the use of interruptible which becomes obviously redundant (interruptible will never get to be interrupted) after previous commit. Finally, Refactor genesis block creation is the only change that modifies behavior (not just refactors the code): instead of trying to defer creation of genesis block to the latest possible point, it's always being executed at the beginning of |
This PR updates old commits :