Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Conversation

@brooksprumo
Copy link
Contributor

Problem

The accounts hash interval is fraught with sharp edges. Here's a few of them:

Halt on accounts hash mismatch

The --halt-on-known-validators-accounts-hash-mismatch flag exists as a way to ensure the accounts state is correct. This flag has now been deprecated. With the Epoch Accounts Hash feature, the accounts state will be included as part of a bank hash once per epoch, thus baking it into consensus itself.

Since the accounts state is now checked explicitly (with EAH), and the extra flag has been deprecated, there is not a need to perform extra accounts hash verifications.

Incremental snapshots

Incremental snapshots will soon use the Incremental Accounts Hash feature, which substantially reduces the amount of time (and resources) required to calculate the accounts hash necessary for an incremental snapshot. This was born from the need to support continued growth of the cluster as the number of accounts grows to be greater than 1 billion.

If the incremental snapshot interval is not equal to the accounts hash interval, then that means additional accounts hash calculations will run, and they will be full accounts hash calculations. This will negate the benefits of the incremental accounts hash calculation.

For performance, we want to avoid full accounts hash calculations, and thus should not perform additional/unnecessary ones.

Summary of Changes

  • Deprecate the accounts-hash-interval-slots flag
  • Internally, set the accounts hash interval to the snapshot interval

Later, we'll be able to remove the whole AccountsHashVerifier background request too!

@brooksprumo brooksprumo self-assigned this Jun 6, 2023
@brooksprumo brooksprumo marked this pull request as ready for review June 6, 2023 14:25
@brooksprumo brooksprumo requested review from mvines and t-nelson June 6, 2023 14:26
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #31987 (a387024) into master (4227d0e) will increase coverage by 0.0%.
The diff coverage is 16.6%.

@@           Coverage Diff           @@
##           master   #31987   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         761      761           
  Lines      207599   207595    -4     
=======================================
+ Hits       170071   170079    +8     
+ Misses      37528    37516   -12     

@brooksprumo brooksprumo requested a review from t-nelson June 6, 2023 18:05
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha! i knew fmt was going to fail. github wouldn't let me include the context line in the suggested changes 😢

:shipit:

@brooksprumo brooksprumo merged commit b8ed614 into solana-labs:master Jun 20, 2023
@brooksprumo brooksprumo deleted the accounts-hash-interval/validator branch June 21, 2023 00:08
wen-coding pushed a commit to wen-coding/solana that referenced this pull request Aug 15, 2023
wen-coding pushed a commit to wen-coding/solana that referenced this pull request Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants