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

Provide recent syncer response lengths as a watch channel #2602

Merged
merged 14 commits into from
Aug 19, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Aug 11, 2021

Motivation

We want Zebra to activate the mempool once it reaches the chain tip.

To find out when that happens, we need to know the recent syncer response lengths.

Specifications

We know that the mempool should only activate once Zebra reaches the chain tip, but there doesn't seem to be a specification for it.

Designs

As part of this ticket, I split the "Check Initial Sync" and "Chain Update" tokio tasks in the design.

Diagram: https://jamboard.google.com/d/1HUhdqx8afJUqgiBdBcjCw0aAfDod9R9KBJeAH24U1Qg/viewer?f=0
Scope: https://docs.google.com/document/d/10PP6wKlnS5gannRph_EQ8V9diR7byddf3W2YFZ564FU/edit#

Solution

Closes #2595.

Implementation and Tests

  • Add a sync lengths struct that turns obtain and extend tips lengths into a watch channel with recent lengths
  • Add tests for sync lengths
    • initially empty
    • new lengths go at the start (and are pruned from the end)
    • list length is limited

Monitoring

This code might be hard to test, so I've added metrics and a dashboard.
They will help us work out what happens when Zebra gets near the chain tip.
And we'll also be able to see what happens when the network or peers fail.

  • Add debug logs and metrics for each peer response, and for the number of queued downloads
    • Add a grafana dashboard
  • Add temporary info logs

Review

@jvff or @upbqdn can review this PR.
(It's based on the recent heights channel from PR #2519.)

We'll do more testing as part of ticket #2592.

In ticket #2592, I wrote down what happens to the sync lengths in different situations.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

We need to create a tokio task which turns these lengths into enable/disable mempool requests.

@teor2345 teor2345 added A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium labels Aug 11, 2021
@teor2345 teor2345 self-assigned this Aug 11, 2021
@teor2345 teor2345 marked this pull request as ready for review August 12, 2021 03:24
@teor2345 teor2345 requested a review from oxarbitrage August 12, 2021 03:24
@oxarbitrage oxarbitrage removed their request for review August 13, 2021 00:40
@teor2345 teor2345 requested a review from jvff August 16, 2021 22:03
Also includes metrics and logging, to make diagnosing bugs easier.
- initially empty
- pruned to correct length
- newest entries go first
This seems to be a nightly or beta Rust change,
but hopefully stable just accepts it.
This length makes it easier to distinguish between temporary and
sustained errors/syncs.
@teor2345 teor2345 force-pushed the recent-syncer-lengths branch from cf1de4d to 9d0a436 Compare August 19, 2021 04:24
@teor2345 teor2345 force-pushed the recent-syncer-lengths branch from 4c3acc3 to 9f20b7b Compare August 19, 2021 04:30
Copy link
Contributor Author

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

I applied your fixes and suggestions, except for SmallVec.

zebrad/src/components/sync/recent_sync_lengths.rs Outdated Show resolved Hide resolved
zebrad/src/components/sync/recent_sync_lengths.rs Outdated Show resolved Hide resolved
@teor2345 teor2345 requested a review from jvff August 19, 2021 04:33
@teor2345 teor2345 enabled auto-merge (squash) August 19, 2021 22:50
@teor2345 teor2345 merged commit 6f8f4d8 into main Aug 19, 2021
@teor2345 teor2345 deleted the recent-syncer-lengths branch August 19, 2021 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get recent obtain/extend lengths out of the sync service
2 participants