From 00070019b42d866a0b3f77f5bb891ab3e1f1c84e Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Mon, 2 Nov 2020 10:15:29 -0500 Subject: [PATCH 01/14] workload/schemachange: screen for errors in dropTable op Release note: None --- .../schemachange/operation_generator.go | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 1f25aba5b1a8..fd57689b5961 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -92,6 +92,7 @@ var opsWithExecErrorScreening = map[opType]bool{ dropColumn: true, dropColumnDefault: true, dropColumnNotNull: true, + dropTable: true, dropSchema: true, renameColumn: true, @@ -800,7 +801,30 @@ func (og *operationGenerator) dropTable(tx *pgx.Tx) (string, error) { if err != nil { return "", err } - return fmt.Sprintf(`DROP TABLE %s`, tableName), nil + tableExists, err := tableExists(tx, tableName) + if err != nil { + return "", err + } + tableHasDependencies, err := tableHasDependencies(tx, tableName) + if err != nil { + return "", err + } + + dropBehavior := tree.DropBehavior(og.randIntn(3)) + + ifExists := og.randIntn(2) == 0 + dropTable := tree.DropTable{ + Names: []tree.TableName{*tableName}, + IfExists: ifExists, + DropBehavior: dropBehavior, + } + + codesWithConditions{ + {pgcode.UndefinedTable, !ifExists && !tableExists}, + {pgcode.DependentObjectsStillExist, dropBehavior != tree.DropCascade && tableHasDependencies}, + }.add(og.expectedExecErrors) + + return dropTable.String(), nil } func (og *operationGenerator) dropView(tx *pgx.Tx) (string, error) { From 52e1ec3e9b9e2e47778306869b8d530d38f3fb6b Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Mon, 2 Nov 2020 10:30:19 -0500 Subject: [PATCH 02/14] workload/schemachange: screen for errors in dropView op Release note: None --- .../schemachange/operation_generator.go | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index fd57689b5961..862c952fa402 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -93,6 +93,7 @@ var opsWithExecErrorScreening = map[opType]bool{ dropColumnDefault: true, dropColumnNotNull: true, dropTable: true, + dropView: true, dropSchema: true, renameColumn: true, @@ -832,7 +833,29 @@ func (og *operationGenerator) dropView(tx *pgx.Tx) (string, error) { if err != nil { return "", err } - return fmt.Sprintf(`DROP VIEW %s`, viewName), nil + viewExists, err := tableExists(tx, viewName) + if err != nil { + return "", err + } + viewHasDependencies, err := tableHasDependencies(tx, viewName) + if err != nil { + return "", err + } + + dropBehavior := tree.DropBehavior(og.randIntn(3)) + + ifExists := og.randIntn(2) == 0 + dropView := tree.DropView{ + Names: []tree.TableName{*viewName}, + IfExists: ifExists, + DropBehavior: dropBehavior, + } + + codesWithConditions{ + {pgcode.UndefinedTable, !ifExists && !viewExists}, + {pgcode.DependentObjectsStillExist, dropBehavior != tree.DropCascade && viewHasDependencies}, + }.add(og.expectedExecErrors) + return dropView.String(), nil } func (og *operationGenerator) renameColumn(tx *pgx.Tx) (string, error) { From 8264d84aff74d026d3fa4e83e917adcb793de864 Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Mon, 2 Nov 2020 17:22:50 -0500 Subject: [PATCH 03/14] workload/schemachange: fix bug in randTable where new name was returned The intention here is to return the table name which was fetched from the previous query. Previously, a new, unique name was being returned. Release note: None --- pkg/workload/schemachange/operation_generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 862c952fa402..70078d958e4e 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -1318,7 +1318,7 @@ func (og *operationGenerator) randTable( treeTableName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{ SchemaName: tree.Name(desiredSchema), ExplicitSchema: true, - }, tree.Name(fmt.Sprintf("table%d", og.newUniqueSeqNum()))) + }, tree.Name(tableName)) return &treeTableName, nil } From 18ea68e2b177e002cc2503a38a49b8467aa7bb3b Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Tue, 3 Nov 2020 10:13:58 -0500 Subject: [PATCH 04/14] workload/schemachange: expand sequence generation features Previously sequences would only be attached to the public schema by default. This commit enhances the workload by allowing sequences to belong to other schemas. Furthermore, this commit adds the possibility that a sequence can be owned by a column in a table which belongs to a different schema. Release note: None --- .../schemachange/operation_generator.go | 144 +++++++++++++++--- pkg/workload/schemachange/schemachange.go | 43 +++--- 2 files changed, 143 insertions(+), 44 deletions(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 70078d958e4e..0ba307adb390 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -31,12 +31,13 @@ import ( // seqNum may be shared across multiple instances of this, so it should only // be change atomically. type operationGeneratorParams struct { - seqNum *int64 - errorRate int - enumPct int - rng *rand.Rand - ops *deck - maxSourceTables int + seqNum *int64 + errorRate int + enumPct int + rng *rand.Rand + ops *deck + maxSourceTables int + sequenceOwnedByPct int } // The OperationBuilder has the sole responsibility of generating ops @@ -353,7 +354,40 @@ func (og *operationGenerator) createIndex(tx *pgx.Tx) (string, error) { } func (og *operationGenerator) createSequence(tx *pgx.Tx) (string, error) { - return fmt.Sprintf(`CREATE SEQUENCE "seq%d"`, og.newUniqueSeqNum()), nil + seqName, err := og.randSequence(tx, og.pctExisting(false), "") + if err != nil { + return "", err + } + + ifNotExists := og.randIntn(2) == 0 + + var seqOptions tree.SequenceOptions + // Decide if the sequence should be owned by a column. If so, it can + // set using the tree.SeqOptOwnedBy sequence option. + if og.randIntn(100) < og.params.sequenceOwnedByPct { + table, err := og.randTable(tx, og.pctExisting(true), "") + if err != nil { + return "", err + } + column, err := og.randColumn(tx, *table, og.pctExisting(true)) + if err != nil { + return "", err + } + + seqOptions = append( + seqOptions, + tree.SequenceOption{ + Name: tree.SeqOptOwnedBy, + ColumnItemVal: &tree.ColumnItem{TableName: table.ToUnresolvedObjectName(), ColumnName: tree.Name(column)}}, + ) + } + createSeq := &tree.CreateSequence{ + IfNotExists: ifNotExists, + Name: *seqName, + Options: seqOptions, + } + + return tree.Serialize(createSeq), nil } func (og *operationGenerator) createTable(tx *pgx.Tx) (string, error) { @@ -790,11 +824,16 @@ func (og *operationGenerator) dropIndex(tx *pgx.Tx) (string, error) { } func (og *operationGenerator) dropSequence(tx *pgx.Tx) (string, error) { - sequenceName, err := og.randSequence(tx, og.pctExisting(true)) + sequenceName, err := og.randSequence(tx, og.pctExisting(true), "") if err != nil { return "", err } - return fmt.Sprintf(`DROP SEQUENCE "%s"`, sequenceName), nil + ifExists := og.randIntn(2) == 0 + dropSeq := &tree.DropSequence{ + Names: tree.TableNames{*sequenceName}, + IfExists: ifExists, + } + return tree.Serialize(dropSeq), nil } func (og *operationGenerator) dropTable(tx *pgx.Tx) (string, error) { @@ -928,17 +967,17 @@ func (og *operationGenerator) renameIndex(tx *pgx.Tx) (string, error) { } func (og *operationGenerator) renameSequence(tx *pgx.Tx) (string, error) { - srcSequenceName, err := og.randSequence(tx, og.pctExisting(true)) + srcSequenceName, err := og.randSequence(tx, og.pctExisting(true), "") if err != nil { return "", err } - destSequenceName, err := og.randSequence(tx, og.pctExisting(false)) + destSequenceName, err := og.randSequence(tx, og.pctExisting(false), "") if err != nil { return "", err } - return fmt.Sprintf(`ALTER SEQUENCE "%s" RENAME TO "%s"`, srcSequenceName, destSequenceName), nil + return fmt.Sprintf(`ALTER SEQUENCE %s RENAME TO %s`, srcSequenceName, destSequenceName), nil } func (og *operationGenerator) renameTable(tx *pgx.Tx) (string, error) { @@ -1244,22 +1283,77 @@ ORDER BY random() return name, nil } -func (og *operationGenerator) randSequence(tx *pgx.Tx, pctExisting int) (string, error) { +// randSequence returns a sequence qualified by a schema +func (og *operationGenerator) randSequence( + tx *pgx.Tx, pctExisting int, desiredSchema string, +) (*tree.TableName, error) { + + if desiredSchema != "" { + if og.randIntn(100) >= pctExisting { + treeSeqName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{ + SchemaName: tree.Name(desiredSchema), + ExplicitSchema: true, + }, tree.Name(fmt.Sprintf("seq%d", og.newUniqueSeqNum()))) + return &treeSeqName, nil + } + q := fmt.Sprintf(` + SELECT sequence_name + FROM [SHOW SEQUENCES] + WHERE sequence_name LIKE 'seq%%' + AND sequence_schema = '%s' + ORDER BY random() + LIMIT 1; + `, desiredSchema) + + var seqName string + if err := tx.QueryRow(q).Scan(&seqName); err != nil { + treeSeqName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{}, "") + return &treeSeqName, err + } + + treeSeqName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{ + SchemaName: tree.Name(desiredSchema), + ExplicitSchema: true, + }, tree.Name(seqName)) + return &treeSeqName, nil + } + if og.randIntn(100) >= pctExisting { - return fmt.Sprintf(`seq%d`, og.newUniqueSeqNum()), nil + // Most of the time, this case is for creating sequences, so it + // is preferable that the schema exists. + randSchema, err := og.randSchema(tx, og.pctExisting(true)) + if err != nil { + treeSeqName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{}, "") + return &treeSeqName, err + } + treeSeqName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{ + SchemaName: tree.Name(randSchema), + ExplicitSchema: true, + }, tree.Name(fmt.Sprintf("seq%d", og.newUniqueSeqNum()))) + return &treeSeqName, nil } - const q = ` - SELECT sequence_name - FROM [SHOW SEQUENCES] - WHERE sequence_name LIKE 'seq%' -ORDER BY random() - LIMIT 1; -` - var name string - if err := tx.QueryRow(q).Scan(&name); err != nil { - return "", err + + q := ` + SELECT sequence_schema, sequence_name + FROM [SHOW SEQUENCES] + WHERE sequence_name LIKE 'seq%%' + ORDER BY random() + LIMIT 1; + ` + + var schemaName string + var seqName string + if err := tx.QueryRow(q).Scan(&schemaName, &seqName); err != nil { + treeTableName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{}, "") + return &treeTableName, err } - return name, nil + + treeSeqName := tree.MakeTableNameFromPrefix(tree.ObjectNamePrefix{ + SchemaName: tree.Name(schemaName), + ExplicitSchema: true, + }, tree.Name(seqName)) + return &treeSeqName, nil + } func (og *operationGenerator) randEnum(tx *pgx.Tx, pctExisting int) (tree.UnresolvedName, error) { diff --git a/pkg/workload/schemachange/schemachange.go b/pkg/workload/schemachange/schemachange.go index cd3546848390..513156603bfa 100644 --- a/pkg/workload/schemachange/schemachange.go +++ b/pkg/workload/schemachange/schemachange.go @@ -53,22 +53,24 @@ import ( //For example, an attempt to do something we don't support should be swallowed (though if we can detect that maybe we should just not do it, e.g). It will be hard to use this test for anything more than liveness detection until we go through the tedious process of classifying errors.: const ( - defaultMaxOpsPerWorker = 5 - defaultErrorRate = 10 - defaultEnumPct = 10 - defaultMaxSourceTables = 3 + defaultMaxOpsPerWorker = 5 + defaultErrorRate = 10 + defaultEnumPct = 10 + defaultMaxSourceTables = 3 + defaultSequenceOwnedByPct = 25 ) type schemaChange struct { - flags workload.Flags - dbOverride string - concurrency int - maxOpsPerWorker int - errorRate int - enumPct int - verbose int - dryRun bool - maxSourceTables int + flags workload.Flags + dbOverride string + concurrency int + maxOpsPerWorker int + errorRate int + enumPct int + verbose int + dryRun bool + maxSourceTables int + sequenceOwnedByPct int } var schemaChangeMeta = workload.Meta{ @@ -92,6 +94,8 @@ var schemaChangeMeta = workload.Meta{ s.flags.BoolVarP(&s.dryRun, `dry-run`, `n`, false, ``) s.flags.IntVar(&s.maxSourceTables, `max-source-tables`, defaultMaxSourceTables, `Maximum tables or views that a newly created tables or views can depend on`) + s.flags.IntVar(&s.sequenceOwnedByPct, `seq-owned-pct`, defaultSequenceOwnedByPct, + `Percentage of times that a sequence is owned by column upon creation.`) return s }, } @@ -142,12 +146,13 @@ func (s *schemaChange) Ops( for i := 0; i < s.concurrency; i++ { opGeneratorParams := operationGeneratorParams{ - seqNum: seqNum, - errorRate: s.errorRate, - enumPct: s.enumPct, - rng: rand.New(rand.NewSource(timeutil.Now().UnixNano())), - ops: ops, - maxSourceTables: s.maxSourceTables, + seqNum: seqNum, + errorRate: s.errorRate, + enumPct: s.enumPct, + rng: rand.New(rand.NewSource(timeutil.Now().UnixNano())), + ops: ops, + maxSourceTables: s.maxSourceTables, + sequenceOwnedByPct: s.sequenceOwnedByPct, } w := &schemaChangeWorker{ From 9ec8c0b53f083dd2c64585b057927f3f606177ae Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Tue, 3 Nov 2020 11:35:27 -0500 Subject: [PATCH 05/14] workload/schemachange: screen for errors in createSequence op Release note: None --- pkg/workload/schemachange/error_screening.go | 9 +++ .../schemachange/operation_generator.go | 76 +++++++++++++++---- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/pkg/workload/schemachange/error_screening.go b/pkg/workload/schemachange/error_screening.go index c55cfaca5cf9..3b5a70c633a9 100644 --- a/pkg/workload/schemachange/error_screening.go +++ b/pkg/workload/schemachange/error_screening.go @@ -36,6 +36,15 @@ func viewExists(tx *pgx.Tx, tableName *tree.TableName) (bool, error) { )`, tableName.Schema(), tableName.Object()) } +func sequenceExists(tx *pgx.Tx, seqName *tree.TableName) (bool, error) { + return scanBool(tx, `SELECT EXISTS ( + SELECT sequence_name + FROM information_schema.sequences + WHERE sequence_schema = $1 + AND sequence_name = $2 + )`, seqName.Schema(), seqName.Object()) +} + func columnExistsOnTable(tx *pgx.Tx, tableName *tree.TableName, columnName string) (bool, error) { return scanBool(tx, `SELECT EXISTS ( SELECT column_name diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 0ba307adb390..7a051c954487 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -84,11 +84,12 @@ type opType int var opsWithExecErrorScreening = map[opType]bool{ addColumn: true, - createTable: true, - createTableAs: true, - createView: true, - createEnum: true, - createSchema: true, + createSequence: true, + createTable: true, + createTableAs: true, + createView: true, + createEnum: true, + createSchema: true, dropColumn: true, dropColumnDefault: true, @@ -359,7 +360,27 @@ func (og *operationGenerator) createSequence(tx *pgx.Tx) (string, error) { return "", err } - ifNotExists := og.randIntn(2) == 0 + schemaExists, err := schemaExists(tx, seqName.Schema()) + if err != nil { + return "", err + } + sequenceExists, err := sequenceExists(tx, seqName) + if err != nil { + return "", err + } + + // If the sequence exists and an error should be produced, then + // exclude the IF NOT EXISTS clause from the statement. Otherwise, default + // to including the clause prevent all pgcode.DuplicateRelation errors. + ifNotExists := true + if sequenceExists && og.produceError() { + ifNotExists = false + } + + codesWithConditions{ + {code: pgcode.UndefinedSchema, condition: !schemaExists}, + {code: pgcode.DuplicateRelation, condition: sequenceExists && !ifNotExists}, + }.add(og.expectedExecErrors) var seqOptions tree.SequenceOptions // Decide if the sequence should be owned by a column. If so, it can @@ -369,18 +390,47 @@ func (og *operationGenerator) createSequence(tx *pgx.Tx) (string, error) { if err != nil { return "", err } - column, err := og.randColumn(tx, *table, og.pctExisting(true)) + tableExists, err := tableExists(tx, table) if err != nil { return "", err } - seqOptions = append( - seqOptions, - tree.SequenceOption{ - Name: tree.SeqOptOwnedBy, - ColumnItemVal: &tree.ColumnItem{TableName: table.ToUnresolvedObjectName(), ColumnName: tree.Name(column)}}, - ) + if !tableExists { + // If a duplicate sequence exists, then a new sequence will not be created. In this case, + // a pgcode.UndefinedTable will not occur. + if !sequenceExists { + og.expectedExecErrors.add(pgcode.UndefinedTable) + } + seqOptions = append( + seqOptions, + tree.SequenceOption{ + Name: tree.SeqOptOwnedBy, + ColumnItemVal: &tree.ColumnItem{TableName: table.ToUnresolvedObjectName(), ColumnName: "IrrelevantColumnName"}}, + ) + } else { + column, err := og.randColumn(tx, *table, og.pctExisting(true)) + if err != nil { + return "", err + } + columnExists, err := columnExistsOnTable(tx, table, column) + if err != nil { + return "", err + } + // If a duplicate sequence exists, then a new sequence will not be created. In this case, + // a pgcode.UndefinedColumn will not occur. + if !columnExists && !sequenceExists { + og.expectedExecErrors.add(pgcode.UndefinedColumn) + } + + seqOptions = append( + seqOptions, + tree.SequenceOption{ + Name: tree.SeqOptOwnedBy, + ColumnItemVal: &tree.ColumnItem{TableName: table.ToUnresolvedObjectName(), ColumnName: tree.Name(column)}}, + ) + } } + createSeq := &tree.CreateSequence{ IfNotExists: ifNotExists, Name: *seqName, From 42ed2915577969e23a04885ff9ca1033dd394f85 Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Tue, 3 Nov 2020 11:44:50 -0500 Subject: [PATCH 06/14] workload/schemachange: screen for errors in renameSequence op Release note: None --- .../schemachange/operation_generator.go | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 7a051c954487..9c9b6c17f241 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -98,9 +98,10 @@ var opsWithExecErrorScreening = map[opType]bool{ dropView: true, dropSchema: true, - renameColumn: true, - renameTable: true, - renameView: true, + renameColumn: true, + renameSequence: true, + renameTable: true, + renameView: true, } func opScreensForExecErrors(op opType) bool { @@ -1022,11 +1023,40 @@ func (og *operationGenerator) renameSequence(tx *pgx.Tx) (string, error) { return "", err } - destSequenceName, err := og.randSequence(tx, og.pctExisting(false), "") + // Decide whether or not to produce a 'cannot change schema of table with RENAME' error + desiredSchema := "" + if !og.produceError() { + desiredSchema = srcSequenceName.Schema() + } + + destSequenceName, err := og.randSequence(tx, og.pctExisting(false), desiredSchema) if err != nil { return "", err } + srcSequenceExists, err := sequenceExists(tx, srcSequenceName) + if err != nil { + return "", err + } + + destSchemaExists, err := schemaExists(tx, destSequenceName.Schema()) + if err != nil { + return "", err + } + + destSequenceExists, err := sequenceExists(tx, destSequenceName) + if err != nil { + return "", err + } + + srcEqualsDest := srcSequenceName.String() == destSequenceName.String() + codesWithConditions{ + {code: pgcode.UndefinedTable, condition: !srcSequenceExists}, + {code: pgcode.UndefinedSchema, condition: !destSchemaExists}, + {code: pgcode.DuplicateRelation, condition: !srcEqualsDest && destSequenceExists}, + {code: pgcode.InvalidName, condition: srcSequenceName.Schema() != destSequenceName.Schema()}, + }.add(og.expectedExecErrors) + return fmt.Sprintf(`ALTER SEQUENCE %s RENAME TO %s`, srcSequenceName, destSequenceName), nil } From b3c4604f978a77f32867586723eed90e75f80b1a Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Tue, 3 Nov 2020 11:55:22 -0500 Subject: [PATCH 07/14] workload/schemachange: screen for errors in dropSequence op Release note: None --- pkg/workload/schemachange/operation_generator.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 9c9b6c17f241..38f3f478e5d5 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -94,6 +94,7 @@ var opsWithExecErrorScreening = map[opType]bool{ dropColumn: true, dropColumnDefault: true, dropColumnNotNull: true, + dropSequence: true, dropTable: true, dropView: true, dropSchema: true, @@ -884,6 +885,14 @@ func (og *operationGenerator) dropSequence(tx *pgx.Tx) (string, error) { Names: tree.TableNames{*sequenceName}, IfExists: ifExists, } + + sequenceExists, err := sequenceExists(tx, sequenceName) + if err != nil { + return "", err + } + if !sequenceExists && !ifExists { + og.expectedExecErrors.add(pgcode.UndefinedTable) + } return tree.Serialize(dropSeq), nil } From 0bce7523bfb1c9566e304fbe1f901b00e323aa5c Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Tue, 3 Nov 2020 16:36:24 -0500 Subject: [PATCH 08/14] workload/schemachange: add error screening to insertRow op Release note: None --- pkg/sql/rowenc/testutils.go | 18 +- pkg/workload/schemachange/error_screening.go | 156 ++++++++++++++++++ .../schemachange/operation_generator.go | 46 +++++- 3 files changed, 212 insertions(+), 8 deletions(-) diff --git a/pkg/sql/rowenc/testutils.go b/pkg/sql/rowenc/testutils.go index c42522f585ca..b02ea0fe0004 100644 --- a/pkg/sql/rowenc/testutils.go +++ b/pkg/sql/rowenc/testutils.go @@ -188,7 +188,13 @@ func RandDatumWithNullChance(rng *rand.Rand, typ *types.T, nullChance int) tree. return &tree.DBitArray{BitArray: r} case types.StringFamily: // Generate a random ASCII string. - p := make([]byte, rng.Intn(10)) + var length int + if typ.Oid() == oid.T_char || typ.Oid() == oid.T_bpchar { + length = 1 + } else { + length = rng.Intn(10) + } + p := make([]byte, length) for i := range p { p[i] = byte(1 + rng.Intn(127)) } @@ -750,6 +756,16 @@ func randInterestingDatum(rng *rand.Rand, typ *types.T) tree.Datum { default: panic(errors.AssertionFailedf("float with an unexpected width %d", typ.Width())) } + case types.BitFamily: + // A width of 64 is used by all special BitFamily datums in randInterestingDatums. + // If the provided bit type, typ, has a width of 0 (representing an arbitrary width) or 64 exactly, + // then the special datum will be valid for the provided type. Otherwise, the special type + // must be resized to match the width of the provided type. + if typ.Width() == 0 || typ.Width() == 64 { + return special + } + return &tree.DBitArray{BitArray: special.(*tree.DBitArray).ToWidth(uint(typ.Width()))} + default: return special } diff --git a/pkg/workload/schemachange/error_screening.go b/pkg/workload/schemachange/error_screening.go index 3b5a70c633a9..1075f64087f4 100644 --- a/pkg/workload/schemachange/error_screening.go +++ b/pkg/workload/schemachange/error_screening.go @@ -165,3 +165,159 @@ func colIsPrimaryKey(tx *pgx.Tx, tableName *tree.TableName, columnName string) ( ); `, tableName.Schema(), tableName.Object(), columnName) } + +// valuesViolateUniqueConstraints determines if any unique constraints (including primary constraints) +// will be violated upon inserting the specified rows into the specified table. +func violatesUniqueConstraints( + tx *pgx.Tx, tableName *tree.TableName, columns []string, rows [][]string, +) (bool, error) { + + if len(rows) == 0 { + return false, fmt.Errorf("violatesUniqueConstraints: no rows provided") + } + + // Fetch unique constraints from the database. The format returned is an array of string arrays. + // Each string array is a group of column names for which a unique constraint exists. + constraints, err := scanStringArrayRows(tx, ` + SELECT DISTINCT array_agg(cols.column_name ORDER BY cols.column_name) + FROM ( + SELECT d.oid, + d.table_name, + d.schema_name, + conname, + contype, + unnest(conkey) AS position + FROM ( + SELECT c.oid AS oid, + c.relname AS table_name, + ns.nspname AS schema_name + FROM pg_catalog.pg_class AS c + JOIN pg_catalog.pg_namespace AS ns ON + ns.oid = c.relnamespace + WHERE ns.nspname = $1 + AND c.relname = $2 + ) AS d + JOIN ( + SELECT conname, conkey, conrelid, contype + FROM pg_catalog.pg_constraint + WHERE contype = 'p' OR contype = 'u' + ) ON conrelid = d.oid + ) AS cons + JOIN ( + SELECT table_name, + table_schema, + column_name, + ordinal_position + FROM information_schema.columns + ) AS cols ON cons.schema_name = cols.table_schema + AND cols.table_name = cons.table_name + AND cols.ordinal_position = cons.position + GROUP BY cons.conname; +`, tableName.Schema(), tableName.Object()) + if err != nil { + return false, err + } + + for _, constraint := range constraints { + // previousRows is used to check unique constraints among the values which + // will be inserted into the database. + previousRows := map[string]bool{} + for _, row := range rows { + violation, err := violatesUniqueConstraintsHelper(tx, tableName, columns, constraint, row, previousRows) + if err != nil { + return false, err + } + if violation { + return true, nil + } + } + } + + return false, nil +} + +func violatesUniqueConstraintsHelper( + tx *pgx.Tx, + tableName *tree.TableName, + columns []string, + constraint []string, + row []string, + previousRows map[string]bool, +) (bool, error) { + + // Put values to be inserted into a column name to value map to simplify lookups. + columnsToValues := map[string]string{} + for i := 0; i < len(columns); i++ { + columnsToValues[columns[i]] = row[i] + } + + query := strings.Builder{} + query.WriteString(fmt.Sprintf(`SELECT EXISTS ( + SELECT * + FROM %s + WHERE + `, tableName.String())) + + atLeastOneNonNullValue := false + for _, column := range constraint { + + // Null values are not checked because unique constraints do not apply to null values. + if columnsToValues[column] != "NULL" { + if atLeastOneNonNullValue { + query.WriteString(fmt.Sprintf(` AND %s = %s`, column, columnsToValues[column])) + } else { + query.WriteString(fmt.Sprintf(`%s = %s`, column, columnsToValues[column])) + } + + atLeastOneNonNullValue = true + } + } + query.WriteString(")") + + // If there are only null values being inserted for each of the constrained columns, + // then checking for uniqueness against other rows is not necessary. + if !atLeastOneNonNullValue { + return false, nil + } + + queryString := query.String() + + // Check for uniqueness against other rows to be inserted. For simplicity, the `SELECT EXISTS` + // query used to check for uniqueness against rows in the database can also + // be used as a unique key to check for uniqueness among rows to be inserted. + if _, duplicateEntry := previousRows[queryString]; duplicateEntry { + return true, nil + } + previousRows[queryString] = true + + // Check for uniqueness against rows in the database. + exists, err := scanBool(tx, queryString) + if err != nil { + return false, err + } + if exists { + return true, nil + } + + return false, nil +} + +func scanStringArrayRows(tx *pgx.Tx, query string, args ...interface{}) ([][]string, error) { + rows, err := tx.Query(query, args...) + if err != nil { + return nil, err + } + defer rows.Close() + + results := [][]string{} + for rows.Next() { + var columnNames []string + err := rows.Scan(&columnNames) + if err != nil { + return nil, err + } + results = append(results, columnNames) + } + + return results, err +} diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index 38f3f478e5d5..dd8d016bc17a 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -103,6 +103,8 @@ var opsWithExecErrorScreening = map[opType]bool{ renameSequence: true, renameTable: true, renameView: true, + + insertRow: true, } func opScreensForExecErrors(op opType) bool { @@ -1205,29 +1207,57 @@ func (og *operationGenerator) insertRow(tx *pgx.Tx) (string, error) { if err != nil { return "", errors.Wrapf(err, "error getting random table name") } + tableExists, err := tableExists(tx, tableName) + if err != nil { + return "", err + } + if !tableExists { + og.expectedExecErrors.add(pgcode.UndefinedTable) + return fmt.Sprintf( + `INSERT INTO %s (IrrelevantColumnName) VALUES ("IrrelevantValue")`, + tableName, + ), nil + } cols, err := og.getTableColumns(tx, tableName.String()) if err != nil { return "", errors.Wrapf(err, "error getting table columns for insert row") } colNames := []string{} - rows := []string{} + rows := [][]string{} for _, col := range cols { - colNames = append(colNames, fmt.Sprintf(`"%s"`, col.name)) + colNames = append(colNames, col.name) } - numRows := og.randIntn(10) + 1 + numRows := og.randIntn(3) + 1 for i := 0; i < numRows; i++ { var row []string for _, col := range cols { d := rowenc.RandDatum(og.params.rng, col.typ, col.nullable) row = append(row, tree.AsStringWithFlags(d, tree.FmtParsable)) } - rows = append(rows, fmt.Sprintf("(%s)", strings.Join(row, ","))) + + rows = append(rows, row) } + + // Verify if the new row will violate unique constraints by checking the constraints and + // existing rows in the database. + uniqueConstraintViolation, err := violatesUniqueConstraints(tx, tableName, colNames, rows) + if err != nil { + return "", err + } + if uniqueConstraintViolation { + og.expectedExecErrors.add(pgcode.UniqueViolation) + } + + formattedRows := []string{} + for _, row := range rows { + formattedRows = append(formattedRows, fmt.Sprintf("(%s)", strings.Join(row, ","))) + } + return fmt.Sprintf( `INSERT INTO %s (%s) VALUES %s`, tableName, strings.Join(colNames, ","), - strings.Join(rows, ","), + strings.Join(formattedRows, ","), ), nil } @@ -1287,8 +1317,10 @@ func (og *operationGenerator) getTableColumns(tx *pgx.Tx, tableName string) ([]c if err != nil { return nil, err } - typNames = append(typNames, typName) - ret = append(ret, c) + if c.name != "rowid" { + typNames = append(typNames, typName) + ret = append(ret, c) + } } if err := rows.Err(); err != nil { return nil, err From 868a2867d925d71a7db5a1b3cd05ffa5dcbad13c Mon Sep 17 00:00:00 2001 From: Jayant Shrivastava Date: Thu, 5 Nov 2020 15:14:30 -0500 Subject: [PATCH 09/14] workload/schemachange: allow transaction retry errors Release note: None --- pkg/workload/schemachange/schemachange.go | 60 ++++++++++++++++++----- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/pkg/workload/schemachange/schemachange.go b/pkg/workload/schemachange/schemachange.go index 513156603bfa..c21497d30e96 100644 --- a/pkg/workload/schemachange/schemachange.go +++ b/pkg/workload/schemachange/schemachange.go @@ -268,20 +268,43 @@ func (w *schemaChangeWorker) runInTxn(tx *pgx.Tx) (string, error) { if _, err = tx.Exec(op); err != nil { if w.opGen.screenForExecErrors { - if w.opGen.expectedExecErrors.empty() { - log.WriteString(fmt.Sprintf("***FAIL; Expected no errors, but got %v\n", err)) + // If the error not an instance of pgx.PgError, then it is unexpected. + pgErr := pgx.PgError{} + if !errors.As(err, &pgErr) { + log.WriteString(fmt.Sprintf("***FAIL; Non pg error %v\n", err)) return log.String(), errors.Mark( errors.Wrap(err, "***UNEXPECTED ERROR"), errRunInTxnFatalSentinel, ) - } else if pgErr := (pgx.PgError{}); !errors.As(err, &pgErr) || errors.As(err, &pgErr) && !w.opGen.expectedExecErrors.empty() && !w.opGen.expectedExecErrors.contains(pgcode.MakeCode(pgErr.Code)) { - log.WriteString(fmt.Sprintf("***FAIL; Expected one of SQLSTATES %s, but got %v\n", w.opGen.expectedExecErrors.string(), err)) + } + + // Transaction retry errors are acceptable. Allow the transaction + // to rollback. + if pgcode.MakeCode(pgErr.Code) == pgcode.SerializationFailure { + log.WriteString(fmt.Sprintf("ROLLBACK; %v\n", err)) + w.recordInHist(timeutil.Since(start), txnRollback) + return log.String(), errors.Mark( + err, + errRunInTxnRbkSentinel, + ) + } + + // Screen for any unexpected errors. + if w.opGen.expectedExecErrors.empty() || !w.opGen.expectedExecErrors.contains(pgcode.MakeCode(pgErr.Code)) { + var logMsg string + if w.opGen.expectedExecErrors.empty() { + logMsg = fmt.Sprintf("***FAIL; Expected no errors, but got %v\n", err) + } else { + logMsg = fmt.Sprintf("***FAIL; Expected one of SQLSTATES %s, but got %v\n", w.opGen.expectedExecErrors.string(), err) + } + log.WriteString(logMsg) return log.String(), errors.Mark( errors.Wrap(err, "***UNEXPECTED ERROR"), errRunInTxnFatalSentinel, ) } + // Rollback because the error was anticipated. log.WriteString(fmt.Sprintf("ROLLBACK; expected SQLSTATE(S) %s, and got %v\n", w.opGen.expectedExecErrors.string(), err)) w.recordInHist(timeutil.Since(start), txnRollback) return log.String(), errors.Mark( @@ -348,14 +371,29 @@ func (w *schemaChangeWorker) run(_ context.Context) error { } } - // If there were no errors commit the txn. - histBin := txnOk - cmtErrMsg := "" if err = tx.Commit(); err != nil { - histBin = txnCommitError - cmtErrMsg = fmt.Sprintf("***FAIL: %v", err) + // If the error not an instance of pgx.PgError, then it is unexpected. + pgErr := pgx.PgError{} + if !errors.As(err, &pgErr) { + w.recordInHist(timeutil.Since(start), txnCommitError) + logs = logs + fmt.Sprintf("***FAIL; Non pg error %v\n", err) + return errors.Mark( + errors.Wrap(err, "***UNEXPECTED ERROR"), + errRunInTxnFatalSentinel, + ) + } + + // Transaction retry errors are acceptable. Allow the transaction + // to rollback. + if pgcode.MakeCode(pgErr.Code) == pgcode.SerializationFailure { + w.recordInHist(timeutil.Since(start), txnCommitError) + logs = logs + fmt.Sprintf("COMMIT; %v\n", err) + return nil + } } - w.recordInHist(timeutil.Since(start), histBin) - logs = logs + fmt.Sprintf("COMMIT; %s\n", cmtErrMsg) + + // If there were no errors while committing the txn. + w.recordInHist(timeutil.Since(start), txnOk) + logs = logs + "COMMIT; \n" return nil } From b9d4389ee143cd45dff0e48c6a3821487d0fd919 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 30 Nov 2020 22:29:24 -0500 Subject: [PATCH 10/14] roachtest: Add missing gossiped StoreDescriptor to expected relocate errors Closes #57191. This seems possible shortly after the cluster starts up, if gossip hasn't propagated by the time the `ALTER TABLE _ EXPERIMENTAL_RELOCATE _` statement is run. --- pkg/kv/test_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kv/test_utils.go b/pkg/kv/test_utils.go index faf3c022b33d..79a63418efb9 100644 --- a/pkg/kv/test_utils.go +++ b/pkg/kv/test_utils.go @@ -38,7 +38,6 @@ func OnlyFollowerReads(rec tracing.Recording) bool { // IsExpectedRelocateError maintains an allowlist of errors related to // atomic-replication-changes we want to ignore / retry on for tests. // See: -// https://github.com/cockroachdb/cockroach/issues/33732 // https://github.com/cockroachdb/cockroach/issues/33708 // https://github.cm/cockroachdb/cockroach/issues/34012 // https://github.com/cockroachdb/cockroach/issues/33683#issuecomment-454889149 @@ -53,6 +52,7 @@ func IsExpectedRelocateError(err error) bool { "snapshot failed:", "breaker open", "unable to select removal target", // https://github.com/cockroachdb/cockroach/issues/49513 + "cannot up-replicate to .*; missing gossiped StoreDescriptor", } pattern := "(" + strings.Join(allowlist, "|") + ")" return testutils.IsError(err, pattern) From 187198ae7f7ef4afa0f374072b14d7e9cdecef1f Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 30 Nov 2020 22:53:31 -0500 Subject: [PATCH 11/14] roachtest: remove initial disk space check from drop test Fixes #56040. Will need to be backported. This assertion seemed like a good idea, but it was repeatedly (dec148a) a source of flakiness when fixtures changed. Now that we're using IMPORT for TPC-C, the check is even harder to get right without making it so small as to be useless. It doesn't seem to be worth the instability, so remove it. --- pkg/cmd/roachtest/drop.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/roachtest/drop.go b/pkg/cmd/roachtest/drop.go index 34d06210d35c..2912b7c4ce97 100644 --- a/pkg/cmd/roachtest/drop.go +++ b/pkg/cmd/roachtest/drop.go @@ -29,7 +29,7 @@ func registerDrop(r *testRegistry) { // by a truncation for the `stock` table (which contains warehouses*100k // rows). Next, it issues a `DROP` for the whole database, and sets the GC TTL // to one second. - runDrop := func(ctx context.Context, t *test, c *cluster, warehouses, nodes int, initDiskSpace int) { + runDrop := func(ctx context.Context, t *test, c *cluster, warehouses, nodes int) { c.Put(ctx, cockroach, "./cockroach", c.Range(1, nodes)) c.Put(ctx, workload, "./workload", c.Range(1, nodes)) c.Start(ctx, t, c.Range(1, nodes), startArgs("-e", "COCKROACH_MEMPROF_INTERVAL=15s")) @@ -86,12 +86,6 @@ func registerDrop(r *testRegistry) { } t.l.Printf("Node %d space used: %s\n", j, humanizeutil.IBytes(int64(size))) - - // Return if the size of the directory is less than expected. - if size < initDiskSpace { - t.Fatalf("Node %d space used: %s less than %s", j, humanizeutil.IBytes(int64(size)), - humanizeutil.IBytes(int64(initDiskSpace))) - } } for i := minWarehouse; i <= maxWarehouse; i++ { @@ -159,7 +153,6 @@ func registerDrop(r *testRegistry) { warehouses := 100 numNodes := 9 - initDiskSpace := 256 << 20 // 256 MB r.Add(testSpec{ Name: fmt.Sprintf("drop/tpcc/w=%d,nodes=%d", warehouses, numNodes), @@ -172,10 +165,9 @@ func registerDrop(r *testRegistry) { if local { numNodes = 4 warehouses = 1 - initDiskSpace = 5 << 20 // 5 MB fmt.Printf("running with w=%d,nodes=%d in local mode\n", warehouses, numNodes) } - runDrop(ctx, t, c, warehouses, numNodes, initDiskSpace) + runDrop(ctx, t, c, warehouses, numNodes) }, }) } From e7896b3277aa1d334f54f377a6a6129bb38f27ad Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Thu, 3 Dec 2020 14:00:01 +0100 Subject: [PATCH 12/14] util/log: de-flake TestSecondaryGC The GC daemon was performing an unconditional round of deletion, asynchronously with the start of the test. This was causing a race condition. Release note: None --- pkg/util/log/file_log_gc.go | 1 - pkg/util/log/file_log_gc_test.go | 6 ++++-- pkg/util/log/flags.go | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/util/log/file_log_gc.go b/pkg/util/log/file_log_gc.go index 30f12b789822..41f51009dced 100644 --- a/pkg/util/log/file_log_gc.go +++ b/pkg/util/log/file_log_gc.go @@ -21,7 +21,6 @@ import ( // gcDaemon runs the GC loop for the given logger. func (l *fileSink) gcDaemon(ctx context.Context) { - l.gcOldFiles() for { select { case <-ctx.Done(): diff --git a/pkg/util/log/file_log_gc_test.go b/pkg/util/log/file_log_gc_test.go index 35e3e5a4816b..9015b61b4377 100644 --- a/pkg/util/log/file_log_gc_test.go +++ b/pkg/util/log/file_log_gc_test.go @@ -88,8 +88,6 @@ func testLogGC( }(logging.mu.disableDaemons) logging.mu.Unlock() - const newLogFiles = 20 - // Make an entry in the target logger. This ensures That there is at // least one file in the target directory for the logger being // tested. This serves two @@ -132,6 +130,8 @@ func testLogGC( // Pick a max total size that's between 2 and 3 log files in size. maxTotalLogFileSize := logFileSize*expectedFilesAfterGC + logFileSize // 2 + t.Logf("new max total log file size: %d", maxTotalLogFileSize) + // We want to create multiple log files below. For this we need to // override the size/number limits first to the values suitable for // the test. @@ -143,6 +143,8 @@ func testLogGC( atomic.StoreInt64(&fileSink.logFilesCombinedMaxSize, maxTotalLogFileSize) // Create the number of expected log files. + const newLogFiles = 20 + for i := 1; i < newLogFiles; i++ { logFn(context.Background(), "%d", i) Flush() diff --git a/pkg/util/log/flags.go b/pkg/util/log/flags.go index 96394a30e51c..e9b13e9d711e 100644 --- a/pkg/util/log/flags.go +++ b/pkg/util/log/flags.go @@ -179,8 +179,10 @@ func ApplyConfig(config logconfig.Config) (cleanupFn func(), err error) { allSinkInfos.put(fileSinkInfo) if fileSink.logFilesCombinedMaxSize > 0 { + // Do a start round of GC, so clear up past accumulated files. + fileSink.gcOldFiles() // Start the GC process. This ensures that old capture files get - // erased as necessary. + // erased as new files get created. go fileSink.gcDaemon(secLoggersCtx) } From 40f01b9e83cf28097a345df3eb564d3521884157 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Thu, 3 Dec 2020 14:20:14 +0100 Subject: [PATCH 13/14] util/log: de-flake certain SQL tests As discussed previously in #57222 - the SQL servers do not obey the server stop logic well and leak goroutines that continue to log while the TestLogScope gets closed. This commit continues to tolerate this misdesign by ensuring that some more stderr-related flags are accessed atomically. Release note: None --- pkg/util/log/flags.go | 4 ++-- pkg/util/log/format_crdb_v1.go | 4 ++-- pkg/util/log/stderr_sink.go | 7 +++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/util/log/flags.go b/pkg/util/log/flags.go index e9b13e9d711e..086f05cddd03 100644 --- a/pkg/util/log/flags.go +++ b/pkg/util/log/flags.go @@ -225,7 +225,7 @@ func ApplyConfig(config logconfig.Config) (cleanupFn func(), err error) { } // Apply the stderr sink configuration. - logging.stderrSink.noColor = config.Sinks.Stderr.NoColor + logging.stderrSink.noColor.Set(config.Sinks.Stderr.NoColor) if err := logging.stderrSinkInfoTemplate.applyConfig(config.Sinks.Stderr.CommonSinkConfig); err != nil { cleanupFn() return nil, err @@ -356,7 +356,7 @@ func DescribeAppliedConfig() string { } // Describe the stderr sink. - config.Sinks.Stderr.NoColor = logging.stderrSink.noColor + config.Sinks.Stderr.NoColor = logging.stderrSink.noColor.Get() config.Sinks.Stderr.CommonSinkConfig = logging.stderrSinkInfoTemplate.describeAppliedConfig() describeConnections := func(l *loggerT, ch Channel, diff --git a/pkg/util/log/format_crdb_v1.go b/pkg/util/log/format_crdb_v1.go index 1d3bb141cc32..efaf8be9b791 100644 --- a/pkg/util/log/format_crdb_v1.go +++ b/pkg/util/log/format_crdb_v1.go @@ -62,7 +62,7 @@ func (formatCrdbV1TTY) formatterName() string { return "crdb-v1-tty" } func (formatCrdbV1TTY) formatEntry(entry logpb.Entry, stacks []byte) *buffer { cp := ttycolor.StderrProfile - if logging.stderrSink.noColor { + if logging.stderrSink.noColor.Get() { cp = nil } return formatLogEntryInternal(entry, false /*showCounter*/, cp, stacks) @@ -77,7 +77,7 @@ func (formatCrdbV1TTYWithCounter) formatterName() string { return "crdb-v1-tty-c func (formatCrdbV1TTYWithCounter) formatEntry(entry logpb.Entry, stacks []byte) *buffer { cp := ttycolor.StderrProfile - if logging.stderrSink.noColor { + if logging.stderrSink.noColor.Get() { cp = nil } return formatLogEntryInternal(entry, true /*showCounter*/, cp, stacks) diff --git a/pkg/util/log/stderr_sink.go b/pkg/util/log/stderr_sink.go index cafc83987808..5d626a5ae5c6 100644 --- a/pkg/util/log/stderr_sink.go +++ b/pkg/util/log/stderr_sink.go @@ -10,13 +10,16 @@ package log -import "github.com/cockroachdb/cockroach/pkg/cli/exit" +import ( + "github.com/cockroachdb/cockroach/pkg/cli/exit" + "github.com/cockroachdb/cockroach/pkg/util/syncutil" +) // Type of a stderr copy sink. type stderrSink struct { // the --no-color flag. When set it disables escapes code on the // stderr copy. - noColor bool + noColor syncutil.AtomicBool } // activeAtSeverity implements the logSink interface. From a352d26151df96d2d1e40550a85354a852eeed2e Mon Sep 17 00:00:00 2001 From: Sam Huang Date: Wed, 25 Nov 2020 15:49:17 -0500 Subject: [PATCH 14/14] Adds reset-quorum CLI command. This command invokes the ResetQuorum RPC to restore quorum to a given range ID. Note that data on any surviving replicas will not be used to restore quorum. Instead, these replicas will be removed irrevocably. Release note: None. --- pkg/cli/BUILD.bazel | 1 + pkg/cli/debug.go | 1 + pkg/cli/debug_reset_quorum.go | 68 +++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 pkg/cli/debug_reset_quorum.go diff --git a/pkg/cli/BUILD.bazel b/pkg/cli/BUILD.bazel index a9326215c212..c42b00dd0075 100644 --- a/pkg/cli/BUILD.bazel +++ b/pkg/cli/BUILD.bazel @@ -12,6 +12,7 @@ go_library( "debug.go", "debug_check_store.go", "debug_merge_logs.go", + "debug_reset_quorum.go", "debug_synctest.go", "decode.go", "demo.go", diff --git a/pkg/cli/debug.go b/pkg/cli/debug.go index 9a2b0de963c9..1fcc3401f5e6 100644 --- a/pkg/cli/debug.go +++ b/pkg/cli/debug.go @@ -1178,6 +1178,7 @@ var debugCmds = append(DebugCmdsForRocksDB, debugEnvCmd, debugZipCmd, debugMergeLogsCommand, + debugResetQuorumCmd, ) // DebugCmd is the root of all debug commands. Exported to allow modification by CCL code. diff --git a/pkg/cli/debug_reset_quorum.go b/pkg/cli/debug_reset_quorum.go new file mode 100644 index 000000000000..fa613042e04d --- /dev/null +++ b/pkg/cli/debug_reset_quorum.go @@ -0,0 +1,68 @@ +// Copyright 2020 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package cli + +import ( + "context" + "fmt" + "strconv" + + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/spf13/cobra" +) + +var debugResetQuorumCmd = &cobra.Command{ + Use: "reset-quorum [range ID]", + Short: "Reset quorum on the given range" + + " by designating the target node as the sole voter.", + Long: ` +Reset quorum on the given range by designating the current node as +the sole voter. Any existing data for the range is discarded. + +This command is UNSAFE and should only be used with the supervision +of Cockroach Labs support. It is a last-resort option to recover a +specified range after multiple node failures and loss of quorum. + +Data on any surviving replicas will not be used to restore quorum. +Instead, these replicas will be removed irrevocably. +`, + Args: cobra.ExactArgs(1), + RunE: MaybeDecorateGRPCError(runDebugResetQuorum), +} + +func runDebugResetQuorum(cmd *cobra.Command, args []string) error { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + rangeID, err := strconv.ParseInt(args[0], 10, 32) + if err != nil { + return err + } + + // Set up GRPC Connection for running ResetQuorum. + cc, _, finish, err := getClientGRPCConn(ctx, serverCfg) + if err != nil { + return err + } + defer finish() + + // Call ResetQuorum to reset quorum for given range on target node. + _, err = roachpb.NewInternalClient(cc).ResetQuorum(ctx, &roachpb.ResetQuorumRequest{ + RangeID: int32(rangeID), + }) + if err != nil { + return err + } + + fmt.Printf("ok; please verify https:///#/reports/range/%d", rangeID) + + return nil +}