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

change(state): Stop re-downloading blocks that are in state queues #6397

Closed
wants to merge 4 commits into from

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Mar 23, 2023

Motivation

This PR is a follow-up to #6335 to look for block hashes from Contains requests in state queues.

Depends-On: #6335

Solution

  • Add and prune finalized block hashes to sent_blocks in drain_queue_and_commit_finalized
  • Checks if the block hash is in:
    • any non-finalized chains
    • queued_non_finalized_blocks, queued_finalized_blocks, or sent_blocks
    • finalized best chain

Related changes:

  • Renames sent_non_finalized_block_hashes to sent_blocks

Review

Anyone can review.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@arya2 arya2 added C-bug Category: This is a bug C-enhancement Category: This is an improvement P-Medium ⚡ C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness I-usability Zebra is hard to understand or use A-state Area: State / database changes C-audit Category: Issues arising from audit findings labels Mar 23, 2023
@arya2 arya2 requested a review from a team as a code owner March 23, 2023 20:51
@arya2 arya2 self-assigned this Mar 23, 2023
@arya2 arya2 requested a review from a team as a code owner March 23, 2023 20:51
@arya2 arya2 requested review from teor2345 and removed request for a team March 23, 2023 20:51
@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Merging #6397 (0152c86) into state-contains-request (2c15632) will decrease coverage by 0.09%.
The diff coverage is 67.64%.

Additional details and impacted files
@@                    Coverage Diff                     @@
##           state-contains-request    #6397      +/-   ##
==========================================================
- Coverage                   77.83%   77.74%   -0.09%     
==========================================================
  Files                         304      304              
  Lines                       39614    39667      +53     
==========================================================
+ Hits                        30835    30841       +6     
- Misses                       8779     8826      +47     

@teor2345
Copy link
Contributor

It seems like this change makes syncing hang:

2023-03-24T05:02:09.109361Z WARN {net="Main"}: zebrad::components::sync::progress: chain updates have stalled, state height has not increased for 354 minutes. Hint: check your network connection, and your computer clock and time zone sync_percent=99.986% current_height=Height(2026663) network_upgrade=Nu5 time_since_last_state_block=5h 54m target_block_spacing=PT75S max_block_spacing=None is_syncer_stopped=false

https://github.com/ZcashFoundation/zebra/actions/runs/4505734009/jobs/7932315464?pr=6397

We decided that we wouldn't do any more work on this at the moment, but we can turn it into an optional ticket for later.

(It won't be an audit ticket, because duplicate queued blocks are already handled in the syncer and inbound downloader, we just don't handle duplicates across both of them. Which is ok for now.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-state Area: State / database changes C-audit Category: Issues arising from audit findings C-bug Category: This is a bug C-enhancement Category: This is an improvement C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness I-usability Zebra is hard to understand or use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants