v1.16: Add ability to output Bank hash details (backport of #32632)

[mergify]

This is an automatic backport of pull request #32632 done by Mergify.
Cherry-pick of 6bbf514 has failed:

On branch mergify/bp/v1.16/pr-32632
Your branch is up to date with 'origin/v1.16'.

You are currently cherry-picking commit 6bbf514e78.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   core/src/replay_stage.rs
	modified:   runtime/src/bank.rs
	new file:   runtime/src/bank/bank_hash_details.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   Cargo.lock
	both modified:   ledger-tool/src/args.rs
	both modified:   ledger-tool/src/main.rs
	both modified:   programs/sbf/Cargo.lock
	both modified:   runtime/Cargo.toml
	both modified:   runtime/src/accounts_db.rs
	both modified:   validator/src/main.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

---

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

• @Mergifyio refresh will re-evaluate the rules
• @Mergifyio rebase will rebase this PR on its base branch
• @Mergifyio update will merge the base branch into this PR
• @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

• look at your merge queues
• generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

[codecov]

Codecov Report

Merging #34257 (3c6b136) into v1.16 (302238b) will increase coverage by 0.0%.
Report is 1 commits behind head on v1.16.
The diff coverage is 89.7%.

❗ Current head 3c6b136 differs from pull request most recent head d0f7ff2. Consider uploading reports for the commit d0f7ff2 to get more accurate results

Additional details and impacted files

@@           Coverage Diff            @@
##            v1.16   #34257    +/-   ##
========================================
  Coverage    81.8%    81.8%            
========================================
  Files         766      767     +1     
  Lines      209077   209251   +174     
========================================
+ Hits       171128   171312   +184     
+ Misses      37949    37939    -10     

[t-nelson]

we really shouldn't be debugging many legit hash mismatches on 1.16 in the next ~6wks. do we really need this?

[steviez]

we really shouldn't be debugging many legit hash mismatches on 1.16 in the next ~6wks. do we really need this?

Well, we've had a handful of panic reports on v1.16:

• validator version 1.16.16 crashed with "unable to freeze a block with bankhash" #33740
• Panic due to a duplicate block produced (1.16.17, Jito) #34142
• The one from Triton without an issue
• Probably more in Discord or metrics if I go sift through all of the out-of-disk & unable-to-start-from-genesis panics

The couple of these failures that we've looked into haven't been reproducible with solana-ledger-tool. At this point, we really have no idea what the cause is (hardware issue, non-deterministic bug, gremlins, etc). This change isn't a fix for these issues, but it could allow us to gain additional insight into the failures and potentially lead to a solution.

To me, not backporting this change is saying that we're ok with intermittent panics until we get to v1.17 on mnb, either because v1.17 somehow resolves the issue or because v1.17 has this change present and will allow us to debug then.

I realize the diff is large for backport this late in the release cycle, but if you trace it down, the only place where it gets calls on a production validator is when the node is essentially already doomed in the dump/repair/retry loop

[t-nelson]

right, but the stable branch should be sufficiently solidified at this point that carrying the patch set externally won't be much effort. we can just build a new patched bin should the need arise. no reason to impose any risk on production

[steviez]

right, but the stable branch should be sufficiently solidified at this point that carrying the patch set externally won't be much effort. we can just build a new patched bin should the need arise. no reason to impose any risk on production

I think the most important part of this PR is that solana-validator will create the file when it is about to panic after it has diverged. If we don't merge this PR, the order of operations is:

• Validator hits panic
• If we're lucky, we get validator to apply patch
• We wait and see if we observe the panic on the node that applied the patch
• Repeat the above process for each new validator that encounters the panic

In #33740 specifically, I got Zan to apply the patch eventually after he ran into the panic several times. However, he has not yet seen the panic with the patch present in his binary. Additionally, he rightfully so wants some stability with his node

[steviez]

Quick datapoint, looking at mnb metrics from the last 7 days as of time of me writing this:

• There are 35 instances of the We are attempting to dump a block that we produced ... (node is leader)
• There are 28 instances of the "We have tried to repair duplicate blocks ...` logs (node is NOT leader)

Skimming through, a couple of these are operator error (ie running v1.14.X), but plenty are current versions. This also doesn't account for number of people who may have hit this who aren't reporting metrics. Granted, someone has to chime up in Discord that they hit this and send us the file, but if we bake the change into the binary, I feel like we'll have a higher percentage of getting some hits

[t-nelson]

r+ @brooksprumo's approval

[steviez]

We should also backport #34256 to v1.16 once this backport lands; this PR covers the non-leader path, that PR will create the same debug file when the node in question is the leader

[brooksprumo]

The accounts-db directories look good to me.

There's some nit comments that are basically questions on consistency with v1.17/master, but they are not deal breakers IMO.

[brooksprumo]

lgtm

[bji]

right, but the stable branch should be sufficiently solidified at this point that carrying the patch set externally won't be much effort. we can just build a new patched bin should the need arise. no reason to impose any risk on production

I think the most important part of this PR is that solana-validator will create the file when it is about to panic after it has diverged. If we don't merge this PR, the order of operations is:

• Validator hits panic
• If we're lucky, we get validator to apply patch
• We wait and see if we observe the panic on the node that applied the patch
• Repeat the above process for each new validator that encounters the panic

In #33740 specifically, I got Zan to apply the patch eventually after he ran into the panic several times. However, he has not yet seen the panic with the patch present in his binary. Additionally, he rightfully so wants some stability with his node

I'm ashamed to admit that I did not return to 1.16 with the patch that enables the extra debugging. I am still on 1.17.5. I promise that tomorrow I will downgrade my secondary to 1.16.21. 1.17.5 has been completely solid for me, so I feel like "that's the solution anyway". But still I'll downgrade to help out with the debugging of this issue.