cli: debug zip uses read-only range probe#108313
cli: debug zip uses read-only range probe#108313craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
abarganier
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @dhartunian, @j82w, @joshimhoff, @nvanbenschoten, and @Santamaura)
pkg/cli/zip_table_registry.go line 233 at r1 (raw file):
"crdb_internal.probe_ranges_1s_read_limit_100": { customQueryRedacted: `SELECT * FROM crdb_internal.probe_ranges(INTERVAL '1000ms', 'read') WHERE error != '' ORDER BY end_to_end_latency_ms DESC LIMIT 100;`, customQueryUnredacted: `SELECT * FROM crdb_internal.probe_ranges(INTERVAL '1000ms', 'read') WHERE error != '' ORDER BY end_to_end_latency_ms DESC LIMIT 100;`,
nit: is it worth putting a comment here noting the potential dangers of using write here, so future maintainers have that context?
joshimhoff
left a comment
There was a problem hiding this comment.
LGTM! Thanks. I agree with @abarganier that we should add a comment about the corruption bug.
To reduce the chance of corruption issues when run against older clusters, the recently added use of 'crdb_internal.probe_ranges` is modified to use a `read` probe instead of a `write` probe. See discussion in cockroachdb#107720 Release note: None Epic: None
d89c92b to
0f44cac
Compare
dhartunian
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/cli/zip_table_registry.go line 233 at r1 (raw file):
Previously, abarganier (Alex Barganier) wrote…
nit: is it worth putting a comment here noting the potential dangers of using
writehere, so future maintainers have that context?
Done.
|
bors r=joshimhoff |
|
thx for the fix! |
j82w
left a comment
There was a problem hiding this comment.
Reviewable status:
|
Build failed (retrying...): |
|
Build succeeded: |
To reduce the chance of corruption issues when run against older clusters, the recently added use of
crdb_internal.probe_rangesis modified to use areadprobe instead of awriteprobe.See discussion in #107720
Release note: None
Epic: None