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/schemachanger: setColumnType operations in schemachange/random-load will occasionally fail #66662

Closed
ajstorm opened this issue Jun 21, 2021 · 0 comments · Fixed by #133131
Assignees
Labels
A-schema-changes A-testing Testing tools and infrastructure C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ajstorm
Copy link
Collaborator

ajstorm commented Jun 21, 2021

When running the random schema changer workload (schemachange/random-load) the setColumnType operation will occasionally fail complaining that the operation is "not supported inside a transaction". This is strange because the workload specifically checks to see that the schema change operation is non-trivial, and if so, adds that as an acceptable error. It would appear as though some schema change operations are being flagged as non-trivial in the schema change code, but not in the workload. Note that this is possible with schema changes which are provided an expression (due to the following code snippet), but that doesn't seem to be happening in the attached example.

	var kind schemachange.ColumnConversionKind
	if t.Using != nil {
		// If an expression is provided, we always need to try a general conversion.
		// We have to follow the process to create a new column and backfill it
		// using the expression.
		kind = schemachange.ColumnConversionGeneral
	} else {
		kind, err = schemachange.ClassifyConversion(ctx, col.GetType(), typ)
		if err != nil {
			return err
		}
	}

For the time being, I've disabled setColumnType operations in the schemachange/random-load and will use this as a tracking issue.
run_132543.970_n1_workload_run_schemachange.log

Jira issue: CRDB-8168

Epic CRDB-25314

@ajstorm ajstorm added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-schema-changes T-sql-schema-deprecated Use T-sql-foundations instead labels Jun 21, 2021
ajstorm added a commit to ajstorm/cockroach that referenced this issue Jun 21, 2021
This issue addresses several issues uncovered in running the randomized
schema changer. Specifically:

- Makes several errors pgcodes, so that they can be properly added to
  the expected errors list in the randomized schema changer.
- Detects cases where the region column (crdb_region) is used multiple
  times in an index definition.
- Allows for column type changes, which must have the experimental flag
  enable_experimental_alter_column_type_general flag set.

It also disables the testing of setColumnType (tracked with cockroachdb#66662) as
well as making a column nullable/non-nullable due to a timing hole
(tracked with cockroachdb#66663).

Release note: None.
ajstorm added a commit to ajstorm/cockroach that referenced this issue Jun 22, 2021
This issue addresses several issues uncovered in running the randomized
schema changer. Specifically:

- Makes several errors pgcodes, so that they can be properly added to
  the expected errors list in the randomized schema changer.
- Detects cases where the region column (crdb_region) is used multiple
  times in an index definition.
- Allows for column type changes, which must have the experimental flag
  enable_experimental_alter_column_type_general flag set.

It also disables the testing of setColumnType (tracked with cockroachdb#66662) as
well as making a column nullable/non-nullable due to a timing hole
(tracked with cockroachdb#66663).

Release note: None.
ajstorm added a commit to ajstorm/cockroach that referenced this issue Jun 22, 2021
This issue addresses several issues uncovered in running the randomized
schema changer. Specifically:

- Makes several errors pgcodes, so that they can be properly added to
  the expected errors list in the randomized schema changer.
- Detects cases where the region column (crdb_region) is used multiple
  times in an index definition.
- Allows for column type changes, which must have the experimental flag
  enable_experimental_alter_column_type_general flag set.

It also disables the testing of setColumnType (tracked with cockroachdb#66662) as
well as making a column nullable/non-nullable due to a timing hole
(tracked with cockroachdb#66663).

Release note: None.
craig bot pushed a commit that referenced this issue Jun 22, 2021
66599: sql: enable adding columns which are not written to the current primary index r=postamar a=postamar

    row,schemachanger: use StoreColumnIDs/Names in primary index
    
    In a recent commit, the StoreColumnIDs and StoreColumnNames slices in
    primary indexes were populated when previously they had simply been
    empty. We simply assumed that all non-virtual columns in a table would
    be stored in the primary index: primary key columns in the key, the rest
    in the value.
    
    This commit breaks that assumption by using the StoreColumnIDs slice to
    determine what goes into the primary index. This makes it possible for
    the new schema changer to add columns safely, preventing unwanted writes
    to the old primary index while the schema change is underway.
    
    Fixes #59149.
    
    Release note: None


    sql,tabledesc: add new IndexDescriptorVersion for primary indexes
    
    Previously, the IndexDescriptorVersion type was only used to describe
    the encoding of secondary indexes. This commit adds a new value for
    use in primary indexes, PrimaryIndexWithStoredColumnsVersion, to signify
    that the StoredColumnIDs and StoredColumnNames slices are populated
    correctly.
    
    Previously, these slices did not need to be populated at all. This is
    because the set of columns comprising the primary index of a table is
    assumed to be all non-virtual columns of that table. Our upcoming work on
    the new schema changer will require us to violate that assumption
    however. This commit is in preparation of that change.
    
    In our effort to make meaningful the concept of stored columns in
    primary indexes, this commit also changes the contents of the
    information_schema.statistics table. As a result, SHOW INDEXES and SHOW
    COLUMNS behave the same way regardless of whether an index is primary or
    secondary.
    
    Release note (sql change): The contents of the statistics table in the
    information schema have changed, therefore so have the results of SHOW
    INDEX and SHOW COLUMNS. A column which is not in the primary key will
    now be listed as belonging to the primary index as a stored column.
    Previously, it was simply not listed as belonging to the primary index.

66664: sql: First round of cleanup of schemachange/random-load r=ajwerner,otan a=ajstorm

This issue addresses several issues uncovered in running the randomized
schema changer. Specifically:

- Makes several errors pgcodes, so that they can be properly added to
  the expected errors list in the randomized schema changer.
- Detects cases where the region column (crdb_region) is used multiple
  times in an index definition.
- Allows for column type changes, which must have the experimental flag
  enable_experimental_alter_column_type_general flag set.

It also disables the testing of setColumnType (tracked with #66662) as
well as making a column nullable/non-nullable due to a timing hole
(tracked with #66663).

Release note: None.

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: Adam Storm <[email protected]>
ajstorm added a commit to ajstorm/cockroach that referenced this issue Jul 14, 2021
This issue addresses several issues uncovered in running the randomized
schema changer. Specifically:

- Makes several errors pgcodes, so that they can be properly added to
  the expected errors list in the randomized schema changer.
- Detects cases where the region column (crdb_region) is used multiple
  times in an index definition.
- Allows for column type changes, which must have the experimental flag
  enable_experimental_alter_column_type_general flag set.

It also disables the testing of setColumnType (tracked with cockroachdb#66662) as
well as making a column nullable/non-nullable due to a timing hole
(tracked with cockroachdb#66663).

Release note: None.
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
@rafiss rafiss added the A-testing Testing tools and infrastructure label Jul 18, 2023
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue Oct 21, 2024
…m-load will occasionally fail

Previously this test would fail due to 0A000 and 2BP01 even though those failures are expected.
In this code changes we properly handle these errors.

Fixes: cockroachdb#66662
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue Oct 21, 2024
…m-load will occasionally fail

Previously this test would fail due to 0A000 and 2BP01 even though those failures are expected.
In this code changes we properly handle these errors.

Fixes: cockroachdb#66662
Release note: None
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue Oct 21, 2024
…m-load will occasionally fail

Previously this test would fail due to 0A000 and 2BP01 even though those failures are expected.
In this code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: cockroachdb#66662
Release note: None
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue Oct 21, 2024
Previously this test would fail due to 0A000 and 2BP01
even though those failures are expected.
In this code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: cockroachdb#66662
Release note: None
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue Oct 21, 2024
Previously this test would fail due to 0A000 and 2BP01
even though those failures are expected.
In these code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: cockroachdb#66662
Release note: None
Dedej-Bergin added a commit to Dedej-Bergin/cockroach that referenced this issue Oct 22, 2024
Previously this test would fail due to 0A000 and 2BP01
even though those failures are expected.
In these code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: cockroachdb#66662
Release note: None
craig bot pushed a commit that referenced this issue Nov 1, 2024
133131: workload/schemachange: setColumnType operations will occasionally fail r=Dedej-Bergin a=Dedej-Bergin

Previously this test would fail due to 0A000 and 2BP01 even though those failures are expected.
In these code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: #66662
Release note: None

133670: raft: enable store liveness in raft unit tests phase 1 r=iskettaneh a=iskettaneh

This commit modifies the following tests to allow them to run in both store liveness enabled/disabled configurations:

1) TestCommitWithoutNewTermEntry
2) TestConfChangeCheckBeforeCampaign
3) TestConfChangeV2CheckBeforeCampaign
4) TestLeaderCycle
5) TestLeaderCyclePreVote
6) TestLeaderTransferReceiveHigherTermVote
7) TestLogReplication
8) TestOldMessages

References: #132241

Release note: None

Co-authored-by: Bergin Dedej <[email protected]>
Co-authored-by: Ibrahim Kettaneh <[email protected]>
craig bot pushed a commit that referenced this issue Nov 1, 2024
133131: workload/schemachange: setColumnType operations will occasionally fail r=Dedej-Bergin a=Dedej-Bergin

Previously this test would fail due to 0A000 and 2BP01 even though those failures are expected.
In these code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: #66662
Release note: None

133845: sql/schemachanger: Preserving column family order for complex column type changes r=spilchen a=spilchen

When altering a column’s type in a way that requires a backfill, we drop the old column and add a new one. Previously, the new column was always added to the end of the column family. This change ensures the column family order is preserved.

In the DSC, the column family is updated when a new ColumnType element is added. I introduced a new field to this element to control the order, which requires specifying the column ID that the new column should follow.

Epic: CRDB-25314
Closes #133040
Release note: none

Co-authored-by: Bergin Dedej <[email protected]>
Co-authored-by: Matt Spilchen <[email protected]>
@craig craig bot closed this as completed in 3ec0074 Nov 1, 2024
annrpom pushed a commit to annrpom/cockroach that referenced this issue Nov 7, 2024
Previously this test would fail due to 0A000 and 2BP01
even though those failures are expected.
In these code changes we properly handle these errors.

Epic: CRDB-25314

Fixes: cockroachdb#66662
Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-changes A-testing Testing tools and infrastructure C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants