storage: fix DeprecatedReplicas migration

In 52333d1 we made the incorrect
assumption that a 19.2 node running a ChangeReplicas invocation is also
the node which would evaluate the corresponding EndTransaction request,
but this is only often true, not generally true.
If a 19.1 node ended up evaluating a trigger composed by a 19.2 node,
it would see an empty UpdatedReplicas slice and a zero NextReplicaID
and would go ahead and construct a broken descriptor from that which
it would then go on and disseminate through Raft. This caused the
affected range to be fatally corrupted.

The solution is simple: populate the old field for now as well and
remove that only in the next release.

Release note: None

pkg/storage/batcheval/cmd_end_transaction.go

@@ -1109,15 +1109,16 @@ func changeReplicasTrigger(
 
 	var desc roachpb.RangeDescriptor
 	if change.Desc != nil {
+		// Trigger proposed by a 19.2+ node (and we're a 19.2+ node as well).
 		desc = *change.Desc
 	} else {
+		// Trigger proposed by a 19.1 node. Reconstruct descriptor from deprecated
+		// fields.
 		desc = *rec.Desc()
 		desc.SetReplicas(roachpb.MakeReplicaDescriptors(change.DeprecatedUpdatedReplicas))
 		desc.NextReplicaID = change.DeprecatedNextReplicaID
 	}
 
-	// TODO(tschottdorf): duplication of Desc with the trigger below, should
-	// likely remove it from the trigger.
 	pd.Replicated.State = &storagepb.ReplicaState{
 		Desc: &desc,
 	}

pkg/storage/replica_command.go

@@ -1406,9 +1406,10 @@ func execChangeReplicasTxn(
 			deprecatedRepDesc = removed[0]
 		}
 		crt = &roachpb.ChangeReplicasTrigger{
-			DeprecatedChangeType: deprecatedChangeType,
-			DeprecatedReplica:    deprecatedRepDesc,
-			Desc:                 &updatedDesc,
+			DeprecatedChangeType:      deprecatedChangeType,
+			DeprecatedReplica:         deprecatedRepDesc,
+			DeprecatedUpdatedReplicas: updatedDesc.Replicas().All(),
+			DeprecatedNextReplicaID:   updatedDesc.NextReplicaID,
 		}
 	} else {
 		crt = &roachpb.ChangeReplicasTrigger{