storage: verbose logging for conf changes

[tbg]

Cannot hurt to have this for #7224. It's a bit noisy during replication, but that is a lesser concern.

---

This change is

[tamird]

---

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks pending.

---

storage/replica.go, line 1858 [r1] (raw file):

  // TODO(tschottdorf): remove when #7224 is cleared.
  if ba.Txn != nil && ba.Txn.Name == replicaChangeTxnName {
      log.Infof("range %d: applied part of replica change txn: %s, pErr=%v",

s/pErr/rErr/, s/txn/batch/?

---

storage/replica_command.go, line 2755 [r1] (raw file):

          // We read the previous value, it wasn't what we supposedly used in
          // the CPut, but we still overwrote in the CPut above.
          panic(fmt.Sprintf("committed replica change, but oldDesc != assumedOldDesc:\n%+v\n%+v\nnew desc:\n%+v",

if you have to eat another round of CI: log.Fatalf

---

Comments from Reviewable

[petermattis]

Why are you doing this check here instead of above right after you read oldDesc?

[BramGruneir]

How slow is reflect.DeepEqual, I've always avoided reflect entirely in the main execution path.

[tbg]

@petermattis Because it's only here that the transaction has committed. Conceivably we might try to change replicas and the CPut would fail because of a race.

@BramGruneir this is pretty far off the hot execution path.

[BramGruneir]

LGTM

[tbg]

Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

storage/replica.go, line 1858 [r1] (raw file):

Previously, tamird (Tamir Duberstein) wrote…

s/pErr/rErr/, s/txn/batch/?

It's part of the txn, so s/txn/batch/ isn't quite right. `rErr` is a non-typical name for `pErr` (and I'm using `pErr` not for the variable name, but for "the usual *roachpb.Error`). CI is green, so unless you feel strongly I prefer to leave as-is.

---

storage/replica_command.go, line 2755 [r1] (raw file):

Previously, tamird (Tamir Duberstein) wrote…

if you have to eat another round of CI: log.Fatalf

As luck would have it, I don't.

---

Comments from Reviewable