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
release-23.1: upgrade: remove buggy TTL repair #110500
Conversation
Thanks for opening a backport. Please check the backport criteria before merging:
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
Add a brief release justification to the body of your PR to justify this backport. Some other things to consider:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's add an additional change to this backport. it should use an environment variable to make this function return early by default:
func FirstUpgradeFromReleasePrecondition( |
upgrade all | ||
|
||
query B retry | ||
SELECT version LIKE '%23.1-%' FROM [SHOW CLUSTER SETTING version] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the backport, this should be LIKE '%22.2-%'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)
pkg/sql/logictest/testdata/logic_test/mixed_version_upgrade_repair_descriptors
line 11 at r2 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
in the backport, this should be
LIKE '%22.2-%'
Done
`upgrade all` upgrades all nodes in the cluster. It has the same behavior as using `upgrade <idx>` for each index in the cluster. Release note: None
Fixes #110363 The TTL descriptor repair in FirstUpgradeFromReleasePrecondition incorrectly removes TTL fields from table descriptors after incorrectly comparing the table descriptor's TTL job schedule ID to a set of job IDs. This change removes the repair until tests are properly added. Release note (bug fix): Remove buggy TTL descriptor repair. Previously, upgrading from 22.2.X to 23.1.9 incorrectly removed TTL storage params from tables (visible via `SHOW CREATE TABLE <ttl-table>;`) while attempting to repair table descriptors. This resulted in the node that attempts to run the TTL job crashing due to a panic caused by the missing TTL storage params. Clusters currently on 22.2.X should NOT be upgraded to 23.1.9 and should be upgraded to 23.1.10 or later directly.
This short-circuits FirstUpgradeFromReleasePrecondition and is false by default. It has been added to 23.1 releases to prevent automatic descriptor repair on start up until more test coverage is added. Release note: None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, lgtm!
blathers backport 23.1.11-rc |
blathers backport staging-v23.1.10 |
Backport 2/2 commits from #110364 and #110816.
/cc @cockroachdb/release
Fixes #110363
The TTL descriptor repair in FirstUpgradeFromReleasePrecondition incorrectly
removes TTL fields from table descriptors after incorrectly comparing the
table descriptor's TTL job schedule ID to a set of job IDs.
This change removes the repair until tests are properly added.
Release note (bug fix): Remove buggy TTL descriptor repair. Previously,
upgrading from 22.2.X to 23.1.9 incorrectly removed TTL storage params from
tables (visible via
SHOW CREATE TABLE <ttl-table>;
) while attempting torepair table descriptors. This resulted in the node that attempts to run the
TTL job crashing due to a panic caused by the missing TTL storage params.
Clusters currently on 22.2.X should NOT be upgraded to 23.1.9 and should
be upgraded to 23.1.10 or later directly.
Release justification: Backport to base extraordinary release 23.1.10 on.