Skip to content

fix: keep 9W blocks in ancient db when prune block#2481

Merged
zzzckck merged 5 commits intobnb-chain:developfrom
jingjunLi:fix-2437-issue
May 29, 2024
Merged

fix: keep 9W blocks in ancient db when prune block#2481
zzzckck merged 5 commits intobnb-chain:developfrom
jingjunLi:fix-2437-issue

Conversation

@jingjunLi
Copy link
Copy Markdown
Contributor

@jingjunLi jingjunLi commented May 20, 2024

Description

After introducing the feature to use the finalized block as the chain freeze indicator for the multiDatabase feature, the kv database only contains a few dozen blocks. Performing prune block operations and then force-killing the process can lead to the blockchain rewinding to a non-existent block. To address this issue, the prune block will by default retain 90,000 blocks in the ancient database.

Rationale

Other Changes:

  1. Finalized Block as Chain Freeze Indicator: The feature to use the finalized block as the chain freeze indicator for the multiDatabase feature will only be effective when the multiDatabase is enabled.
  2. Prune Block Fix: Fixed the bug where prune block operations could not execute when the trie journal was set to use journal file.
  3. Journal Optimization: Improved logging for the creation of JournalReader and JournalWriter in the Journal.

These changes ensure that the blockchain does not rewind to a non-existent block during force-kill scenarios and provide better operational logging and bug fixes for prune block executions.

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@jingjunLi jingjunLi requested review from RenRick, fynnss and sysvm May 21, 2024 02:54
fynnss
fynnss previously approved these changes May 27, 2024
sysvm
sysvm previously approved these changes May 27, 2024
Copy link
Copy Markdown
Contributor

@sysvm sysvm left a comment

Choose a reason for hiding this comment

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

LGTM

Name: "block-amount-reserved",
Usage: "Sets the expected remained amount of blocks for offline block prune",
Category: flags.BlockHistoryCategory,
Value: 90000,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use params.FullImmutabilityThreshold.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@jingjunLi jingjunLi dismissed stale reviews from sysvm and fynnss via 9561de1 May 27, 2024 08:49
@jingjunLi jingjunLi force-pushed the fix-2437-issue branch 2 times, most recently from 9561de1 to 8c11ccb Compare May 27, 2024 09:03
joey0612
joey0612 previously approved these changes May 27, 2024
@joey0612
Copy link
Copy Markdown
Contributor

LGTM

@jingjunLi jingjunLi changed the title fix: prune block always keep 9W blocks in ancient db fix: keep 9W blocks in ancient db when prune block May 28, 2024
Copy link
Copy Markdown
Collaborator

@zzzckck zzzckck left a comment

Choose a reason for hiding this comment

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

LGTM.
1.This PR revert the upstream commit: https://github.com/ethereum/go-ethereum/pull/28683/files?diff=split&w=0
Actually, it is not that necessary, we can keep this commit. But it is ok to revert as well, we can get it back during our next code merge.
2.This PR will prevent use from pruning block of the most recent FullImmutabilityThreshold (9w) blocks.
3.But due to the multi-DB feature, the freeze logic is different now, it will only keep quite a few blocks in mainDB, so the these block access will only be kept in memory.
Simply describe it as bellow:
a.MultiDB:
--ancientDB: 0 -> finalizeHeight-1
--mainDB: finalizeHeight -> head
b.No MultiDB or MultiDB with --pruneancientdb:
--ancientDB: 0 -> head-9w
--mainDB: head-9w -> head

@zzzckck zzzckck merged commit 63e7eac into bnb-chain:develop May 29, 2024
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 this pull request may close these issues.

6 participants