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

Use setTimer in NetworkManager.startPeriodicCatchup #921

Merged
merged 1 commit into from Jun 3, 2020

Conversation

TrustHenry
Copy link
Member

These need to be replaced to use setTimer once TaskManager is fixed to use it.

Relates #872

These need to be replaced to use setTimer once TaskManager is fixed to use it.
@@ -430,6 +430,10 @@ public class NetworkManager
address, &onHandshakeComplete, &onFailedRequest);
}
}

connnectionTask(); // avoid delay
this.taskman.setTimer(this.node_config.retry_delay.msecs,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not correct. Previously the calling fiber was yielded, but here you are spawning a new one. You're going to end up with multiple fibers running within this class. This part should not be changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what I was curious about. Thank you for your clear answer.

Copy link
Member Author

Choose a reason for hiding this comment

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

The commit has been deleted.

@TrustHenry TrustHenry changed the title Use setTimer in NetworkManager.startPeriodicCatchup & connnectionTask Use setTimer in NetworkManager.startPeriodicCatchup Jun 3, 2020
@codecov
Copy link

codecov bot commented Jun 3, 2020

Codecov Report

Merging #921 into v0.x.x will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           v0.x.x     #921      +/-   ##
==========================================
- Coverage   90.45%   90.45%   -0.01%     
==========================================
  Files          71       71              
  Lines        5669     5668       -1     
==========================================
- Hits         5128     5127       -1     
  Misses        541      541              
Flag Coverage Δ
#integration 54.67% <100.00%> (-0.38%) ⬇️
#unittests 88.81% <100.00%> (-0.01%) ⬇️
Impacted Files Coverage Δ
source/agora/network/NetworkManager.d 78.16% <100.00%> (-0.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1cd919...8cbf08e. Read the comment docs.

@AndrejMitrovic AndrejMitrovic merged commit 095bc42 into bosagora:v0.x.x Jun 3, 2020
@TrustHenry TrustHenry deleted the start branch June 4, 2020 00:37
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

2 participants