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

upgrades: backfill new jobs cols/table #137728

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dt
Copy link
Member

@dt dt commented Dec 18, 2024

Release note: none.
Epic: none.

@dt dt requested a review from msbutler December 18, 2024 23:05
@dt dt requested review from a team as code owners December 18, 2024 23:05
@dt dt requested review from mgartner and removed request for a team December 18, 2024 23:06
@cockroach-teamcity

This comment was marked as off-topic.

@dt dt force-pushed the jobs-backfill branch 5 times, most recently from d5e20a7 to c12dff0 Compare December 19, 2024 16:18
Copy link
Collaborator

@msbutler msbutler left a comment

Choose a reason for hiding this comment

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

Nice test! Only important question is around user_name being an empty string.

pkg/upgrade/upgrades/v25_1_add_jobs_tables.go Show resolved Hide resolved
pkg/upgrade/upgrades/v25_1_add_jobs_tables.go Show resolved Hide resolved
jobutils.WaitForJobToSucceed(t, sqlDB, 1006)
jobutils.WaitForJobToFail(t, sqlDB, 1008)

// Let's look at the jobs table before the backfill.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note to self: when you hook up SHOW JOBS, we should run historical queries at timestamps before/during the migrations.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can do that, but note that the type of gates we are using in all of these changes resolve to the version value as of the txn time, so if you run a historical your query to before V25_1_JobsBackfill was committed, SHOW won't use any of these tables.

Copy link
Collaborator

Choose a reason for hiding this comment

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

right, i think this would be a good test to have in your next pr since we were burned by this during the last jobs migration.

Release note: none.
Epic: none.
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.

3 participants