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

[1.0] Limit sync-fetch-span by max-reversible-blocks if needed #545

Merged
merged 8 commits into from
Aug 18, 2024

Conversation

heifner
Copy link
Member

@heifner heifner commented Aug 14, 2024

Example output:

warn  2024-08-16T12:56:55.440 net-7     net_plugin.cpp:2021           active_sync_fetch_sp ] sync-fetch-span 5000 restricted to 3270 by max-reversible-blocks 3600, fork_db_size 329
info  2024-08-16T12:56:55.440 net-7     net_plugin.cpp:2179           operator()           ] ["jungle4.cryptolions.io:9870 - 34355da" - 2 95.216.44.76:9870] requesting range 1480342 to 1483611, fhead 1480341, lib 1480012

Resolves #528

@heifner heifner added the OCI Work exclusive to OCI team label Aug 14, 2024
@heifner heifner changed the base branch from main to release/1.0 August 15, 2024 18:14
@heifner heifner changed the title Limit sync-fetch-span by max-reversible-blocks if needed [1.0] Limit sync-fetch-span by max-reversible-blocks if needed Aug 15, 2024
@heifner heifner marked this pull request as ready for review August 15, 2024 18:57
,sync_state(in_sync)
,min_blocks_distance(min_blocks_distance)
{
}

uint32_t sync_manager::active_sync_fetch_span() const {
auto fork_db_size = my_impl->chain_plug->chain().fork_db_size();
auto reversible_remaining = max_reversible_blocks - fork_db_size;
Copy link
Contributor

Choose a reason for hiding this comment

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

We shutdown when fork_db_size() >= max_reversible_blocks, so really we should have:

Suggested change
auto reversible_remaining = max_reversible_blocks - fork_db_size;
auto reversible_remaining = max_reversible_blocks - fork_db_size - 1;

I think.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to check max_reversible_blocks > fork_db_size before doing subtract?

// block was not applied, possibly because we already have the block
fc_dlog(logger, "Requesting ${fs} blocks ahead, head: ${h} fhead ${fh} blk_num: ${bn} sync_next_expected_num ${nen} "
"sync_last_requested_num: ${lrn}, sync_last_requested_block: ${lrb}",
("fs", fetch_span)("h", my_impl->get_chain_head_num())("fh", my_impl->get_fork_head_num())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
("fs", fetch_span)("h", my_impl->get_chain_head_num())("fh", my_impl->get_fork_head_num())
("fs", fetch_span)("h", head)("fh", my_impl->get_fork_head_num())

@ericpassmore
Copy link
Contributor

Note:start
group: STABILITY
category: INTERNALS
summary: Limit sync-fetch-span by max-reversible-blocks if needed.
Note:end

@heifner heifner merged commit 4c48ac7 into release/1.0 Aug 18, 2024
36 checks passed
@heifner heifner deleted the GH-528-limit-sync branch August 18, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2P: Limit requested blocks to avoid max-reversible-blocks
4 participants