sql: fix dangling zone config on REGIONAL BY ROW -> REGIONAL BY TABLE #60659
Conversation
|
This change is |
otan
force-pushed
the
fix_zone_config_master
branch
2 times, most recently
from
badb85f to
05ce19c
Compare
There was a problem hiding this comment.
Nit: Not following what you mean by "...since it no longer things the RBR zone config is a placeholder". Can you clarify?
Also, last sentence may be clearer if you say "But this patch ensures that SHOW ZONE CONFIGURATION will return the same result for tables created RBT as those that have been altered from RBR to RBT."
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @otan)
pkg/sql/region_util.go, line 563 at r1 (raw file):
// Mark the NumReplicas as 0 if we have subzones but no other features // in the zone config. This signifies a placeholder. if len(zoneConfig.Subzones) > 0 && isPlaceholderZoneConfigForMultiRegion(zoneConfig) {
Nit: in this check, can we not use "hasNewSubzones"? If there are existing subzones, will the NumReplicas not already be set to 0?
We previously did not set NumReplicas to 0 for RBR -> RBT conversions. This means the zone configuration do not cleanup and inherit properly after the async job completes for dropping old indexes since it no longer thinks the RBR zone config is a placeholder, leaving a dangling reference. Patch this up correctly. N.B.: In theory there's nothing wrong with this. But this patch makes SHOW ZONE CONFIGURATION consistent when you convert RBR to RBT compared to making a RBT table directly. Release note: None
otan
force-pushed
the
fix_zone_config_master
branch
from
05ce19c to
718034b
Compare
There was a problem hiding this comment.
Oops typo. see the new commit msg!
Reviewable status:
pkg/sql/region_util.go, line 563 at r1 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: in this check, can we not use "hasNewSubzones"? If there are existing subzones, will the NumReplicas not already be set to 0?
nah -- added a comment. basically, we can have no new subzones but subzones still one the zoneconfig in a RBR - > RBT migration
|
bors r=ajstorm |
|
Build succeeded: |
We previously did not set NumReplicas to 0 for RBR -> RBT conversions.
This means the zone configuration do not cleanup and inherit properly
after the async job completes for dropping old indexes since it no
longer thinks the RBR zone config is a placeholder, leaving a
dangling reference. Patch this up correctly.
N.B.: In theory there's nothing wrong with this. But this patch makes
SHOW ZONE CONFIGURATION consistent when you convert RBR to RBT
compared to making a RBT table directly.
Release note: None