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

Latch is_initial_block_download_finished flag #1124

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

pavel-kokolemin
Copy link
Contributor

No description provided.

Comment on lines 767 to 769
// TODO: Add a check for importing and reindex.

// TODO: Add a check for the chain trust.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether these TODOs are still relevant (and if so, what they actually mean).

Copy link
Collaborator

Choose a reason for hiding this comment

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

The first one: It's about reindexing. Reindexing is the process of reparsing blocks such that the database can be rebuilt. This is useful in cases of database corruption or when settings like "tx index" are changed. If the reindex process is happening, then it's always true that we're in the "initial block download" state.

The second one: Assuming a monolithic network that we work on during the mainnet, the chain-trust always goes up. Given that, on every release, we can say that a chain-trust below a certain point is always an "initial block download", because we know that the monotonous mainnet must be longer than that point.

Yes, both points are still relevant. But they're more like optimizations than necessities.

chainstate/src/detail/mod.rs Outdated Show resolved Hide resolved
Comment on lines +778 to +782
if self.is_fresh_block(&tip_timestamp) {
self.is_initial_block_download_finished.set();
}

Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether we should write is_initial_block_download_finished to the DB once it becomes true, so that if the node is relaunched, it doesn't get reset to false again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be reset when the node is shutdown, because the node could be down for a while and hence the "redownload" process should be started again.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be reset when the node is shutdown, because the node could be down for a while and hence the "redownload" process should be started again.

Something similar can happen when the network is down for a while but the node keeps on running. Once we're reconnected, the node has to catch up. How should that be handled? Seems the current PR prevents the node from reverting into the IBD stage in that case but maybe it should be handled somehow too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Under normal operation circumstances, the node should never fall back. Besides, the IBD state should be seen as an optimization, more than it being a global state.

@TheQuantumPhysicist TheQuantumPhysicist merged commit d52c5b5 into master Aug 2, 2023
23 checks passed
@TheQuantumPhysicist TheQuantumPhysicist deleted the latch_ibd_flag branch August 2, 2023 20:09
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.

None yet

4 participants