Repository: dragonflyoss/Dragonfly2
Pull request #1697 (2022-09-22 01:29:57)

fix: grpc download tidy file error
Signed-off-by: Jim Ma <majinjing3@gmail.com>

<!--- Provide a general summary of your changes in the Title above -->

## Description

* Fix: grpc download a new tidy file error

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the **CONTRIBUTING** document.
- [ ] I have added tests to cover my changes.


codecov-commenter: # [Codecov](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) Report
> Merging [#1697](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) (fefe54f) into [main](https://codecov.io/gh/dragonflyoss/Dragonfly2/commit/9dd2168d621d9da60d88b82fe48cdc7bbdbb9cac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) (9dd2168) will **decrease** coverage by `0.00%`.
> The diff coverage is `33.33%`.

[![Impacted file tree graph](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697/graphs/tree.svg?width=650&height=150&src=pr&token=PUICJYSAT7&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)

```diff
@@            Coverage Diff             @@
##             main    #1697      +/-   ##
==========================================
- Coverage   41.76%   41.75%   -0.01%     
==========================================
  Files         125      125              
  Lines       15555    15535      -20     
==========================================
- Hits         6496     6487       -9     
+ Misses       8545     8534      -11     
  Partials      514      514              
```

| Flag | Coverage Δ | |
|---|---|---|
| Object-compatibility-e2etests | `?` | |
| e2etests | `?` | |
| unittests | `41.75% <33.33%> (-0.01%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) | Coverage Δ | |
|---|---|---|
| [client/daemon/peer/peertask\_manager\_mock.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wZWVyL3BlZXJ0YXNrX21hbmFnZXJfbW9jay5nbw==) | `36.81% <0.00%> (+0.16%)` | :arrow_up: |
| [client/daemon/peer/peertask\_manager.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wZWVyL3BlZXJ0YXNrX21hbmFnZXIuZ28=) | `25.67% <40.00%> (ø)` | |
| [client/daemon/rpcserver/rpcserver.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9ycGNzZXJ2ZXIvcnBjc2VydmVyLmdv) | `18.43% <100.00%> (+0.58%)` | :arrow_up: |
| [client/daemon/peer/peertask\_conductor.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1697/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wZWVyL3BlZXJ0YXNrX2NvbmR1Y3Rvci5nbw==) | `56.51% <0.00%> (-0.67%)` | :arrow_down: |


Patch:
diff --git a/client/daemon/peer/peertask_manager.go b/client/daemon/peer/peertask_manager.go
index 26ac9d92..30ed89c1 100644
--- a/client/daemon/peer/peertask_manager.go
+++ b/client/daemon/peer/peertask_manager.go
@@ -48,7 +48,7 @@ type TaskManager interface {
 	// return a progress channel for request download progress
 	// tiny stands task file is tiny and task is done
 	StartFileTask(ctx context.Context, req *FileTaskRequest) (
-		progress chan *FileTaskProgress, tiny *TinyData, err error)
+		progress chan *FileTaskProgress, err error)
 	// StartStreamTask starts a peer task with stream io
 	StartStreamTask(ctx context.Context, req *StreamTaskRequest) (
 		readCloser io.ReadCloser, attribute map[string]string, err error)
@@ -260,15 +260,15 @@ func (ptm *peerTaskManager) prefetchParentTask(request *schedulerv1.PeerTaskRequ
 	return prefetch
 }
 
-func (ptm *peerTaskManager) StartFileTask(ctx context.Context, req *FileTaskRequest) (chan *FileTaskProgress, *TinyData, error) {
+func (ptm *peerTaskManager) StartFileTask(ctx context.Context, req *FileTaskRequest) (chan *FileTaskProgress, error) {
 	if req.KeepOriginalOffset && !ptm.Prefetch {
-		return nil, nil, fmt.Errorf("please enable prefetch when use original offset feature")
+		return nil, fmt.Errorf("please enable prefetch when use original offset feature")
 	}
 	if ptm.Multiplex {
 		progress, ok := ptm.tryReuseFilePeerTask(ctx, req)
 		if ok {
 			metrics.PeerTaskCacheHitCount.Add(1)
-			return progress, nil, nil
+			return progress, nil
 		}
 	}
 	// TODO ensure scheduler is ok first
@@ -281,12 +281,12 @@ func (ptm *peerTaskManager) StartFileTask(ctx context.Context, req *FileTaskRequ
 	}
 	ctx, pt, err := ptm.newFileTask(ctx, req, limit)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	// FIXME when failed due to SchedulerClient error, relocate SchedulerClient and retry
 	progress, err := pt.Start(ctx)
-	return progress, pt.peerTaskConductor.tinyData, err
+	return progress, err
 }
 
 func (ptm *peerTaskManager) StartStreamTask(ctx context.Context, req *StreamTaskRequest) (io.ReadCloser, map[string]string, error) {
diff --git a/client/daemon/peer/peertask_manager_mock.go b/client/daemon/peer/peertask_manager_mock.go
index ae9ab12e..5ad97121 100644
--- a/client/daemon/peer/peertask_manager_mock.go
+++ b/client/daemon/peer/peertask_manager_mock.go
@@ -84,13 +84,12 @@ func (mr *MockTaskManagerMockRecorder) IsPeerTaskRunning(taskID interface{}) *go
 }
 
 // StartFileTask mocks base method.
-func (m *MockTaskManager) StartFileTask(ctx context.Context, req *FileTaskRequest) (chan *FileTaskProgress, *TinyData, error) {
+func (m *MockTaskManager) StartFileTask(ctx context.Context, req *FileTaskRequest) (chan *FileTaskProgress, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "StartFileTask", ctx, req)
 	ret0, _ := ret[0].(chan *FileTaskProgress)
-	ret1, _ := ret[1].(*TinyData)
-	ret2, _ := ret[2].(error)
-	return ret0, ret1, ret2
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
 }
 
 // StartFileTask indicates an expected call of StartFileTask.
diff --git a/client/daemon/rpcserver/rpcserver.go b/client/daemon/rpcserver/rpcserver.go
index 41402807..89d3fa99 100644
--- a/client/daemon/rpcserver/rpcserver.go
+++ b/client/daemon/rpcserver/rpcserver.go
@@ -485,32 +485,11 @@ func (s *server) doDownload(ctx context.Context, req *dfdaemonv1.DownRequest, st
 	}
 	log := logger.With("peer", peerTask.PeerId, "component", "downloadService")
 
-	peerTaskProgress, tiny, err := s.peerTaskManager.StartFileTask(ctx, peerTask)
+	peerTaskProgress, err := s.peerTaskManager.StartFileTask(ctx, peerTask)
 	if err != nil {
 		return dferrors.New(commonv1.Code_UnknownError, fmt.Sprintf("%s", err))
 	}
-	if tiny != nil {
-		err = stream.Send(&dfdaemonv1.DownResult{
-			TaskId:          tiny.TaskID,
-			PeerId:          tiny.PeerID,
-			CompletedLength: uint64(len(tiny.Content)),
-			Done:            true,
-			Output:          req.Output,
-		})
-		if err != nil {
-			log.Infof("send download result error: %s", err.Error())
-			return err
-		}
-		log.Infof("tiny file, wrote to output")
-		if req.Uid != 0 && req.Gid != 0 {
-			if err = os.Chown(req.Output, int(req.Uid), int(req.Gid)); err != nil {
-				log.Errorf("change own failed: %s", err)
-				return err
-			}
-		}
 
-		return nil
-	}
 	for {
 		select {
 		case p, ok := <-peerTaskProgress:


Test patch:
diff --git a/client/daemon/peer/peertask_manager_test.go b/client/daemon/peer/peertask_manager_test.go
index e7af5121..659a69f3 100644
--- a/client/daemon/peer/peertask_manager_test.go
+++ b/client/daemon/peer/peertask_manager_test.go
@@ -699,7 +699,7 @@ func (ts *testSpec) runFileTaskTest(assert *testifyassert.Assertions, require *t
 	defer func() {
 		assert.Nil(os.Remove(output))
 	}()
-	progress, _, err := mm.peerTaskManager.StartFileTask(
+	progress, err := mm.peerTaskManager.StartFileTask(
 		context.Background(),
 		&FileTaskRequest{
 			PeerTaskRequest: schedulerv1.PeerTaskRequest{