storage: do not use non-authoritative Desc()

[tbg]

After ec00732, we were using a denormalized
copy of the descriptor which is not protected by the same mutex in places
where we shouldn't, resulting in subtle races. Change it back to use the
authoritative one almost everywhere, except for informational callsites
(such as stringifying a Replica).

Fixes #8086.
Fixes #8157.

---

This change is

[tamird]

---

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[petermattis]

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

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

// does not acquire a lock and is to be used whenever a descriptor is needed
// for informational purposes.
func (r *Replica) InformalDesc() *roachpb.RangeDescriptor {

Informal is an unusual term to use here. It doesn't adequately convey when this should be used. NonAuthoritative would be more accurate, but a mouthful. How about UnsafeDesc?

---

storage/store_test.go, line 1065 [r1] (raw file):

  if r := store.LookupReplica(roachpb.RKey("0"), nil); r != r0 {
      t.Errorf("mismatched range %+v != %+v", r.InformalDesc(), r0.InformalDesc())

I'm curious why these couldn't remain as .Desc().

---

Comments from Reviewable

[tamird]

Unsafe implies something different. Inconsistent?

[petermattis]

Inconsistent is good.

[tbg]

Can one of you take this setting that I'm out?

On Wed, Aug 3, 2016, 09:55 Peter Mattis [email protected] wrote:

Inconsistent is good.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#8283 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE135GMMSkjlgfdmAqO-nQdSa_BB7Bf-ks5qcMS9gaJpZM4JbhCB
.

-- Tobias

[tamird]

I got it.