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

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

Merged
merged 5 commits into from
May 29, 2024

Conversation

jingjunLi
Copy link
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
  • ...

cmd/geth/snapshot.go Outdated Show resolved Hide resolved
cmd/geth/snapshot.go Outdated Show resolved Hide resolved
cmd/geth/snapshot.go Outdated Show resolved Hide resolved
fynnss
fynnss previously approved these changes May 27, 2024
sysvm
sysvm previously approved these changes May 27, 2024
Copy link
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

@@ -1082,6 +1082,7 @@ Please note that --` + MetricsHTTPFlag.Name + ` must be set to start the server.
Name: "block-amount-reserved",
Usage: "Sets the expected remained amount of blocks for offline block prune",
Category: flags.BlockHistoryCategory,
Value: 90000,
Copy link
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
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
joeylichang
joeylichang previously approved these changes May 27, 2024
@joeylichang
Copy link
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
core/rawdb/database.go Show resolved Hide resolved
core/blockchain_repair_test.go Show resolved Hide resolved
core/blockchain_sethead_test.go Show resolved Hide resolved
core/rawdb/chain_freezer.go Show resolved Hide resolved
core/rawdb/chain_freezer.go Outdated Show resolved Hide resolved
Copy link
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
7 checks passed
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