This repository has been archived by the owner on Feb 27, 2023. It is now read-only.
feature: limit the number which download from supernode for one task #819
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
starnop
force-pushed
the
supernode-task-limit
branch
2 times, most recently
from
b3b0e6e to
17196e8
Compare
starnop
changed the title
lowzj
reviewed
Aug 16, 2019
starnop
force-pushed
the
supernode-task-limit
branch
from
17196e8 to
987d1c0
Compare
starnop
force-pushed
the
supernode-task-limit
branch
3 times, most recently
from
f947b6e to
b6ab252
Compare
starnop
force-pushed
the
supernode-task-limit
branch
from
b6ab252 to
7f73758
Compare
starnop
force-pushed
the
supernode-task-limit
branch
2 times, most recently
from
a13ee3b to
86aa7a8
Compare
Codecov Report
@@ Coverage Diff @@
## master #819 +/- ##
==========================================
- Coverage 39.67% 39.44% -0.23%
==========================================
Files 109 110 +1
Lines 6415 6464 +49
==========================================
+ Hits 2545 2550 +5
- Misses 3661 3704 +43
- Partials 209 210 +1
Continue to review full report at Codecov.
|
lowzj
reviewed
Aug 21, 2019
| pm.superLoad.add(taskID, newSuperLoadState()) | ||
| } | ||
|
|
||
| if loadState.loadValue.Add(delta) > limit && limit > 0 { |
There was a problem hiding this comment.
The loadState maybe nil. It's not the object we created above when it's not exist before. We should get it again from map after adding. Otherwise we may use different objects with same taskId in multi-goroutines. It's better to make the get and add as one atomic operation.
starnop
force-pushed
the
supernode-task-limit
branch
3 times, most recently
from
349b528 to
1d80d21
Compare
lowzj
reviewed
Aug 30, 2019
| // when a peer success to download a piece from supernode, | ||
| // and the load of supernode for the taskID should be decremented by one. | ||
| if tm.cfg.IsSuperPID(pieceUpdateRequest.DstPID) { | ||
| _, err := tm.progressMgr.UpdateSuperLoad(ctx, taskID, -1, -1) |
There was a problem hiding this comment.
The load of supernode would not be decreased here because that the limit is less than zero and the if statement below would not execute the code loadState.loadValue.Add(delta).
if limit > 0 && loadState.loadValue.Add(delta) > limit {
loadState.loadValue.Add(-delta)
return false, nil
}There was a problem hiding this comment.
Thanks for pointing that. And I have updated it. PTAL again. THX>
Signed-off-by: Starnop <[email protected]>
starnop
force-pushed
the
supernode-task-limit
branch
from
1d80d21 to
718f2fc
Compare
|
LGTM |
starnop
pushed a commit
to starnop/Dragonfly
that referenced
this pull request
Nov 27, 2019
feature: limit the number which download from supernode for one task
inoc603
pushed a commit
to inoc603/Dragonfly
that referenced
this pull request
Dec 23, 2019
feature: limit the number which download from supernode for one task
sungjunyoung
pushed a commit
to sungjunyoung/Dragonfly
that referenced
this pull request
May 8, 2022
* feat: support list plugin Signed-off-by: Jim Ma <[email protected]> * chore: optmize plugin output Signed-off-by: Jim Ma <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Starnop [email protected]
Ⅰ. Describe what this PR did
When dragonfly begin to distribute a new file, a large number of download requests will be dispatched to the supernode node because there are no nodes in the current P2P network owns the file. Then the load of supernode will become heavier and heavier. According to the test, sometimes one peer will cost almost 2 minutes to download a piece which is only 4M from supernode(NOTE: the network bandwidth is enough). In order to avoid this, we should add a limit which only allows a limited number of downloads from supernode for one task at the same time. And then the others will download the piece form the peers which have downloaded from the supernode.
This will greatly reduce the load of the supernode.
Ⅱ. Does this pull request fix one issue?
None.
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
None.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews