Merge pull request #10492 from petermattis/pmattis/replicate-queue-pr…

…iorities storage: prioritize up-replication over dead-replica removal
cockroachdb · Nov 7, 2016 · 3afe3ec · 3afe3ec
2 parents 80bef08 + 419fe71
commit 3afe3ec
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 41 deletions.
diff --git a/pkg/storage/allocator.go b/pkg/storage/allocator.go
@@ -40,8 +40,8 @@ const (
 	maxFractionUsedThreshold = 0.95
 
 	// priorities for various repair operations.
-	removeDeadReplicaPriority  float64 = 10000
-	addMissingReplicaPriority  float64 = 1000
+	addMissingReplicaPriority  float64 = 10000
+	removeDeadReplicaPriority  float64 = 1000
 	removeExtraReplicaPriority float64 = 100
 )
 

diff --git a/pkg/storage/allocator_test.go b/pkg/storage/allocator_test.go
@@ -799,7 +799,7 @@ func TestAllocatorComputeAction(t *testing.T) {
 		desc           roachpb.RangeDescriptor
 		expectedAction AllocatorAction
 	}{
-		// Needs three replicas, two are on dead stores.
+		// Needs Three replicas, have two
 		{
 			zone: config.ZoneConfig{
 				NumReplicas:   3,
@@ -815,23 +815,18 @@ func TestAllocatorComputeAction(t *testing.T) {
 						ReplicaID: 1,
 					},
 					{
-						StoreID:   7,
-						NodeID:    7,
-						ReplicaID: 7,
-					},
-					{
-						StoreID:   6,
-						NodeID:    6,
-						ReplicaID: 6,
+						StoreID:   2,
+						NodeID:    2,
+						ReplicaID: 2,
 					},
 				},
 			},
-			expectedAction: AllocatorRemoveDead,
+			expectedAction: AllocatorAdd,
 		},
-		// Needs three replicas, one is on a dead store.
+		// Needs Five replicas, have four.
 		{
 			zone: config.ZoneConfig{
-				NumReplicas:   3,
+				NumReplicas:   5,
 				Constraints:   config.Constraints{Constraints: []config.Constraint{{Value: "us-east"}}},
 				RangeMinBytes: 0,
 				RangeMaxBytes: 64000,
@@ -849,15 +844,20 @@ func TestAllocatorComputeAction(t *testing.T) {
 						ReplicaID: 2,
 					},
 					{
-						StoreID:   6,
-						NodeID:    6,
-						ReplicaID: 6,
+						StoreID:   3,
+						NodeID:    3,
+						ReplicaID: 3,
+					},
+					{
+						StoreID:   4,
+						NodeID:    4,
+						ReplicaID: 4,
 					},
 				},
 			},
-			expectedAction: AllocatorRemoveDead,
+			expectedAction: AllocatorAdd,
 		},
-		// Needs three replicas, one is dead.
+		// Needs three replicas, two are on dead stores.
 		{
 			zone: config.ZoneConfig{
 				NumReplicas:   3,
@@ -873,20 +873,20 @@ func TestAllocatorComputeAction(t *testing.T) {
 						ReplicaID: 1,
 					},
 					{
-						StoreID:   2,
-						NodeID:    2,
-						ReplicaID: 2,
+						StoreID:   7,
+						NodeID:    7,
+						ReplicaID: 7,
 					},
 					{
-						StoreID:   8,
-						NodeID:    8,
-						ReplicaID: 8,
+						StoreID:   6,
+						NodeID:    6,
+						ReplicaID: 6,
 					},
 				},
 			},
 			expectedAction: AllocatorRemoveDead,
 		},
-		// Needs five replicas, one is on a dead store.
+		// Needs three replicas, one is on a dead store.
 		{
 			zone: config.ZoneConfig{
 				NumReplicas:   3,
@@ -906,16 +906,6 @@ func TestAllocatorComputeAction(t *testing.T) {
 						NodeID:    2,
 						ReplicaID: 2,
 					},
-					{
-						StoreID:   3,
-						NodeID:    3,
-						ReplicaID: 3,
-					},
-					{
-						StoreID:   4,
-						NodeID:    4,
-						ReplicaID: 4,
-					},
 					{
 						StoreID:   6,
 						NodeID:    6,
@@ -925,7 +915,7 @@ func TestAllocatorComputeAction(t *testing.T) {
 			},
 			expectedAction: AllocatorRemoveDead,
 		},
-		// Needs Three replicas, have two
+		// Needs three replicas, one is dead.
 		{
 			zone: config.ZoneConfig{
 				NumReplicas:   3,
@@ -945,14 +935,19 @@ func TestAllocatorComputeAction(t *testing.T) {
 						NodeID:    2,
 						ReplicaID: 2,
 					},
+					{
+						StoreID:   8,
+						NodeID:    8,
+						ReplicaID: 8,
+					},
 				},
 			},
-			expectedAction: AllocatorAdd,
+			expectedAction: AllocatorRemoveDead,
 		},
-		// Needs Five replicas, have four.
+		// Needs five replicas, one is on a dead store.
 		{
 			zone: config.ZoneConfig{
-				NumReplicas:   5,
+				NumReplicas:   3,
 				Constraints:   config.Constraints{Constraints: []config.Constraint{{Value: "us-east"}}},
 				RangeMinBytes: 0,
 				RangeMaxBytes: 64000,
@@ -979,9 +974,14 @@ func TestAllocatorComputeAction(t *testing.T) {
 						NodeID:    4,
 						ReplicaID: 4,
 					},
+					{
+						StoreID:   6,
+						NodeID:    6,
+						ReplicaID: 6,
+					},
 				},
 			},
-			expectedAction: AllocatorAdd,
+			expectedAction: AllocatorRemoveDead,
 		},
 		// Need three replicas, have four.
 		{