sql: add old zone config in zone config logs #132677
Conversation
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
annrpom
force-pushed
the
zc-logging
branch
3 times, most recently
from
4c4a393
to
2e0e525
Compare
|
Tests are failing because I need to update some side effects files from our end to end testing; I'll update before merge |
annrpom
requested review from
angles-n-daemons,
arjunmahishi and
aa-joshi
and removed request for
a team
There was a problem hiding this comment.
nice work!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aa-joshi, @angles-n-daemons, @annrpom, and @arjunmahishi)
pkg/ccl/logictestccl/testdata/logic_test/event_log line 0 at r1 (raw file):
do you know why this had to be in the ccl package?
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/logictestccl/testdata/logic_test/event_log line at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
do you know why this had to be in the
cclpackage?
relevant pr: #132788
i'll make this a stacked PR on top of ^; it doesn't have to be in the ccl package after the above
With the deprecation of the core license, we no longer need to an enterprise license check for configuring replication controls regarding subzones. This patch mainly just performs clean-up -- as in real servers, we overwrite `base.CheckEnterpriseEnabled` with the one in `ccl/utilccl/license_check.go`, which always returns true now. Epic: None Release note: None
This patch adds another field in `eventpb.SetZoneConfig`. This field, `ResolvedOldConfig`, will aid in logging the zone configuration that we have changed. We use the `completeZone/completeSubzone` here because we want to be more useful in our zone config logging. The benefit of using the complete* is that we can explicitly see what fields are changing, even if they are actually modeled as a zone config that just inherits from its parent. The disadvantage of this is that we are not able to easily tell the inheritence of the old zone config, but knowing the actual diff between the old vs new config seems more relevant for debugging. Fixes: cockroachdb#127176 Release note: None
This patch enables logging for index and partition zone configs in the DSC. Release note: None
This patch fills the `ResolvedOldConfig` field in `eventpb.SetZoneConfig` to log the old zone configuration in the declarative schema changer. Informs: cockroachdb#127176 Release note: None
This PR is stacked on top of another one!
Please consider reviewing that PR first. This message will be removed once the prerequisite PR is merged and this one is rebased.
sql: add old zone config in zone config logs
This patch adds another field in
eventpb.SetZoneConfig.This field,
ResolvedOldConfig, will aid in logging the zone configurationthat we have changed.
We use the
completeZone/completeSubzonehere because we want tobe more useful in our zone config logging. The benefit of using the
complete* is that we can explicitly see what fields are changing,
even if they are actually modeled as a zone config that just inherits
from its parent. The disadvantage of this is that we are not able
to easily tell the inheritence of the old zone config, but knowing
the actual diff between the old vs new config seems more relevant
for debugging.
Fixes: #127176
Release note: None
schemachanger: log index, partition
This patch enables logging for index and partition
zone configs in the DSC.
Release note: None
schemachanger: add old zone config in zone config logs in DSC
This patch fills the
ResolvedOldConfigfield ineventpb.SetZoneConfigto log the old zone configurationin the declarative schema changer.
Informs: #127176
Release note: None