cli: Warn users if --locality doesn't include "region"#57179
cli: Warn users if --locality doesn't include "region"#57179craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
pkg/cli/start.go
Outdated
| // if they've provided a --locality value which doesn't include a "region" tier. | ||
| if len(serverCfg.Locality.Tiers) > 0 { | ||
| if _, containsRegion := serverCfg.Locality.Find("region"); !containsRegion { | ||
| warningString := "The --locality flag does not contain a \"region\" tier. To add regions\n" + |
There was a problem hiding this comment.
maybe to enable multi-region functionality
| // (and optionally, "zones"). To push users in that direction, we warn them here | ||
| // if they've provided a --locality value which doesn't include a "region" tier. | ||
| if len(serverCfg.Locality.Tiers) > 0 { | ||
| if _, containsRegion := serverCfg.Locality.Find("region"); !containsRegion { |
There was a problem hiding this comment.
do we have to check whether region is the first tier, or does all out logic allow that?
ajstorm
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @otan)
pkg/cli/start.go, line 1128 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
do we have to check whether
regionis the first tier, or does all out logic allow that?
Doesn't matter which tier the region is in, so long as it's there somewhere.
pkg/cli/start.go, line 1129 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
maybe
to enable multi-region functionality
I contemplated something like that, but I found that the wording was too vague to be actionable. I wanted to have something very specific (and provide a doc link with more detail) to ensure that users could take this warning message and act on it with very little additional context.
With the new multi-region abstractions coming in 21.1, users will need to add regions to databases (and optionally, tables). The regions defined at the cluster level are derived from the locality flags, but we search explicitly for "region". As a result, if the user wishes to leverage the new abstractions, they must provide a "region" tier in the locality flag when starting their nodes. To push users in that direction, we warn on `cockroach start` if the supplied locality flag doesn't include a region tier. Release note (cli change): Warn users if locality flag doesn't contain a "region" tier.
fe7344b to
def8f6f
Compare
|
bors r=otan |
|
Build succeeded: |
With the new multi-region abstractions coming in 21.1, users will need
to add regions to databases (and optionally, tables). The regions
defined at the cluster level are derived from the locality flags, but we
search explicitly for "region". As a result, if the user wishes to
leverage the new abstractions, they must provide a "region" tier in the
locality flag when starting their nodes.
To push users in that direction, we warn on
cockroach startif thesupplied locality flag doesn't include a region tier.
Release note (cli change): Warn users if locality flag doesn't contain a
"region" tier.