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

sql, kv: add sql.mutations.max_row_size guardrails #67953

Merged
merged 3 commits into from
Aug 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions docs/generated/eventlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -1879,6 +1879,29 @@ are only emitted via external logging.
Events in this category are logged to the `SQL_PERF` channel.


### `large_row`

An event of type `large_row` is recorded when a statement tries to write a row larger than
cluster setting `sql.mutations.max_row_size.log` to the database. Multiple
LargeRow events will be recorded for statements writing multiple large rows.
LargeRow events are recorded before the transaction commits, so in the case
of transaction abort there will not be a corresponding row in the database.




#### Common fields

| Field | Description | Sensitive |
|--|--|--|
| `Timestamp` | The timestamp of the event. Expressed as nanoseconds since the Unix epoch. | no |
| `EventType` | The type of the event. | no |
| `RowSize` | | no |
| `TableID` | | no |
| `FamilyID` | | no |
| `PrimaryKey` | | yes |
| `ViolatesMaxRowSizeErr` | | no |

### `slow_query`

An event of type `slow_query` is recorded when a query triggers the "slow query" condition.
Expand Down Expand Up @@ -1929,6 +1952,27 @@ are only emitted via external logging.
Events in this category are logged to the `SQL_INTERNAL_PERF` channel.


### `large_row_internal`

An event of type `large_row_internal` is recorded when an internal query tries to write a row
larger than cluster settings `sql.mutations.max_row_size.log` or
`sql.mutations.max_row_size.err` to the database.




#### Common fields

| Field | Description | Sensitive |
|--|--|--|
| `Timestamp` | The timestamp of the event. Expressed as nanoseconds since the Unix epoch. | no |
| `EventType` | The type of the event. | no |
| `RowSize` | | no |
| `TableID` | | no |
| `FamilyID` | | no |
| `PrimaryKey` | | yes |
| `ViolatesMaxRowSizeErr` | | no |

### `slow_query_internal`

An event of type `slow_query_internal` is recorded when a query triggers the "slow query" condition,
Expand Down
2 changes: 2 additions & 0 deletions docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ sql.metrics.statement_details.plan_collection.period duration 5m0s the time unti
sql.metrics.statement_details.threshold duration 0s minimum execution time to cause statement statistics to be collected. If configured, no transaction stats are collected.
sql.metrics.transaction_details.enabled boolean true collect per-application transaction statistics
sql.multiregion.drop_primary_region.enabled boolean true allows dropping the PRIMARY REGION of a database if it is the last region
sql.mutations.max_row_size.err byte size 512 MiB maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an error is returned; setting to 0 disables large row errors
sql.mutations.max_row_size.log byte size 64 MiB maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an event is logged to SQL_PERF (or SQL_INTERNAL_PERF if the mutating statement was internal); setting to 0 disables large row logging
sql.notices.enabled boolean true enable notices in the server/client protocol being sent
sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled boolean false if enabled, uniqueness checks may be planned for mutations of UUID columns updated with gen_random_uuid(); otherwise, uniqueness is assumed due to near-zero collision probability
sql.spatial.experimental_box2d_comparison_operators.enabled boolean false enables the use of certain experimental box2d comparison operators
Expand Down
2 changes: 2 additions & 0 deletions docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@
<tr><td><code>sql.metrics.statement_details.threshold</code></td><td>duration</td><td><code>0s</code></td><td>minimum execution time to cause statement statistics to be collected. If configured, no transaction stats are collected.</td></tr>
<tr><td><code>sql.metrics.transaction_details.enabled</code></td><td>boolean</td><td><code>true</code></td><td>collect per-application transaction statistics</td></tr>
<tr><td><code>sql.multiregion.drop_primary_region.enabled</code></td><td>boolean</td><td><code>true</code></td><td>allows dropping the PRIMARY REGION of a database if it is the last region</td></tr>
<tr><td><code>sql.mutations.max_row_size.err</code></td><td>byte size</td><td><code>512 MiB</code></td><td>maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an error is returned; setting to 0 disables large row errors</td></tr>
<tr><td><code>sql.mutations.max_row_size.log</code></td><td>byte size</td><td><code>64 MiB</code></td><td>maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an event is logged to SQL_PERF (or SQL_INTERNAL_PERF if the mutating statement was internal); setting to 0 disables large row logging</td></tr>
<tr><td><code>sql.notices.enabled</code></td><td>boolean</td><td><code>true</code></td><td>enable notices in the server/client protocol being sent</td></tr>
<tr><td><code>sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled</code></td><td>boolean</td><td><code>false</code></td><td>if enabled, uniqueness checks may be planned for mutations of UUID columns updated with gen_random_uuid(); otherwise, uniqueness is assumed due to near-zero collision probability</td></tr>
<tr><td><code>sql.spatial.experimental_box2d_comparison_operators.enabled</code></td><td>boolean</td><td><code>false</code></td><td>enables the use of certain experimental box2d comparison operators</td></tr>
Expand Down
50 changes: 50 additions & 0 deletions pkg/ccl/backupccl/testdata/backup-restore/max-row-size
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
new-server name=m1
----

exec-sql
CREATE DATABASE orig;
USE orig;
CREATE TABLE maxrow (i INT PRIMARY KEY, s STRING);
INSERT INTO maxrow VALUES (1, repeat('x', 20000));
----

query-sql
SELECT i, pg_column_size(s) FROM maxrow ORDER BY i;
----
1 20004

exec-sql
SET CLUSTER SETTING sql.mutations.max_row_size.err = '16KiB';
----

query-sql
INSERT INTO maxrow VALUES (2, repeat('x', 20000))
----
pq: row larger than max row size: table 55 family 0 primary key /Table/55/1/2/0 size 20013

exec-sql
BACKUP maxrow TO 'nodelocal://1/maxrow';
CREATE DATABASE d2;
RESTORE maxrow FROM 'nodelocal://1/maxrow' WITH into_db='d2';
----

query-sql
SELECT i, pg_column_size(s) FROM d2.maxrow ORDER BY i;
----
1 20004

query-sql
INSERT INTO d2.maxrow VALUES (2, repeat('y', 20000));
----
pq: row larger than max row size: table 57 family 0 primary key /Table/57/1/2/0 size 20013

exec-sql
SET CLUSTER SETTING sql.mutations.max_row_size.err = default;
INSERT INTO d2.maxrow VALUES (2, repeat('y', 20000));
----

query-sql
SELECT i, pg_column_size(s) FROM d2.maxrow ORDER BY i;
----
1 20004
2 20004
8 changes: 4 additions & 4 deletions pkg/ccl/importccl/import_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestConverterFlushesBatches(t *testing.T) {
}

ctx := context.Background()
evalCtx := tree.MakeTestingEvalContext(nil)
evalCtx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())

tests := []testSpec{
newTestSpec(ctx, t, csvFormat(), "testdata/csv/data-0"),
Expand Down Expand Up @@ -223,7 +223,7 @@ func TestImportIgnoresProcessedFiles(t *testing.T) {
defer log.Scope(t).Close(t)
ctx := context.Background()

evalCtx := tree.MakeTestingEvalContext(nil)
evalCtx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
flowCtx := &execinfra.FlowCtx{
EvalCtx: &evalCtx,
Cfg: &execinfra.ServerConfig{
Expand Down Expand Up @@ -324,7 +324,7 @@ func TestImportHonorsResumePosition(t *testing.T) {
pkBulkAdder := &doNothingKeyAdder{}
ctx := context.Background()

evalCtx := tree.MakeTestingEvalContext(nil)
evalCtx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
flowCtx := &execinfra.FlowCtx{
EvalCtx: &evalCtx,
Cfg: &execinfra.ServerConfig{
Expand Down Expand Up @@ -452,7 +452,7 @@ func TestImportHandlesDuplicateKVs(t *testing.T) {

batchSize := 13
defer row.TestingSetDatumRowConverterBatchSize(batchSize)()
evalCtx := tree.MakeTestingEvalContext(nil)
evalCtx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
flowCtx := &execinfra.FlowCtx{
EvalCtx: &evalCtx,
Cfg: &execinfra.ServerConfig{
Expand Down
4 changes: 3 additions & 1 deletion pkg/ccl/importccl/import_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -2517,7 +2517,9 @@ func (r *importResumer) dropTables(
// older-format (v1.1) descriptor. This enables ClearTableData to use a
// RangeClear for faster data removal, rather than removing by chunks.
empty[i].TableDesc().DropTime = dropTime
if err := gcjob.ClearTableData(ctx, execCfg.DB, execCfg.DistSender, execCfg.Codec, empty[i]); err != nil {
if err := gcjob.ClearTableData(
ctx, execCfg.DB, execCfg.DistSender, execCfg.Codec, &execCfg.Settings.SV, empty[i],
); err != nil {
return errors.Wrapf(err, "clearing data for table %d", empty[i].GetID())
}
}
Expand Down
14 changes: 8 additions & 6 deletions pkg/kv/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,15 @@ func (b *Batch) MustPErr() *roachpb.Error {
return b.pErr
}

func (b *Batch) prepare() error {
for _, r := range b.Results {
if r.Err != nil {
return r.Err
}
// validate that there were no errors while marshaling keys and values.
func (b *Batch) validate() error {
err := b.resultErr()
if err != nil {
// Set pErr just as sendAndFill does, so that higher layers can find it
// using MustPErr.
b.pErr = roachpb.NewError(err)
}
return nil
return err
}

func (b *Batch) initResult(calls, numRows int, raw bool, err error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ func sendAndFill(ctx context.Context, send SenderFunc, b *Batch) error {
// operation. The order of the results matches the order the operations were
// added to the batch.
func (db *DB) Run(ctx context.Context, b *Batch) error {
if err := b.prepare(); err != nil {
if err := b.validate(); err != nil {
return err
}
return sendAndFill(ctx, db.send, b)
Expand Down
9 changes: 9 additions & 0 deletions pkg/kv/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,15 @@ func TestBatch(t *testing.T) {
"bb": []byte("2"),
}
checkResults(t, expected, b.Results)

b2 := &kv.Batch{}
b2.Put(42, "the answer")
if err := db.Run(context.Background(), b2); !testutils.IsError(err, "unable to marshal key") {
t.Fatal("expected marshaling error from running bad put")
}
if err := b2.MustPErr(); !testutils.IsPError(err, "unable to marshal key") {
t.Fatal("expected marshaling error from MustPErr")
}
}

func TestDB_Scan(t *testing.T) {
Expand Down
13 changes: 9 additions & 4 deletions pkg/kv/kvserver/replica.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,20 @@ var disableSyncRaftLog = settings.RegisterBoolSetting(
false,
)

// MaxCommandSizeFloor is the minimum allowed value for the MaxCommandSize
// cluster setting.
const MaxCommandSizeFloor = 4 << 20 // 4MB
const (
// MaxCommandSizeFloor is the minimum allowed value for the
// kv.raft.command.max_size cluster setting.
MaxCommandSizeFloor = 4 << 20 // 4MB
// MaxCommandSizeDefault is the default for the kv.raft.command.max_size
// cluster setting.
MaxCommandSizeDefault = 64 << 20
)

// MaxCommandSize wraps "kv.raft.command.max_size".
var MaxCommandSize = settings.RegisterByteSizeSetting(
"kv.raft.command.max_size",
"maximum size of a raft command",
64<<20,
MaxCommandSizeDefault,
func(size int64) error {
if size < MaxCommandSizeFloor {
return fmt.Errorf("max_size must be greater than %s", humanizeutil.IBytes(MaxCommandSizeFloor))
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ func (txn *Txn) DelRange(ctx context.Context, begin, end interface{}) error {
// operation. The order of the results matches the order the operations were
// added to the batch.
func (txn *Txn) Run(ctx context.Context, b *Batch) error {
if err := b.prepare(); err != nil {
if err := b.validate(); err != nil {
return err
}
return sendAndFill(ctx, txn.Send, b)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ go_test(
"//pkg/util/log/channel",
"//pkg/util/log/eventpb",
"//pkg/util/log/logconfig",
"//pkg/util/log/logpb",
"//pkg/util/metric",
"//pkg/util/mon",
"//pkg/util/protoutil",
Expand Down
14 changes: 11 additions & 3 deletions pkg/sql/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,9 @@ func TruncateInterleavedIndexes(
resumeAt := resume
// Make a new txn just to drop this chunk.
if err := db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
rd := row.MakeDeleter(codec, table, nil /* requestedCols */)
rd := row.MakeDeleter(
codec, table, nil /* requestedCols */, &execCfg.Settings.SV, true, /* internal */
)
td := tableDeleter{rd: rd, alloc: alloc}
if err := td.init(ctx, txn, nil /* *tree.EvalContext */); err != nil {
return err
Expand Down Expand Up @@ -878,7 +880,10 @@ func (sc *SchemaChanger) truncateIndexes(
if err != nil {
return err
}
rd := row.MakeDeleter(sc.execCfg.Codec, tableDesc, nil /* requestedCols */)
rd := row.MakeDeleter(
sc.execCfg.Codec, tableDesc, nil /* requestedCols */, &sc.settings.SV,
true, /* internal */
)
td := tableDeleter{rd: rd, alloc: alloc}
if err := td.init(ctx, txn, nil /* *tree.EvalContext */); err != nil {
return err
Expand Down Expand Up @@ -2479,7 +2484,10 @@ func indexTruncateInTxn(
alloc := &rowenc.DatumAlloc{}
var sp roachpb.Span
for done := false; !done; done = sp.Key == nil {
rd := row.MakeDeleter(execCfg.Codec, tableDesc, nil /* requestedCols */)
rd := row.MakeDeleter(
execCfg.Codec, tableDesc, nil /* requestedCols */, &execCfg.Settings.SV,
evalCtx.SessionData.Internal,
)
td := tableDeleter{rd: rd, alloc: alloc}
if err := td.init(ctx, txn, evalCtx); err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/backfill/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ func (cb *ColumnBackfiller) RunColumnBackfillChunk(
requestedCols,
row.UpdaterOnlyColumns,
&cb.alloc,
&cb.evalCtx.Settings.SV,
cb.evalCtx.SessionData.Internal,
)
if err != nil {
return roachpb.Key{}, err
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ func (n *createTableNode) startExec(params runParams) error {
params.ExecCfg().Codec,
desc.ImmutableCopy().(catalog.TableDescriptor),
desc.PublicColumns(),
params.p.alloc)
params.p.alloc,
&params.ExecCfg().Settings.SV,
params.p.SessionData().Internal,
)
if err != nil {
return err
}
Expand Down
Loading