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

[Downloader] Allow early cancel in downloader #2100

Merged
merged 1 commit into from Jan 26, 2024
Merged

Conversation

hyunsooda
Copy link
Contributor

@hyunsooda hyunsooda commented Jan 26, 2024

Proposed changes

Downloader.Cancel() signals fetcher threads to cancel their operation. Each listening thread terminates instantly upon receiving the cancel request. However, one of the six fetchers (importBlockResults) lacks a cancel listener, preventing instant termination.

For instance, the debug_setHead API requires a downloader cancel before executing the delete operation. Due to the issue mentioned, the delete operation does not instantly start. Since the block import unit is 2048 at maximum, which is a considerable number, operators may experience a significant delay, leading to confusion about the command's effectiveness.

This PR changes the insertion loop so that it can listen to the cancel request in the middle of up to 2048 block insertion. Previously, entire blocks downloaded should have been inserted before stopping.

Types of changes

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Related issues

Further comments

@blukat29 blukat29 changed the title [Downloader] Added error listener [Downloader] Allow early cancel in downloader Jan 26, 2024
Copy link
Contributor

@yoomee1313 yoomee1313 left a comment

Choose a reason for hiding this comment

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

Thank you!

@blukat29 blukat29 merged commit 2ca3b82 into dev Jan 26, 2024
11 checks passed
@blukat29 blukat29 deleted the added-import-err-checker branch January 26, 2024 08:30
@yoomee1313
Copy link
Contributor

yoomee1313 commented Jan 30, 2024

@hyunsooda Tested this PR by calling setHead(klay.blockNumber - 1) at baobab EN. The setHead returned immediately. Although it took over 5 minutes to sync again, I can see that downloader has no problem. For sure, take a look at next log.

log
INFO[01/30,02:05:05 Z] [5] Inserted a new block                      number=144999394 hash=c4bc23…1d2eac txs=2  gas=364074   elapsed=10.346ms   processTxs=2.179ms   finalize=2.687ms  validateState=417.985µs totalWrite=1.766604ms   trieWrite=1.620157ms
WARN[01/30,02:05:05 Z] [5] Rewinding blockchain                      target=144999392
INFO[01/30,02:05:05 Z] [5] Loaded most recent local header           number=144999392 hash=cd3679…e9845f td=144999393 age=2s199ms
INFO[01/30,02:05:05 Z] [5] Loaded most recent local full block       number=144999392 hash=cd3679…e9845f td=144999393 age=2s199ms
INFO[01/30,02:05:05 Z] [5] Loaded most recent local fast block       number=144999392 hash=cd3679…e9845f td=144999393 age=2s199ms
INFO[01/30,02:05:06 Z] [28] Downloader queue stats                    receiptTasks=0 blockTasks=0     stakingInfoTasks=0 itemSize=1.70kB throttle=8192
INFO[01/30,02:05:09 Z] [30] Governance votes are cleared              num=144547200
INFO[01/30,02:07:49 Z] [33] Added a single channel P2P Peer           id=026bf3c2c005375f conn=dyndial            peerID=026bf3c2c005375f
WARN[01/30,02:07:58 Z] [33] ProtocolManager failed to read msg        id=026bf3c2c005375f conn=dyndial            err=EOF


INFO[01/30,02:11:21 Z] [5] Inserted a new block                      number=144999393 hash=01447c…962ade txs=1  gas=196441   elapsed=6m15.826s  processTxs=550.002µs finalize=2.517ms  validateState=383.025µs totalWrite=1.110909ms   trieWrite=980.039µs

@hyunsooda
Copy link
Contributor Author

The delay taken 5min was for the snapshot regeneration. The snapshot regeneration is naturally triggered when it comes to starting sync. Looks like this PR had an effect.

@blukat29 blukat29 mentioned this pull request Jan 31, 2024
13 tasks
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

3 participants