Skip to content
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

storage: increase maxSize of key-value to 512MB, in pebbleMVCCScanner #61818

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

sumeerbhola
Copy link
Collaborator

This is to work around the fact that the only enforcement
that limits writing large key-values is based on the
cluster setting kv.raft.command.max_size. So if someone
increases the setting and allows a large value to be
written, the read path will go into a panic.

Release note: None

This is to work around the fact that the only enforcement
that limits writing large key-values is based on the
cluster setting kv.raft.command.max_size. So if someone
increases the setting and allows a large value to be
written, the read path will go into a panic.

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

but later please come and explain why 128MB is the right number.
And also to consider if the right TODO is about better handling / rejection of the large write in the storage engine rather than relying on the KV setting to do anything.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @jordanlewis)

@sumeerbhola
Copy link
Collaborator Author

bors r+

@sumeerbhola
Copy link
Collaborator Author

And also to consider if the right TODO is about better handling / rejection of the large write in the storage engine rather than relying on the KV setting to do anything.

I've created #61822

@craig
Copy link
Contributor

craig bot commented Mar 11, 2021

Build succeeded:

@craig craig bot merged commit dec0fec into cockroachdb:release-20.2 Mar 11, 2021
michae2 added a commit to michae2/cockroach that referenced this pull request Aug 23, 2021
Addresses: cockroachdb#67400

Add sql.mutations.max_row_size.err, a new cluster setting similar to
sql.mutations.max_row_size.log, which limits the size of rows written to
the database. Statements trying to write a row larger than this will
fail with an error. (Internal queries will not fail with an error, but
will log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.)

We're reusing eventpb.CommonLargeRowDetails as the error type, out of
convenience.

Release note (ops change): A new cluster setting,
sql.mutations.max_row_size.err, was added, which limits the size of rows
written to the database (or individual column families, if multiple
column families are in use). Statements trying to write a row larger
than this will fail with a code 54000 (program_limit_exceeded) error.
(Internal queries writing a row larger than this will not fail, but will
log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.) This
limit is enforced for INSERT, UPSERT, and UPDATE statements. CREATE
TABLE AS, CREATE INDEX, ALTER TABLE, ALTER INDEX, IMPORT, and RESTORE
will not fail with an error, but will log LargeRowInternal events to the
SQL_INTERNAL_PERF channel. SELECT, DELETE, TRUNCATE, and DROP are not
affected by this limit. **Note that existing rows violating the limit
*cannot* be updated, unless the update shrinks the size of the row
below the limit, but *can* be selected, deleted, altered, backed-up, and
restored.** For this reason we recommend using the accompanying setting
sql.mutations.max_row_size.log in conjunction with
SELECT pg_column_size() queries to detect and fix any existing large
rows before lowering sql.mutations.max_row_size.err.

Release justification: Low risk, high benefit change to existing
functionality. This causes statements adding large rows to fail with an
error. Default is 512 MiB, which was the maximum KV size in 20.2 as of
cockroachdb#61818 and also the default
range_max_bytes in 21.1, meaning rows larger than this were not possible
in 20.2 and are not going to perform well in 21.1.
michae2 added a commit to michae2/cockroach that referenced this pull request Aug 23, 2021
Addresses: cockroachdb#67400

Add sql.mutations.max_row_size.err, a new cluster setting similar to
sql.mutations.max_row_size.log, which limits the size of rows written to
the database. Statements trying to write a row larger than this will
fail with an error. (Internal queries will not fail with an error, but
will log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.)

We're reusing eventpb.CommonLargeRowDetails as the error type, out of
convenience.

Release note (ops change): A new cluster setting,
sql.mutations.max_row_size.err, was added, which limits the size of rows
written to the database (or individual column families, if multiple
column families are in use). Statements trying to write a row larger
than this will fail with a code 54000 (program_limit_exceeded) error.
(Internal queries writing a row larger than this will not fail, but will
log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.) This
limit is enforced for INSERT, UPSERT, and UPDATE statements. CREATE
TABLE AS, CREATE INDEX, ALTER TABLE, ALTER INDEX, IMPORT, and RESTORE
will not fail with an error, but will log LargeRowInternal events to the
SQL_INTERNAL_PERF channel. SELECT, DELETE, TRUNCATE, and DROP are not
affected by this limit. **Note that existing rows violating the limit
*cannot* be updated, unless the update shrinks the size of the row
below the limit, but *can* be selected, deleted, altered, backed-up, and
restored.** For this reason we recommend using the accompanying setting
sql.mutations.max_row_size.log in conjunction with
SELECT pg_column_size() queries to detect and fix any existing large
rows before lowering sql.mutations.max_row_size.err.

Release justification: Low risk, high benefit change to existing
functionality. This causes statements adding large rows to fail with an
error. Default is 512 MiB, which was the maximum KV size in 20.2 as of
cockroachdb#61818 and also the default
range_max_bytes in 21.1, meaning rows larger than this were not possible
in 20.2 and are not going to perform well in 21.1.
michae2 added a commit to michae2/cockroach that referenced this pull request Aug 24, 2021
Addresses: cockroachdb#67400

Add sql.mutations.max_row_size.err, a new cluster setting similar to
sql.mutations.max_row_size.log, which limits the size of rows written to
the database. Statements trying to write a row larger than this will
fail with an error. (Internal queries will not fail with an error, but
will log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.)

We're reusing eventpb.CommonLargeRowDetails as the error type, out of
convenience.

Release note (ops change): A new cluster setting,
sql.mutations.max_row_size.err, was added, which limits the size of rows
written to the database (or individual column families, if multiple
column families are in use). Statements trying to write a row larger
than this will fail with a code 54000 (program_limit_exceeded) error.
(Internal queries writing a row larger than this will not fail, but will
log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.) This
limit is enforced for INSERT, UPSERT, and UPDATE statements. CREATE
TABLE AS, CREATE INDEX, ALTER TABLE, ALTER INDEX, IMPORT, and RESTORE
will not fail with an error, but will log LargeRowInternal events to the
SQL_INTERNAL_PERF channel. SELECT, DELETE, TRUNCATE, and DROP are not
affected by this limit. **Note that existing rows violating the limit
*cannot* be updated, unless the update shrinks the size of the row
below the limit, but *can* be selected, deleted, altered, backed-up, and
restored.** For this reason we recommend using the accompanying setting
sql.mutations.max_row_size.log in conjunction with
SELECT pg_column_size() queries to detect and fix any existing large
rows before lowering sql.mutations.max_row_size.err.

Release justification: Low risk, high benefit change to existing
functionality. This causes statements adding large rows to fail with an
error. Default is 512 MiB, which was the maximum KV size in 20.2 as of
cockroachdb#61818 and also the default
range_max_bytes in 21.1, meaning rows larger than this were not possible
in 20.2 and are not going to perform well in 21.1.
michae2 added a commit to michae2/cockroach that referenced this pull request Aug 24, 2021
Addresses: cockroachdb#67400

Add sql.mutations.max_row_size.err, a new cluster setting similar to
sql.mutations.max_row_size.log, which limits the size of rows written to
the database. Statements trying to write a row larger than this will
fail with an error. (Internal queries will not fail with an error, but
will log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.)

We're reusing eventpb.CommonLargeRowDetails as the error type, out of
convenience.

Release note (ops change): A new cluster setting,
sql.mutations.max_row_size.err, was added, which limits the size of rows
written to the database (or individual column families, if multiple
column families are in use). Statements trying to write a row larger
than this will fail with a code 54000 (program_limit_exceeded) error.
(Internal queries writing a row larger than this will not fail, but will
log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.) This
limit is enforced for INSERT, UPSERT, and UPDATE statements. CREATE
TABLE AS, CREATE INDEX, ALTER TABLE, ALTER INDEX, IMPORT, and RESTORE
will not fail with an error, but will log LargeRowInternal events to the
SQL_INTERNAL_PERF channel. SELECT, DELETE, TRUNCATE, and DROP are not
affected by this limit. **Note that existing rows violating the limit
*cannot* be updated, unless the update shrinks the size of the row
below the limit, but *can* be selected, deleted, altered, backed-up, and
restored.** For this reason we recommend using the accompanying setting
sql.mutations.max_row_size.log in conjunction with
SELECT pg_column_size() queries to detect and fix any existing large
rows before lowering sql.mutations.max_row_size.err.

Release justification: Low risk, high benefit change to existing
functionality. This causes statements adding large rows to fail with an
error. Default is 512 MiB, which was the maximum KV size in 20.2 as of
cockroachdb#61818 and also the default
range_max_bytes in 21.1, meaning rows larger than this were not possible
in 20.2 and are not going to perform well in 21.1.
craig bot pushed a commit that referenced this pull request Aug 24, 2021
67953: sql, kv: add sql.mutations.max_row_size guardrails r=rytaft,andreimatei a=michae2

**kv: set Batch.pErr during Batch.prepare**

If we fail to construct a Batch (e.g. fail to marshal a key or value)
then an error will be placed in the resultsBuf and the batch will not
actually be sent to the layers below. In this case we still need to set
Batch.pErr, so that Batch.MustPErr is able to return a roachpb.Error to
higher layers without panicking.

I imagine in practice we never fail to marshal the key or value, so we
have never seen this panic in the wild.

Release note: None

Release justification: Bug fix.

**sql: add sql.mutations.max_row_size.log guardrail (large row logging)**

Addresses: #67400

Add sql.mutations.max_row_size.log, a new cluster setting which
controls large row logging. Rows larger than this size will have their
primary keys logged to the SQL_PERF or SQL_INTERNAL_PERF channels
whenever the SQL layer puts them into the KV layer.

This logging takes place in rowHelper, which is used by both
row.Inserter and row.Updater. Most of the work is plumbing
settings.Values and SessionData into rowHelper, and adding a new
structured event type.

Release note (ops change): A new cluster setting,
sql.mutations.max_row_size.log, was added, which controls large row
logging. Whenever a row larger than this size is written (or a single
column family if multiple column families are in use) a LargeRow event
is logged to the SQL_PERF channel (or a LargeRowInternal event is logged
to SQL_INTERNAL_PERF if the row was added by an internal query). This
could occur for INSERT, UPSERT, UPDATE, CREATE TABLE AS, CREATE INDEX,
ALTER TABLE, ALTER INDEX, IMPORT, or RESTORE statements. SELECT, DELETE,
TRUNCATE, and DROP are not affected by this setting.

Release justification: Low risk, high benefit change to existing
functionality. This adds logging whenever a large row is written to the
database. Default is 64 MiB, which is also the default for
kv.raft.command.max_size, meaning on a cluster with default settings
statements writing these rows will fail with an error anyway.

**sql: add sql.mutations.max_row_size.err guardrail (large row errors)**

Addresses: #67400

Add sql.mutations.max_row_size.err, a new cluster setting similar to
sql.mutations.max_row_size.log, which limits the size of rows written to
the database. Statements trying to write a row larger than this will
fail with an error. (Internal queries will not fail with an error, but
will log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.)

We're reusing eventpb.CommonLargeRowDetails as the error type, out of
convenience.

Release note (ops change): A new cluster setting,
sql.mutations.max_row_size.err, was added, which limits the size of rows
written to the database (or individual column families, if multiple
column families are in use). Statements trying to write a row larger
than this will fail with a code 54000 (program_limit_exceeded) error.
(Internal queries writing a row larger than this will not fail, but will
log a LargeRowInternal event to the SQL_INTERNAL_PERF channel.) This
limit is enforced for INSERT, UPSERT, and UPDATE statements. CREATE
TABLE AS, CREATE INDEX, ALTER TABLE, ALTER INDEX, IMPORT, and RESTORE
will not fail with an error, but will log LargeRowInternal events to the
SQL_INTERNAL_PERF channel. SELECT, DELETE, TRUNCATE, and DROP are not
affected by this limit. **Note that existing rows violating the limit
*cannot* be updated, unless the update shrinks the size of the row
below the limit, but *can* be selected, deleted, altered, backed-up, and
restored.** For this reason we recommend using the accompanying setting
sql.mutations.max_row_size.log in conjunction with
SELECT pg_column_size() queries to detect and fix any existing large
rows before lowering sql.mutations.max_row_size.err.

Release justification: Low risk, high benefit change to existing
functionality. This causes statements adding large rows to fail with an
error. Default is 512 MiB, which was the maximum KV size in 20.2 as of
#61818 and also the default
range_max_bytes in 21.1, meaning rows larger than this were not possible
in 20.2 and are not going to perform well in 21.1.



Co-authored-by: Michael Erickson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants