sqlbase: generalize IMPORTING state to OFFLINE

[dt]

This renames the IMPORTING TableDescriptor state to OFFLINE for reuse outside IMPORT,
and adds a free-form string field OfflineReason in which the process that marked the
table as OFFFLINE can record the reason to surface in user-visible error messages.

[cockroach-teamcity]

This change is 

[adityamaru27]

Reviewed 2 of 2 files at r1, 8 of 8 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @lucy-zhang)

[thoszhang]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @jordanlewis)

---

pkg/sql/sqlbase/structured.proto, line 556 at r2 (raw file):

  }
  optional State state = 19 [(gogoproto.nullable) = false];
  optional string offline_reason = 38 [(gogoproto.nullable) = false];

Do you think this should be an enum instead, in case we ever want to match on specific reasons?

[dt]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @lucy-zhang)

---

pkg/sql/sqlbase/structured.proto, line 556 at r2 (raw file):

Previously, lucy-zhang (Lucy Zhang) wrote…

Do you think this should be an enum instead, in case we ever want to match on specific reasons?

I thought about an enum but I ended up figuring if you need to match a specific reason and handle it differently, the table state enum already gets you that -- the value of Offline is that you don't need to modify the proto and all the switch statements if you want to take a table offline for a different reason.

[thoszhang]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @jordanlewis)

---

pkg/sql/sqlbase/structured.proto, line 556 at r2 (raw file):

Previously, dt (David Taylor) wrote…

I thought about an enum but I ended up figuring if you need to match a specific reason and handle it differently, the table state enum already gets you that -- the value of Offline is that you don't need to modify the proto and all the switch statements if you want to take a table offline for a different reason.

I was thinking offline_reason would be a separate enum, so that adding new values wouldn't affect the switch statements, etc. on state. I guess if there's a new reason for taking the table offline whose behavior is different enough from the existing States, then we would add a new State anyway. I think the string is fine.

[dt]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)