-
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
distsqlrun: fail gracefully if out of memory during stats creation #39738
Conversation
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 (waiting on @justinj, @RaduBerinde, and @rytaft)
pkg/sql/distsqlpb/data.proto, line 282 at r1 (raw file):
optional uint64 rows_processed = 1 [(gogoproto.nullable) = false]; // Indicates that sample collection for histograms should be disabled, // likely because the sampler processor ran out of memory.
[nit] a sampler processor
This commit ensures that the sampler and sampleAggregator processors fail gracefully if they run out of memory. Instead of causing the entire CREATE STATISTICS job to fail in case of an out of memory error, they will continue to collect statistics for row counts, distinct counts and null counts, but will disable collection of histograms. Release note: None
e6eb5c6
to
a243141
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.
TFTR!
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @justinj and @RaduBerinde)
pkg/sql/distsqlpb/data.proto, line 282 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] a sampler processor
I think "the" sampler processor works here because it corresponds to whichever sampler processor is sending this metadata message (same reason we use "the" sampler processor in the line above for rows_processed).
Although I also don't really feel strongly -- happy to change it if you think it would be clearer with "a".
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 @justinj)
pkg/sql/distsqlpb/data.proto, line 282 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
I think "the" sampler processor works here because it corresponds to whichever sampler processor is sending this metadata message (same reason we use "the" sampler processor in the line above for rows_processed).
Although I also don't really feel strongly -- happy to change it if you think it would be clearer with "a".
👍
bors r+ |
Build failed |
bors r+ |
39738: distsqlrun: fail gracefully if out of memory during stats creation r=rytaft a=rytaft This commit ensures that the `sampler` and `sampleAggregator` processors fail gracefully if they run out of memory. Instead of causing the entire `CREATE STATISTICS` job to fail in case of an out of memory error, they will continue to collect statistics for row counts, distinct counts and null counts, but will disable collection of histograms. Release note: None Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
Build succeeded |
This commit ensures that the
sampler
andsampleAggregator
processorsfail gracefully if they run out of memory. Instead of causing the
entire
CREATE STATISTICS
job to fail in case of an out of memory error,they will continue to collect statistics for row counts, distinct
counts and null counts, but will disable collection of histograms.
Release note: None