sql: update EXPERIMENTAL_RELOCATE to support relocation of non-voters

[aayushshah15]

This PR contains 3 commits:

sql: stop spuriously removing non-voters on EXPERIMENTAL_RELOCATE

Previously, a call to EXPERIMENTAL_RELOCATE.. (when relocating
replicas, not the lease) would incorrectly remove all non-voters on the
target range(s). This commit fixes the bug.

Release justification: bug fix
Release note: None

sql, parser: add EXPERIMENTAL_RELOCATE VOTERS as alias of
EXPERIMENTAL_RELOCATE

Release note (sql change): EXPERIMENTAL_RELOCATE VOTERS is now an
alias of EXPERIMENTAL_RELOCATE (not EXPERIMENTAL_RELOCATE LEASES).

sql, parser: add EXPERIMENTAL_RELOCATE NON_VOTERS

This commit adds a new variant of the EXPERIMENTAL_RELOCATE statement
which enables relocation of non-voting replicas via SQL.

Release note (sql change): EXPERIMENTAL_RELOCATE now has a new
variant: EXPERIMENTAL_RELOCATE NON_VOTERS which allows users to
relocate non-voting replicas via SQL.

[cockroach-teamcity]

This change is 

[nvanbenschoten]

This all . Is your intention to backport this to the v21.1 release branch?

Reviewed 1 of 1 files at r1, 5 of 5 files at r2, 15 of 15 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @aayushshah15)

---

pkg/sql/relocate.go, line 138 at r1 (raw file):

	} else {
		if err := params.p.ExecCfg().DB.AdminRelocateRange(
			params.ctx, rowKey, relocationTargets, rangeDesc.Replicas().NonVoters().ReplicationTargets(),

nit: consider pulling this into a variable like existingNonVoters := ... to make the intention more clear.

Same thing with existingVoters later on in this PR.

---

pkg/sql/parser/sql.y, line 1697 at r2 (raw file):

    }
  }
| ALTER TABLE table_name relocate_kw VOTERS select_stmt

Should we add a voters_kw target that includes VOTERS or | /* EMPTY */, to avoid needing two targets here and below?

---

pkg/sql/sem/tree/split.go, line 89 at r2 (raw file):

	}
	ctx.FormatNode(&node.TableOrIndex)
	ctx.WriteString(" EXPERIMENTAL_RELOCATE ")

nit: Should we keep this, since it's still common among all variants?

[aayushshah15]

Is your intention to backport this to the v21.1 release branch?

Anecdotally speaking, EXPERIMENTAL_RELOCATE seems to have come in handy in a bunch of support escalations in order to, say, quickly move a bunch of replicas off of a "bad" node for various reasons. Since the blast radius of this change is fairly small (and it's an experimental statement to begin with), I was thinking it'd be nice to have it in 21.1.

Do you have any concerns? I'll defer to you for the final call.

[aayushshah15]

TFTR!

bors r+

[aayushshah15]

bors r-

[craig]

Canceled.

[aayushshah15]

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nvanbenschoten)

---

pkg/sql/relocate.go, line 138 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

nit: consider pulling this into a variable like existingNonVoters := ... to make the intention more clear.

Same thing with existingVoters later on in this PR.

Done.

---

pkg/sql/parser/sql.y, line 1697 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Should we add a voters_kw target that includes VOTERS or | /* EMPTY */, to avoid needing two targets here and below?

Done.

---

pkg/sql/sem/tree/split.go, line 89 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

nit: Should we keep this, since it's still common among all variants?

Done.

[nvanbenschoten]

Since the blast radius of this change is fairly small (and it's an experimental statement to begin with), I was thinking it'd be nice to have it in 21.1.

This all checks out for me, so I'm ok backporting it into the release. Thanks for getting it out so quickly.

Reviewed 15 of 15 files at r5.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @aayushshah15)

[craig]

Build succeeded:

• GitHub CI (Cockroach)