storage: code movement

[tamird]

Moves some protos that should never have been in roachpb out into storage.

This change requires a stop-the-world upgrade because of RPC method renaming but we already have such a change on deck #8495.

---

This change is

[BramGruneir]

---

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 19 of 19 files at r3, 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks pending.

---

storage/api.proto, line 17 [r3] (raw file):

// Author: Tobias Schottdorf ([email protected])

syntax = "proto2";

What's the status on the proto3 vs proto2? Maybe make this a proto3?

---

storage/store.go, line 2299 [r2] (raw file):

      switch val.GetDetail().(type) {
      case *roachpb.ReplicaTooOldError:
          if err := s.stopper.RunTask(func() {

Any idea why this was in a runtask to begin with?

---

Comments from Reviewable

[BramGruneir]

This makes the code a lot more readable.

---

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

---

Comments from Reviewable

[tamird]

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

---

storage/api.proto, line 17 [r3] (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

What's the status on the proto3 vs proto2? Maybe make this a proto3?

It had to be proto2 because of the nullability of ReservationResponse.range_count, but that changed in the final commit here. Done.

---

storage/store.go, line 2299 [r2] (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Any idea why this was in a runtask to begin with?

None. @petermattis added it in https://github.com//pull/8172, perhaps he can comment? I couldn't see any reason for it.

---

Comments from Reviewable

[BramGruneir]

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

Comments from Reviewable

[bdarnell]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

storage/store.go, line 2299 [r2] (raw file):

Previously, tamird (Tamir Duberstein) wrote…

None. @petermattis added it in #8172, perhaps he can comment? I couldn't see any reason for it.

Please don't make functional changes in PRs titled "code movement", especially without waiting for a response from the person called out as most knowledgeable about the code.

---

Comments from Reviewable

[petermattis]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

storage/store.go, line 2299 [r2] (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Please don't make functional changes in PRs titled "code movement", especially without waiting for a response from the person called out as most knowledgeable about the code.

I believe the intention was to avoid calling `replicaGCQueue.Add` when the stopper had been stopped. Seems related to #8551.

PS Please wait before merging something like this if you want someone's opinion.

---

Comments from Reviewable