Merge pull request #3945 from joostjager/node-level-penalty

routing: penalize node-level failures harder

routing/missioncontrol.go

@@ -333,18 +333,18 @@ func (m *MissionControl) setLastPairResult(fromNode, toNode route.Vertex,
 	nodePairs[toNode] = current
 }
 
-// setAllFail stores a fail result for all known connection of the given node.
+// setAllFail stores a fail result for all known connections to and from the
-func (m *MissionControl) setAllFail(fromNode route.Vertex,
+// given node.
+func (m *MissionControl) setAllFail(node route.Vertex,
 	timestamp time.Time) {
 
-	nodePairs, ok := m.lastPairResult[fromNode]
+	for fromNode, nodePairs := range m.lastPairResult {
-	if !ok {
+		for toNode := range nodePairs {
-		return
+			if fromNode == node || toNode == node {
-	}
+				nodePairs[toNode] = TimedPairResult{
-
+					FailTime: timestamp,
-	for connection := range nodePairs {
+				}
-		nodePairs[connection] = TimedPairResult{
+			}
-			FailTime: timestamp,
 		}
 	}
 }

routing/missioncontrol_test.go

@@ -186,8 +186,8 @@ func TestMissionControl(t *testing.T) {
 	// Check whether history snapshot looks sane.
 	history := ctx.mc.GetHistorySnapshot()
 
-	if len(history.Pairs) != 3 {
+	if len(history.Pairs) != 4 {
-		t.Fatalf("expected 3 pairs, but got %v", len(history.Pairs))
+		t.Fatalf("expected 4 pairs, but got %v", len(history.Pairs))
 	}
 
 	// Test reporting a success.

routing/result_interpretation.go

@@ -424,11 +424,13 @@ func (i *interpretedResult) failNode(rt *route.Route, idx int) {
 	// Mark the incoming connection as failed for the node. We intent to
 	// penalize as much as we can for a node level failure, including future
 	// outgoing traffic for this connection. The pair as it is returned by
-	// getPair is directed towards the failed node. Therefore we first
+	// getPair is penalized in the original and the reversed direction. Note
-	// reverse the pair. We don't want to affect the score of the node
+	// that this will also affect the score of the failing node's peers.
-	// sending towards the failing node.
+	// This is necessary to prevent future routes from keep going into the
+	// same node again.
 	incomingChannelIdx := idx - 1
 	inPair, _ := getPair(rt, incomingChannelIdx)
+	i.pairResults[inPair] = failPairResult(0)
 	i.pairResults[inPair.Reverse()] = failPairResult(0)
 
 	// If not the ultimate node, mark the outgoing connection as failed for
@@ -437,6 +439,7 @@ func (i *interpretedResult) failNode(rt *route.Route, idx int) {
 		outgoingChannelIdx := idx
 		outPair, _ := getPair(rt, outgoingChannelIdx)
 		i.pairResults[outPair] = failPairResult(0)
+		i.pairResults[outPair.Reverse()] = failPairResult(0)
 	}
 }
 

routing/result_interpretation_test.go

@@ -165,6 +165,8 @@ var resultTestCases = []resultTestCase{
 			pairResults: map[DirectedNodePair]pairResult{
 				getTestPair(1, 0): failPairResult(0),
 				getTestPair(1, 2): failPairResult(0),
+				getTestPair(0, 1): failPairResult(0),
+				getTestPair(2, 1): failPairResult(0),
 			},
 		},
 	},
@@ -182,6 +184,7 @@ var resultTestCases = []resultTestCase{
 			nodeFailure:        &hops[1],
 			pairResults: map[DirectedNodePair]pairResult{
 				getTestPair(1, 0): failPairResult(0),
+				getTestPair(0, 1): failPairResult(0),
 			},
 		},
 	},
@@ -233,6 +236,7 @@ var resultTestCases = []resultTestCase{
 					amt:     97,
 				},
 				getTestPair(4, 3): {},
+				getTestPair(3, 4): {},
 			},
 			finalFailureReason: &reasonError,
 			nodeFailure:        &hops[4],
@@ -257,6 +261,7 @@ var resultTestCases = []resultTestCase{
 					amt:     99,
 				},
 				getTestPair(3, 2): {},
+				getTestPair(2, 3): {},
 			},
 			finalFailureReason: &reasonError,
 			nodeFailure:        &hops[3],
@@ -284,6 +289,8 @@ var resultTestCases = []resultTestCase{
 				},
 				getTestPair(3, 2): {},
 				getTestPair(3, 4): {},
+				getTestPair(2, 3): {},
+				getTestPair(4, 3): {},
 			},
 			nodeFailure: &hops[3],
 		},
@@ -301,6 +308,7 @@ var resultTestCases = []resultTestCase{
 		expectedResult: &interpretedResult{
 			pairResults: map[DirectedNodePair]pairResult{
 				getTestPair(1, 0): {},
+				getTestPair(0, 1): {},
 			},
 			finalFailureReason: &reasonError,
 			nodeFailure:        &hops[1],