config, sql: completing partial zone configs

[ridwanmsharif]

With cascading zone configs, as explained in the RFC it is possible
to get into situations where the zones find themselves in an invalid
state as a result of changes made to parent zone configs. This addresses
that issue by writing fields in tandem in the partial zone.

Release note (sql change): Setting only one bound for the range sizes or
setting per-replica constraints will implicitly set other fields in the
zone config for future validation purposes.

[cockroach-teamcity]

This change is 

[benesch]

Just to double-check my understanding, if I run

ALTER TABLE f CONFIGURE ZONE USING range_min_bytes = 4MB
ALTER RANGE default CONFIGURE ZONE USING range_max_bytes = 128MB

the update to the default max bytes won't propagate to f, right? I think I'd find this quite surprising as a user. I was expecting that

ALTER TABLE f CONFIGURE ZONE USING range_min_bytes = 4MB

would return an error, and we'd require users to do something like:

ALTER TABLE f CONFIGURE ZONE USING range_min_bytes = 4MB, range_max_bytes = COPY FROM PARENT

Thoughts? If there was a discussion on this somewhere, apologies! I'd also be curious to get @a-robinson's and @knz's thoughts if they haven't weighed in already.

[ridwanmsharif]

ALTER TABLE f CONFIGURE ZONE USING range_min_bytes = 4MB, range_max_bytes = COPY FROM PARENT

This is essentially what is happening here but implicitly (which I prefer). The issue of the changes to the default max bytes not applying to the child zone will exist regardless, unless we write validation code that validates all descendants when validating a given zone when making a change. There we have a penalty for sure. We talked about this being a problem in the RFC and thought that having these two pairs written in tandem would avoid problems later on fairly easily and it'd me appropriate for most cases. That was a while ago, we may have changed our mind since then. I think the reason we moved away from the descendants validation idea was also in part due to the complexity that would add (in addition to the time penalty), so this + some documentation of what the new behavior under cascading looks like, seemed preferable.

Reviewable status: complete! 0 of 0 LGTMs obtained

[benesch]

This is essentially what is happening here but implicitly (which I prefer)

Yes, exactly—it's the implicitness that scares me. To be clear I'm very much in favor of a solution that doesn't involve descendent validation. I just think that we need to force users to explicitly indicate that they understand when they set field A that they're setting B along with it.

[ridwanmsharif]

Not necessarily disagreeing but to be clear, you'd rather have the user figure out what the range_max_bytes are from the default and only then apply a change to them in tandem manually? Also, what about for per-replica constraints?
when someone does something like:
ALTER ... CONFIGURE ZONE USING constraints = '{+region=west: 2, +region=east: 3}'"
at this point we just solidify the num_replicas that was being applied to the zone.

So in this case, later on when a parent's num_replicas value reduces to < 5, we are rest assured the value for this zone is still >= 5 because of the implicit setting before. Are you saying you want the client to now have to do this instead?:
ALTER ... CONFIGURE ZONE USING num_replicas = num_ge_5, constraints = '{+region=west: 2, +region=east: 3}'"

Reviewable status: complete! 0 of 0 LGTMs obtained

[benesch]

Are you saying you want the client to now have to do this instead?:
ALTER ... CONFIGURE ZONE USING num_replicas = num_ge_5, constraints = '{+region=west: 2, +region=east: 3}'"

Yes, exactly. That way the user will hopefully realize that they are overriding both values. And in the common case where you only intend to set one of the values and want to copy the other from the parent, I think COPY FROM PARENT is a reasonable choice. So in your example:

ALTER ... CONFIGURE ZONE USING num_replicas = COPY FROM PARENT, constraints = '{+region=west: 2, +region=east: 3}'"

But before you go down this road I definitely want to hear what @knz and @a-robinson think.

[ridwanmsharif]

Taking a second stab at this in #32861. We're not moving forward with this approach.