Skip to content
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

Refactor UpdateHighestExecutedBlockIfHigher with UpdateLatestExecutedBlock #6908

Closed
zhangchiqing opened this issue Jan 16, 2025 · 1 comment · Fixed by #6909
Closed

Refactor UpdateHighestExecutedBlockIfHigher with UpdateLatestExecutedBlock #6908

zhangchiqing opened this issue Jan 16, 2025 · 1 comment · Fixed by #6909

Comments

@zhangchiqing
Copy link
Member

Problem Definition

What problem is this solving?

The execution state stores HighestExecutedBlock in database, and updated UpdateHighestExecutedBlockIfHigher and GetHighestExecutedBlockID methods for updating and querying it.

This HighestExecutedBlock introduces extra complexity when refactoring the storage from using badger transaction to badger batch updates, because UpdateHighestExecutedBlockIfHigher needs to read the last executed block from the database and compare it with the current executed block and only update if its height is higher. However, when using batch updates, the last executed block read from database might be stale, which might cause a incorrect block being stored.

It's possible to implement the correct behavior with badger batch update, for instance, using a lock to ensure only 1 process is reading and updating the HighestExecutedBlock at a time, which will prevent potential race condition.

However, I wonder if it's worth the effort to implement the correct behavior. I would argue that it's not that important to know the highest executed block. Instead, we only care about last executed block, which is much simplier to implement.

So I'd like to refactor with a simplier version, but before that, let me explain their difference:

Difference between HIghestExecutedBlock and LastExecutedBlock

In most cases, the HighestExecutedBlock is the same as LastExecutedBlock. They are only different when there are multiple forks, and a block on a fork that is not the highest got executed, in this case, the last executed block will be lower than HighestExecutedBlock.

Where is HighestExecutedBlock being used?

There are two main use cases:

  1. Reporting metrics for the height of highest executed block
  2. Finding last executed and finalized block or finding last executed and sealed block

For use case 1, I think the metrics for HighestExecutedBlock's Height isn't more much useful than just last executed block height even when they are different.

For use case 2, the difference also doesn't matter, because if they are different, the last executed block is a unfinalized block, so the last executed and finalized block will be just the last finalized block, the same for last executed and sealed block.

Proposed Solution

Replace UpdateHighestExecutedBlockIfHigher and GetHighestExecutedBlockID with UpdateLastExecutedBlock and GetLastExecutedBlock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant