Skip to content

server: start jobs registry before running startup migrations#45828

Merged
dt merged 2 commits intocockroachdb:masterfrom
dt:jobs-before-migrations
Mar 7, 2020
Merged

server: start jobs registry before running startup migrations#45828
dt merged 2 commits intocockroachdb:masterfrom
dt:jobs-before-migrations

Conversation

@dt
Copy link
Copy Markdown
Contributor

@dt dt commented Mar 7, 2020

This contains two changes: one bakes in the startup migration that adds the progress column and family to the jobs table, and the other moves the execution of startup migrations to after the jobs registry has been started.

The later is important since migrations themselves often run commands such as REVOKE or CREATE TABLE that themselves can spawn jobs.

Release note: none.

This was supposed to get baked in two releases ago. Whoops.

Release note: none.
@dt dt requested review from spaskob and thoszhang March 7, 2020 02:23
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@dt dt requested a review from jordanlewis March 7, 2020 02:25
Migrations sometimes try to run jobs, for example running schema changes
or other operations that are executed as jobs. Thus the jobs system must
be started before the migrations are run.

Release note: none.
@dt dt force-pushed the jobs-before-migrations branch from c93185c to 2cefc00 Compare March 7, 2020 04:02
Copy link
Copy Markdown
Contributor

@spaskob spaskob left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this!
Please stress test jobs_test.go locally - it has helped me uncover a lot of subtle issues with jobs.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @lucy-zhang, and @spaskob)

@dt
Copy link
Copy Markdown
Contributor Author

dt commented Mar 7, 2020

403 runs so far, 0 failures, over 7m30s

@dt
Copy link
Copy Markdown
Contributor Author

dt commented Mar 7, 2020

@lucy-zhang I tested this locally on top of 497f15e and it looked like the startup deadlock is gone

@dt dt merged commit 50b31b6 into cockroachdb:master Mar 7, 2020
@dt dt deleted the jobs-before-migrations branch March 7, 2020 22:51
@thoszhang
Copy link
Copy Markdown

Thanks for dealing with this.

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.

4 participants