Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

bugfix: notify p2p downloader to pull next piece after reporting #1308

Merged
merged 1 commit into from
Apr 29, 2020

Conversation

lowzj
Copy link
Member

@lowzj lowzj commented Apr 27, 2020

Signed-off-by: lowzj zj3142063@gmail.com

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes #1307

should be merged into branch: master, 1.0.x

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

p2p.maxTimeout *= 2
}
actual, expected := p2p.sleepInterval()
logrus.Infof("pull piece task(%+v) result:%s and sleep actual:%.3fs expected:%.3fs",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to record expected sleep interval which is actually a random value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's more like a debug log, but I think it's necessary to output it in info level if actual > expected.

@@ -68,11 +71,12 @@ type ClientStreamWriter struct {
}

// NewClientStreamWriter creates and initialize a ClientStreamWriter instance.
func NewClientStreamWriter(clientQueue queue.Queue, api api.SupernodeAPI, cfg *config.Config) *ClientStreamWriter {
func NewClientStreamWriter(clientQueue, notifyQueue queue.Queue, api api.SupernodeAPI, cfg *config.Config) *ClientStreamWriter {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should abstract an interface for client writer, WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed, there is an ambitious plan to do this, but not in this pr.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

@starnop
Copy link
Contributor

starnop commented Apr 28, 2020

LGTM.

@starnop
Copy link
Contributor

starnop commented Apr 28, 2020

Well, I think I understand what you want to do, and LGTM for this PR.
However, it is difficult to understand simply with code only, I think some comments should be added for the sleepInterval function and notifyQueue field. 😄

@lowzj
Copy link
Member Author

lowzj commented Apr 28, 2020

Well, I think I understand what you want to do, and LGTM for this PR.
However, it is difficult to understand simply with code only, I think some comments should be added for the sleepInterval function and notifyQueue field. 😄

DONE

Signed-off-by: lowzj <zj3142063@gmail.com>
@starnop starnop merged commit 77c995d into dragonflyoss:master Apr 29, 2020
@lowzj lowzj deleted the fix-sleep branch May 3, 2020 06:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug This is bug report for project size/M
Development

Successfully merging this pull request may close these issues.

bug: dfget sleep much time to wait for pulling next downloading piece
3 participants