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, jobs: schema change jobs are erroneously queued for in-txn schema changes #45985

Closed
thoszhang opened this issue Mar 11, 2020 · 0 comments · Fixed by #58888
Closed

sql, jobs: schema change jobs are erroneously queued for in-txn schema changes #45985

thoszhang opened this issue Mar 11, 2020 · 0 comments · Fixed by #58888
Assignees
Labels
A-schema-changes C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@thoszhang
Copy link
Contributor

In cockroach demo on 2863173:

[email protected]:56758/movr> begin;
Now adding input for a multi-line SQL transaction client-side (smart_prompt enabled).
Press Enter two times to send the SQL text collected so far to the server, or Ctrl+C to cancel.
You can also use \show to display the statements entered so far.
                        -> create table t (a int);
                        -> alter table t add column b int;
                        -> commit;
COMMIT

Time: 27.151ms

[email protected]:56758/movr> show jobs;
        job_id       |   job_type    |                                                        description                                                         | statement | user_name |  status   | running_status |             created              |             started              |             finished             |             modified             | fraction_completed | error | coordinator_id
---------------------+---------------+----------------------------------------------------------------------------------------------------------------------------+-----------+-----------+-----------+----------------+----------------------------------+----------------------------------+----------------------------------+----------------------------------+--------------------+-------+-----------------
  536958125229670401 | SCHEMA CHANGE | ALTER TABLE movr.public.t ADD COLUMN b INT8                                                                                |           | root      | succeeded | NULL           | 2020-03-11 14:30:15.362633+00:00 | 2020-03-11 14:30:15.377983+00:00 | 2020-03-11 14:30:15.384583+00:00 | 2020-03-11 14:30:15.383424+00:00 |                  1 |       |              1
<...other rows...>

We shouldn't be queuing a job for new tables where schema changes are run entirely in the transaction in which the table was created. This is happening because we're calling runSchemaChangesInTxn() in planner.writeTableDescToBatch() if the table is new, but we always indiscriminately queue a schema change job before that. The job doesn't do anything because the schema change was finished, but we should still fix this and write a regression test for it.

@thoszhang thoszhang self-assigned this Mar 11, 2020
@thoszhang thoszhang added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label May 4, 2020
@postamar postamar self-assigned this Dec 3, 2020
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 12, 2021
Creating a table and changing its schema within a transaction would
cause a schema change job to be queued. Such jobs are not necessary and
don't do anything. This patch prevents them from being queued in the
first place.

Fixes cockroachdb#45985.

Release note (sql change): Creating a table and changing its schema
within a transaction no longer schedules a schema change job.
craig bot pushed a commit that referenced this issue Jan 14, 2021
58888: sql, jobs: stop queuing schema change jobs for in-txn schema changes r=postamar a=postamar

Creating a table and changing its schema within a transaction would
cause a schema change job to be queued. Such jobs are not necessary and
don't do anything. This patch prevents them from being queued in the
first place.

Fixes #45985.

Release note (sql change): Creating a table and changing its schema
within a transaction no longer schedules a schema change job.

58919: kvnemesis: fix and unskip TestKVNemesisMultiNode r=aayushshah15 a=aayushshah15

Before this patch, #56197 broke this test because it changed a range
merge error string that let the KV nemesis validator ignore the error.
This patch updates the regexp the validator uses to match the error and
unskips the test.

Fixes #58624.

Release note: None

58982: delegate: remove is_active_region from SHOW REGIONS FROM DATABASE r=ajstorm a=otan

Due to unpopular demand.

Release note: None

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: Aayush Shah <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
@craig craig bot closed this as completed in 2360c4b Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-changes C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants