Feat/fetch p2p network info dfdaemon by wangforthinker · Pull Request #5 · antsystem/Dragonfly

wangforthinker · 2020-03-10T04:39:00Z

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

…work if need update. fix bug

add some log for test.

fix not report to uploader.

fix requestManager get wrong front urls.

Feat/support local task reload

add heart beat for peer

lowzj · 2020-03-13T01:45:55Z

dfdaemon/localManager/finite_queue.go

+	"sync"
+)
+
+// finiteQueue provides a circle queue with capacity, and it will weed out the earlier item if full


pkg/queue 下面有个有限队列的实现，跟这个区别在于队列满时会阻塞。可否考虑将本实现也放到pkg/queue下面。
另外我觉得应该叫循环队列，但是我从实现上看更像个cache。

嗯这是一个有限长度的循环队列。已放在pkg/queue.

lowzj · 2020-03-13T01:57:57Z

dfdaemon/localManager/finite_queue.go

+type finiteQueue struct {
+	sync.Mutex
+	capacity  int
+	head 	  *itemNode


是否可以考虑使用 list.List 使代码更简洁。

lowzj · 2020-03-13T02:06:27Z

dfdaemon/localManager/local_downloader.go

+	uploaderAPI		api.UploaderAPI
+
+	// postNotifyUploader should be called after notify the local uploader finish
+	postNotifyUploader func(req *api.FinishTaskRequest)


没有初始化

初始化在这：
https://github.com/antsystem/Dragonfly/pull/5/files#diff-548ec6945f0e547258fe1d2be5f9d5bcR234

lowzj · 2020-03-13T02:29:47Z

dfget/core/uploader/uploader.go

 	ticker := time.NewTicker(5 * time.Millisecond)
 	defer ticker.Stop()
-	timeout := time.After(233 * time.Millisecond)
+	timeout := time.After(10 * time.Second)


这个为什么要改掉

之前是为了测试，已改回去。

lowzj · 2020-03-13T03:01:36Z

dfget/core/downloader/p2p_downloader/power_client.go

+		logrus.Infof("in downloadPiece by returnSrc, url: %s, header: %v, err: %d", pc.pieceTask.Url, header, err)
+	}else{
+		downloadRequest := pc.createDownloadRequest()
+		downloadRequest.PieceRange = fmt.Sprintf("0-%d", downloadRequest.PieceSize + config.PieceMetaSize)


这样改原有的下载逻辑都变了

已修改回原本的逻辑。

lowzj · 2020-03-13T06:42:01Z

dfget/core/downloader/p2p_downloader/power_client.go

+		buf := make([]byte, 128)
+		binary.BigEndian.PutUint32(buf, uint32((pieceSize)|(pieceSize)<<4))
+		content.Write(buf[:config.PieceHeadSize])
+		defer content.Write([]byte{config.PieceTailChar})


直接添加分片头尾信息有问题，因为分片的md5值是包含了：piece head + piece data + piece tail。
这种情况下考虑使用 NewLimitReaderWithLimiterAndMD5Sum ，下载前后将分片头尾信息放到md5计算中。

已修改。

lowzj · 2020-03-13T07:18:20Z

dfget/core/downloader/p2p_downloader/client_stream_writer.go

 func (csw *ClientStreamWriter) Read(p []byte) (n int, err error) {
 	n, err = csw.limitReader.Read(p)
+	csw.alreadyReadSize += int64(n)
+	if csw.alreadyReadSize == csw.expectReadSize {


这里是为了做什么？csw.alreadyReadSize += int64(n) 结果可能也大于 csw. expectReadSize

当读到的内容跟预计的一样多时，主动close(pipewriter)。不然请求方不会收到EOF。

lowzj · 2020-03-13T07:27:45Z

dfdaemon/transport/transport.go

 // fix resource release
 func (roundTripper *DFRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	if req.Header.Get("x-nydus-proxy-healthcheck") != "" {
+	if roundTripper.config.Extreme.SpecKeyOfDirectRet != "" {


这一段改造可否独立个函数出来，因为跟之前的逻辑基本都不一样，可在这里判断进入不同的处理函数中。

lowzj · 2020-03-18T08:02:40Z

pkg/queue/circle_queue.go

+		// remove the earliest item
+		i := q.internalRemoveTail()
+		if i != nil {
+			delete(q.itemMap, key)


The deleted element is not the tail of the list, it may delete nothing actually.
The stored value in list.Element may be like this:

type elementData struct { key string data interface{} }

So delete the value from map can be written:

delete(q.itemMap, i.Value.(*elementData).key)

lowzj · 2020-03-18T08:21:08Z

dfdaemon/scheduler/scheduler.go

+
+// if pieceRange == "" means all Pieces of file
+func (sm *SchedulerManager) SchedulerByTaskID(ctx context.Context, taskID string, srcCid string, pieceRange string, pieceSize int32) ([]*Result, error) {
+	sm.Lock()


这个锁的粒度似乎太大了，所有任务都需要串行调度，而且对于peer负载的更新也依赖于此锁

实际测试下来，调度所占耗时不到1%，暂时不影响。

lowzj · 2020-03-18T08:22:45Z

dfdaemon/scheduler/scheduler.go

+			PeerInfo: node.Basic,
+			Generation: sm.generation,
+			StartDownload: func(peerID string, generation int64) {
+				sm.downloadStartCh <- notifySt{peerID: peerID, generation: generation}


这里使用chan是为了实现异步更新?

lowzj · 2020-03-18T09:20:41Z

dfdaemon/proxy/proxy.go

 		WithRegistryMirror(c.RegistryMirror),
 		WithStreamDownloaderFactory(func() downloader.Stream {
-			// dfget.NewGetter(c.DFGetConfig())
+			dfget.NewGetter(c.DFGetConfig())


这行是不是多余了？

wangforthinker added 30 commits March 6, 2020 15:09

add init

9583a83

add code for local manager

db3ea61

add chan for local schedule to adjust peer load

a314b08

add code for local manager and local downloader

13601d0

add NO_URL resp code in Register api

aa981f7

add start sync network info

ff6a4b0

change local manager to single mode

e16349c

if local schedule failed, try to direct return src and update p2p net…

a49b32a

…work if need update. fix bug

when download from remote, if direct to source , not to verify the dstIP

22dba5c

fix download resource failed

7dd3f1d

init ratelimiter in power client

1854120

notify the client stream writer to finish to return

d395916

add length for test

4cbd748

fix more code to make it work.

8e298dc

add some log for test.

add report to uploader

078fad0

add some logs, note some code to fast run.

4d932e3

note the service down

c260388

add log for FetchP2PNetworkInfo

e9a01f9

fix not add taskState when add a new one.

f2bbd20

fix not report to uploader.

init local manager in daemon

9ed43a3

update range including piece meta size if p2p download

7db37c0

adapt to get range from header

d2f0125

optimize the close progress of client stream

66fe556

add numerical ware

2518240

add lock in numerical and add auto compute

3bbfb37

fix numerical get data error.

5b4b22f

add reset controll

92417c8

add cache in peer uploader and add unit test.

2cc1d53

add more numerical info

2e058b9

if url not fetch recently, dirently try to fecth from supernode.

d9ff274

fix requestManager get wrong front urls.

wangforthinker and others added 7 commits March 9, 2020 17:28

support local resouce cache to report to supernode

a8d1401

fix the uploader init failed

6bebf54

prior to get local task in download

64870ef

delete task if local task is expired

5431aa7

Merge pull request #6 from antsystem/feat/support-local-task-reload

1086c73

Feat/support local task reload

add heart beat for peer

6e82829

Merge pull request #7 from antsystem/feat/support-heart-beat

91bb821

add heart beat for peer

wangforthinker force-pushed the feat/fetch-p2p-network-info-dfdaemon branch from 79ef05b to d92072a Compare March 12, 2020 02:31

change specfical to config

f241659

wangforthinker force-pushed the feat/fetch-p2p-network-info-dfdaemon branch 2 times, most recently from e8ae41e to f241659 Compare March 12, 2020 07:06

lowzj reviewed Mar 13, 2020

View reviewed changes

wangforthinker force-pushed the feat/fetch-p2p-network-info-dfdaemon branch 3 times, most recently from 0364ad6 to 4d1a730 Compare March 16, 2020 08:03

lowzj reviewed Mar 18, 2020

View reviewed changes

fix some code by CR

5b9b471

wangforthinker force-pushed the feat/fetch-p2p-network-info-dfdaemon branch from 4d1a730 to 5b9b471 Compare March 19, 2020 02:27

Conversation

wangforthinker commented Mar 10, 2020

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants