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

[Fix] Supernode local cdn connet check is not necessary when --cdnPattern=s… #1518

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PWZER
Copy link

@PWZER PWZER commented Dec 2, 2020

…ource

Ⅰ. Describe what this PR did

当 cdn = "source" 时,supernode 机器上的 nginx 服务是非必须的。而 dfget 请求 /peer/task 时返回的 PeerIP, PeerPort 分别是 supernodeIp,8001。这个时候是不需要对 supernodeIp:8001 进行检查的。

Ⅱ. Does this pull request fix one issue?

NONE

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

Yes

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot
Copy link
Collaborator

We found this is your first time to contribute to Dragonfly, @PWZER
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/dragonflyoss/Dragonfly/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

@PWZER PWZER force-pushed the master branch 3 times, most recently from 917e42f to ad27d3f Compare December 2, 2020 04:32
…ource

Signed-off-by: PWZER <pwzergo@gmail.com>
Signed-off-by: PWZER <pwzergo@gmail.com>
@starnop
Copy link
Contributor

starnop commented Dec 11, 2020

Strictly speaking, we don't need an Nginx file server for supernode within source pattern. Thanks for your contribution.

@@ -119,7 +119,7 @@ func (pc *PowerClient) downloadPiece() (content *pool.Buffer, e error) {
peerPort := pc.pieceTask.PeerPort

// check that the target download peer is available
if dstIP != "" && dstIP != pc.node {
if dstIP != "" && dstIP != pc.node && pc.pieceTask.Path != pc.cfg.URL {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use pc.cdnSource != apiTypes.CdnSourceSource is better? WDYT?

Copy link
Author

@PWZER PWZER Dec 11, 2020

Choose a reason for hiding this comment

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

@starnop There are three scenarios.

  • pc.cdnSource == apiTypes.CdnSourceSource && dstIP != supernodeIp, connect check is very necessary.
  • Only pc.pieceTask.Path != pc.cfg.URL, need download piece from other peer, and dstIP != supernodeIp, connect check is very necessary.
  • Only pc.pieceTask.Path == pc.cfg.URL, need download piece from source,and dstIP == supernodeIp, connect check is not necessary.

@PWZER PWZER requested a review from starnop December 11, 2020 03:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants