Repository: dragonflyoss/Dragonfly2
Pull request #1267 (2022-04-21 04:43:21)

Fix scheduler peer state
<!--- Provide a general summary of your changes in the Title above -->

## Description
- Fix scheduler peer state.
<!--- Describe your changes in detail -->

## 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.
- [x] I have read the **CONTRIBUTING** document.
- [ ] I have added tests to cover my changes.


codecov-commenter: # [Codecov](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) Report
> Merging [#1267](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) (4e66b0f) into [main](https://codecov.io/gh/dragonflyoss/Dragonfly2/commit/85a3237823b315b8c63248a62d39bb68e7cba051?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) (85a3237) will **decrease** coverage by `0.19%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267/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/1267?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)

```diff
@@            Coverage Diff             @@
##             main    #1267      +/-   ##
==========================================
- Coverage   46.08%   45.88%   -0.20%     
==========================================
  Files         135      135              
  Lines       11278    11279       +1     
==========================================
- Hits         5197     5175      -22     
- Misses       5516     5542      +26     
+ Partials      565      562       -3     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `45.88% <100.00%> (-0.20%)` | :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/1267?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) | Coverage Δ | |
|---|---|---|
| [scheduler/resource/peer.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-c2NoZWR1bGVyL3Jlc291cmNlL3BlZXIuZ28=) | `76.66% <100.00%> (ø)` | |
| [scheduler/scheduler/evaluator/evaluator\_base.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-c2NoZWR1bGVyL3NjaGVkdWxlci9ldmFsdWF0b3IvZXZhbHVhdG9yX2Jhc2UuZ28=) | `90.76% <100.00%> (+0.14%)` | :arrow_up: |
| [client/daemon/peer/peertask\_piecetask\_poller.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wZWVyL3BlZXJ0YXNrX3BpZWNldGFza19wb2xsZXIuZ28=) | `30.43% <0.00%> (-16.67%)` | :arrow_down: |
| [client/daemon/peer/peertask\_conductor.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/1267/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wZWVyL3BlZXJ0YXNrX2NvbmR1Y3Rvci5nbw==) | `58.19% <0.00%> (ø)` | |


Patch:
diff --git a/scheduler/resource/peer.go b/scheduler/resource/peer.go
index 6cc5696e..3caa1daf 100644
--- a/scheduler/resource/peer.go
+++ b/scheduler/resource/peer.go
@@ -353,6 +353,9 @@ func (p *Peer) ReplaceParent(parent *Peer) {
 
 // Depth represents depth of tree
 func (p *Peer) Depth() int {
+	p.mu.RLock()
+	defer p.mu.RUnlock()
+
 	node := p
 	var depth int
 	for node != nil {
@@ -381,22 +384,19 @@ func (p *Peer) Depth() int {
 
 // IsDescendant determines whether it is ancestor of peer
 func (p *Peer) IsDescendant(ancestor *Peer) bool {
-	p.mu.RLock()
-	defer p.mu.RUnlock()
-
 	return p.isDescendant(ancestor, p)
 }
 
 // IsAncestor determines whether it is descendant of peer
 func (p *Peer) IsAncestor(descendant *Peer) bool {
-	p.mu.RLock()
-	defer p.mu.RUnlock()
-
 	return p.isDescendant(p, descendant)
 }
 
 // isDescendant determines whether it is ancestor of peer
 func (p *Peer) isDescendant(ancestor, descendant *Peer) bool {
+	p.mu.RLock()
+	defer p.mu.RUnlock()
+
 	node := descendant
 	for node != nil {
 		parent, ok := node.LoadParent()
diff --git a/scheduler/scheduler/evaluator/evaluator_base.go b/scheduler/scheduler/evaluator/evaluator_base.go
index d771bdcd..3145f7f8 100644
--- a/scheduler/scheduler/evaluator/evaluator_base.go
+++ b/scheduler/scheduler/evaluator/evaluator_base.go
@@ -177,7 +177,8 @@ func calculateMultiElementAffinityScore(dst, src string) float64 {
 }
 
 func (eb *evaluatorBase) IsBadNode(peer *resource.Peer) bool {
-	if peer.FSM.Is(resource.PeerStateFailed) || peer.FSM.Is(resource.PeerStateLeave) || peer.FSM.Is(resource.PeerStatePending) {
+	if peer.FSM.Is(resource.PeerStateFailed) || peer.FSM.Is(resource.PeerStateLeave) || peer.FSM.Is(resource.PeerStatePending) ||
+		peer.FSM.Is(resource.PeerStateReceivedTiny) || peer.FSM.Is(resource.PeerStateReceivedSmall) || peer.FSM.Is(resource.PeerStateReceivedNormal) {
 		peer.Log.Debugf("peer is bad node because peer status is %s", peer.FSM.Current())
 		return true
 	}


Test patch:
diff --git a/scheduler/scheduler/evaluator/evaluator_base_test.go b/scheduler/scheduler/evaluator/evaluator_base_test.go
index 34aaac82..83e6d1f5 100644
--- a/scheduler/scheduler/evaluator/evaluator_base_test.go
+++ b/scheduler/scheduler/evaluator/evaluator_base_test.go
@@ -593,6 +593,54 @@ func TestEvaluatorBase_IsBadNode(t *testing.T) {
 				assert.True(isBadNode)
 			},
 		},
+		{
+			name:            "peer state is PeerStatePending",
+			peer:            resource.NewPeer(mockPeerID, mockTask, mockHost),
+			totalPieceCount: 1,
+			mock: func(peer *resource.Peer) {
+				peer.FSM.SetState(resource.PeerStatePending)
+			},
+			expect: func(t *testing.T, isBadNode bool) {
+				assert := assert.New(t)
+				assert.True(isBadNode)
+			},
+		},
+		{
+			name:            "peer state is PeerStateReceivedTiny",
+			peer:            resource.NewPeer(mockPeerID, mockTask, mockHost),
+			totalPieceCount: 1,
+			mock: func(peer *resource.Peer) {
+				peer.FSM.SetState(resource.PeerStateReceivedTiny)
+			},
+			expect: func(t *testing.T, isBadNode bool) {
+				assert := assert.New(t)
+				assert.True(isBadNode)
+			},
+		},
+		{
+			name:            "peer state is PeerStateReceivedSmall",
+			peer:            resource.NewPeer(mockPeerID, mockTask, mockHost),
+			totalPieceCount: 1,
+			mock: func(peer *resource.Peer) {
+				peer.FSM.SetState(resource.PeerStateReceivedSmall)
+			},
+			expect: func(t *testing.T, isBadNode bool) {
+				assert := assert.New(t)
+				assert.True(isBadNode)
+			},
+		},
+		{
+			name:            "peer state is PeerStateReceivedNormal",
+			peer:            resource.NewPeer(mockPeerID, mockTask, mockHost),
+			totalPieceCount: 1,
+			mock: func(peer *resource.Peer) {
+				peer.FSM.SetState(resource.PeerStateReceivedNormal)
+			},
+			expect: func(t *testing.T, isBadNode bool) {
+				assert := assert.New(t)
+				assert.True(isBadNode)
+			},
+		},
 		{
 			name:            "download costs does not meet the normal distribution and last cost is twenty times more than mean",
 			peer:            resource.NewPeer(mockPeerID, mockTask, mockHost),