Baker: Make record_state function optional

Context

Closes #6493 (closed).

The record_state was doing the following (time costly) things:

• obtain the wallet's mutex
• binary encode state data (level, round, payload)
• perform I/Os to write to a record_state file

Under normal conditions, during a consensus round, this state saving is performed once so the theoretical gain is estimated to be the average value: 91.6ms. There are also some additional gains due to concurrency that are hard to estimate as it depends on disk I/Os and other components relying on the wallet's lockfile mechanism.

We introduce a record-state flag for protocols Nairobi, Oxford and Alpha.

./octez-baker-<protocol> run with local node ... --record-state

When the option is not set, the default is false. We do this so that we can opt out of recording the state, since in the profiling results we could observe that unnecessary long time was spent:

perform action 'inject endorsements' ......................................... 1          227.734ms  37% +8s35.355ms
  record state ............................................................... 1           94.392ms  23% +0.002ms
    waiting lock ............................................................. 1            0.051ms 100% +0.010ms
    serializing baking state ................................................. 1            4.473ms 100% +0.062ms
    writing baking state ..................................................... 1           89.651ms  18% +4.538ms
    renaming file ............................................................ 1            0.191ms  96% +94.191ms
  inject endorsements ........................................................ 1          133.194ms  48% +94.396ms

Manually testing the MR

dune exec tezt/tests/main.exe -- --file baker_operations_cli_options.ml

After running an experiment with @andrea.cerone using this change, we could see in the baker_profiling.txt file that the record state part was indeed missing (we could usually find it after the perform action 'inject endorsements' part:

perform action 'inject endorsements' ......................................... 1          212.159ms  14% +2s811.634ms
  inject endorsements ........................................................ 1          212.015ms  14% +0.001ms
    sign endorsements ........................................................ 1           55.984ms  25% +0.000ms
  ...

Checklist

• Document the interface of any function added or modified (see the coding guidelines)
• Document any change to the user interface, including configuration parameters (see node configuration)
• Provide automatic testing (see the testing guide).
• For new features and bug fixes, add an item in the appropriate changelog (docs/protocols/alpha.rst for the protocol and the environment, CHANGES.rst at the root of the repository for everything else).
• Select suitable reviewers using the Reviewers field below.
• Select as Assignee the next person who should take action on that MR

changed milestone to %(2023Q4) - Layer 1 - Reduce Block time

added reduce-block-time label

requested review from @andrea.cerone, @ryan.tan3, @vbotbol, @gsebil08, and @ACoquereau

assigned to @gabriel.moise

marked the checklist item Select as Assignee the next person who should take action on that MR as completed

marked the checklist item Select suitable reviewers using the Reviewers field below. as completed

Thank you for your contribution 🙏, I have one remark:

• ⚠ There are changes to protocol-client-code:
  • → src/proto_017_PtNairob/lib_delegate/baking_commands.ml, src/proto_017_PtNairob/lib_delegate/baking_configuration.ml, src/proto_017_PtNairob/lib_delegate/baking_configuration.mli, …
  • → Please make sure changes are properly replicated for all relevant protocols (e.g. src/proto_alpha, src/proto_0XX_PtBLaBLa, …).

---

I, 🤖 👮, ran this inspection on Tue, 24 Oct 2023 12:23:45 +0000. I usually run inspections every 6 hours and will try to edit my own comment if I have any updates. For more information see the Tezos documentation.

changed the description

added 3 commits

• 172d91f8 - Baker: Add record_state flag to baker commands
• a150f97e - Baker: Use record_state flag
• 9ebd18f5 - Baker: Add tezt tests

Compare with previous version

Also, please add some argumentation in the description based on the resulting reports of the experiments where we can notice that writing the state on-disk takes a significant amount of time.

added 3 commits

• f2cb133f - Baker: Add record_state flag to baker commands
• fe4c6c71 - Baker: Use record_state flag
• df45bca2 - Baker: Add tezt tests

Compare with previous version

added 3 commits

• 5b4d4eb9 - Baker: Add record_state flag to baker commands
• ad1323fe - Baker: Use record_state flag
• 7b5b0f46 - Baker: Add tezt tests

Compare with previous version

added 4 commits

• b8d124af - Baker: Add record_state flag to baker commands
• 998324b6 - Baker: Use record_state flag
• fed2e790 - Baker: Add tezt tests
• 55075064 - Baker: Replace state_recorder_config Disabled with Memory

Compare with previous version

What I did in the end:

• created a state_recorder_switch_arg which means that state_recorder will be true iff we use the --record-state argument in the baker commands : b8d124af
• transformed state_recorder from above into a state_recorder_config type : b8d124af
• used the state_recorder variable to determine whether we use the record_state function or not : 998324b6
• tested in tezt both when we write in Memory and for Filesystem : fed2e790
• changed the Disabled name to Memory : 55075064

assigned to @vbotbol and unassigned @gabriel.moise

marked this merge request as ready