-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release-22.1: revert "sql: add database ID to sampled query log" #85026
release-22.1: revert "sql: add database ID to sampled query log" #85026
Conversation
4fa7e40
to
504b37f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained
FYI, there is an extraordinary release (v22.1.5) in progress which uses this commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 5 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @maryliag and @THardy98)
pkg/util/log/eventpb/telemetry.proto
line 63 at r1 (raw file):
// Statement fingerprint ID of the query. uint64 statement_fingerprint_id = 12 [(gogoproto.customname) = "StatementFingerprintID", (gogoproto.jsontag) = ',omitempty'];
from Rail's comment: keep the fingerprint as 13 and reserve 12
cockroachdb/blathers/backport-release-22.1-84195" Reverts: cockroachdb#84195, PR that introduces the DatabaseID to the `SampledQuery` telemetry log. This reverts commit 0b9023d, reversing changes made to 8265e2a. Removes the DatabaseID field from the SampledQuery telemetry log due to the potential of indefinite blocking in the case of a lease acquisition failure. Protobuf field not reserved as no official build was released with these changes yet. Release note (sql change): Removes the DatabaseID field from the SampledQuery telemetry log due to the potential of indefinite blocking in the case of a lease acquisition failure.
504b37f
to
841b4f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @maryliag)
pkg/util/log/eventpb/telemetry.proto
line 63 at r1 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
from Rail's comment: keep the fingerprint as 13 and reserve 12
Reserved 12 and reverted back to 13.
Small gen.go
change to accomodate reserved
fields in the proto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 5 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @maryliag and @THardy98)
pkg/util/log/eventpb/telemetry.proto
line 63 at r1 (raw file):
Previously, THardy98 (Thomas Hardy) wrote…
Reserved 12 and reverted back to 13.
Smallgen.go
change to accomodatereserved
fields in the proto.
same comment as the other PR, put the list of reserved as the last thing
Reverts: #84195, PR that introduces the
DatabaseID
to theSampledQuery
telemetry log.
This reverts commit 0b9023d, reversing
changes made to 8265e2a.
Removes the
DatabaseID
field from theSampledQuery
telemetry log due tothe potential of indefinite blocking in the case of a lease acquisition
failure. Protobuf field not reserved as no official build was released
with these changes yet.
Release note (sql change): Removes the DatabaseID field from the
SampledQuery telemetry log due to the potential of indefinite blocking
in the case of a lease acquisition failure.
Release justification: Category 2: Bug fixes and low-risk updates to new functionality