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

feat: fix mempool sync start early #6767

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Jan 27, 2025

Description

Mempool sync can only start when is_synced is set to true.

  • An edge case was witnessed whereby mempool sync can start early if the entire network is generally
    slow to respond to pings. The ChainMetadataEvent::NetworkSilence event is transmitted if 3x
    consecutive ping cycles were started without corresponding pongs received, which sets
    is_synced to true. The moment that peer metadata is then received, block sync and mempool sync
    will occur at the same time.
  • Prolonged network silence is now detected in the Starting state. At startup, the first successful metadata message received from a peer will signal the transition to Listening, otherwise Listening will be notified of the network silence, which can act accordingly.
  • Additional debug logs were added to track all events that can set the is_synced to true.
  • Removed log-mdc from the dependencies as the relevant code is not used.

The edge case warning message must be monitored to see if it is a real issue:

warn!(
    target: LOG_TARGET,
    "Initial sync achieved based on event 'NetworkSilence'; this may not be true if the entire \
    network in general is slow to respond to pings"
);

Fixes #6766

Motivation and Context

Mempool sync "started" before block sync was achieved.

How Has This Been Tested?

System-level testing

What process can a PR reviewer use to test or verify this change?

Code review
System-level testing

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

Copy link

github-actions bot commented Jan 27, 2025

Test Results (CI)

    3 files    129 suites   37m 13s ⏱️
1 352 tests 1 352 ✅ 0 💤 0 ❌
4 054 runs  4 054 ✅ 0 💤 0 ❌

Results for commit 45212cc.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 27, 2025

Test Results (Integration tests)

36 tests   36 ✅  22m 52s ⏱️
11 suites   0 💤
 2 files     0 ❌

Results for commit 17a6d70.

♻️ This comment has been updated with latest results.

@hansieodendaal hansieodendaal marked this pull request as draft January 28, 2025 05:07
@hansieodendaal hansieodendaal force-pushed the ho_mempool_sync_start_early branch from 137f4ca to b26a8b0 Compare January 28, 2025 10:06
Mempool sync can only start when `is_synced` is set to `true`.
- An edge case was witnessed whereby mempool sync can start early if the entire network is generally
slow to respond to pings. The `ChainMetadataEvent::NetworkSilence` event is transmitted if 3x
consecutive ping cycles were started without corresponding pongs received, which sets
`is_synced` to `true`. The moment that peer metadata is then received, block sync and mempool sync
will occur at the same time. For the time being the consecutive ping cycle threshold was increased
to 4x and a warning message was added to let the user know this may be the case.
- Additional debug logs were added to track all events that can set the `is_synced` to `true`.
- Removed 'log-mdc' form the dependencies as the relevant code is not used
@hansieodendaal hansieodendaal force-pushed the ho_mempool_sync_start_early branch from b26a8b0 to e8a2911 Compare January 28, 2025 10:13
@hansieodendaal hansieodendaal marked this pull request as ready for review January 28, 2025 10:13
@SWvheerden SWvheerden merged commit 2a68bdf into tari-project:development Jan 29, 2025
16 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.

Base node initial sync triggers too early
2 participants