-
Notifications
You must be signed in to change notification settings - Fork 586
Rework, speed up root ledger population from genesis #17874
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
Changes from all commits
3b0baac
0e19357
cefe3b2
1a283ef
af98edb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| This PR eliminates unnecessary genesis ledger rehashing when bootstrapping from | ||
| genesis, speeding up that process by about 1.5 minutes directly, depending on | ||
| hardware. | ||
|
|
||
| This PR also fixes a related bug: the initial best tip network would formerly | ||
| always fail on mainnet when bootstrapping from genesis, due to the daemon | ||
| becoming unresponsive while rehashing the genesis ledger. This would delay | ||
| startup by an additional number of minutes, cause the daemon to report itself as | ||
| synced while its best tip was still at genesis until the next best tip query | ||
| succeeded, and cause confusing behaviour in rosetta. This initial query should | ||
| now only fail under very specific poor network conditions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,17 +309,38 @@ let with_instance_exn t ~f = | |
| let x = f instance in | ||
| Instance.close instance ; x | ||
|
|
||
| let reset_to_genesis_exn t ~precomputed_values = | ||
| (** Clear the factory directory and recreate the snarked ledger instance for | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest to move this doc comment to interface instead. |
||
| this factory with [create_root] and [setup] *) | ||
| let reset_factory_root_exn t ~create_root ~setup = | ||
| let open Async.Deferred.Let_syntax in | ||
| assert (Option.is_none t.instance) ; | ||
| Mina_stdlib_unix.File_system.rmrf t.directory ; | ||
| with_instance_exn t ~f:(fun instance -> | ||
| ignore | ||
| ( Precomputed_values.populate_root precomputed_values | ||
| (Instance.snarked_ledger instance) | ||
| |> Or_error.map ~f:Ledger.Root.as_unmasked | ||
| : Ledger.Any_ledger.witness Or_error.t ) ; | ||
| Instance.set_root_identifier instance | ||
| (genesis_root_identifier | ||
| ~genesis_state_hash: | ||
| (Precomputed_values.genesis_state_hashes precomputed_values) | ||
| .state_hash ) ) | ||
| (* Certain database initialization methods, e.g. creation from a checkpoint, | ||
| depend on the parent directory existing and the target directory _not_ | ||
| existing. *) | ||
| let%bind () = Mina_stdlib_unix.File_system.remove_dir t.directory in | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if there is specific reason to chose these two to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will double-check. I kind of remember having a reason, but I might have been mistaken.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe one API is buggy. We haven't fix it and add unit test for it. |
||
| let%map () = Mina_stdlib_unix.File_system.create_dir t.directory in | ||
| let root = | ||
| create_root | ||
| ~config:(Instance.Config.snarked_ledger t) | ||
| ~depth:t.ledger_depth () | ||
| |> Or_error.ok_exn | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Report an error message here, instead of a failure? |
||
| in | ||
| Ledger.Root.close root ; | ||
| with_instance_exn t ~f:setup | ||
|
|
||
| let reset_to_genesis_exn t ~precomputed_values = | ||
| let open Async.Deferred.Let_syntax in | ||
| let logger = t.logger in | ||
| [%log debug] "Resetting snarked_root in $directory to genesis" | ||
| ~metadata:[ ("directory", `String t.directory) ] ; | ||
| let%map () = | ||
| reset_factory_root_exn t | ||
| ~create_root:(Precomputed_values.create_root precomputed_values) | ||
| ~setup:(fun instance -> | ||
| Instance.set_root_identifier instance | ||
| (genesis_root_identifier | ||
| ~genesis_state_hash: | ||
| (Precomputed_values.genesis_state_hashes precomputed_values) | ||
| .state_hash ) ) | ||
| in | ||
| [%log debug] "Finished resetting snarked_root to genesis" | ||
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.