-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
upgrade: automated catalog repairs incorrectly removes TTL from the table descriptor #110363
Labels
A-cluster-upgrades
A-row-level-ttl
branch-release-23.1
Used to mark GA and release blockers, technical advisories, and bugs for 23.1
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
C-technical-advisory
Caused a technical advisory
release-blocker
Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Comments
ecwall
added
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)
A-row-level-ttl
A-cluster-upgrades
labels
Sep 11, 2023
ecwall
changed the title
update: remove buggy TTL repair
upgrade: remove buggy TTL repair
Sep 11, 2023
rimadeodhar
changed the title
upgrade: remove buggy TTL repair
upgrade: automated catalog repairs incorrectly removes TTL from the table descriptor
Sep 11, 2023
craig bot
pushed a commit
that referenced
this issue
Sep 12, 2023
110150: cli: fix debug pebble commands on encrypted stores r=RaduBerinde a=RaduBerinde Currently the debug pebble commands only work correctly on an encrypted store if the encrypted store's path is `cockroach-data` or the store directory is passed using `--store` (in addition to being passed to the pebble subcommand itself). What's worse, knowledge of this subtle fact was lost among team members. The root cause is that we are trying to resolve encryption options using the server config. The difficulty is that there are a bunch of different commands and there is no unified way to obtain the store directory of interest To fix this, we create `autoDecryptFS`. This is a `vfs.FS` implementation which is able to automatically detect encrypted paths and use the correct unencrypted FS. It does this by having a list of known encrypted stores (the ones in the `--enterprise-encryption` flag), and looking for any of these paths as ancestors of any path in an operation. This new implementation replaces `swappableFS` and `absoluteFS`. We also improve the error message when we try to open an encrypted store without setting up the key correctly. Fixes: #110121 Release note (bug fix): `cockroach debug pebble` commands now work correctly with encrypted stores which don't use the default `cockroach-data` path without having to also pass `--store`. 110173: sql: optimize persistedsqlstats flush size check r=j82w a=j82w Problem: The `persistedsqlstats` size check to make sure the table is not 1.5x the max size is done on every flush which is done on every node every 10 minutes by default. This can cause serialization issues as it is over the entire table. The check is unnecessary most of the time, because it should only fail if the compaction job is failing. Solution: 1. Reduce the check interval to only be done once an hour by default, and make it configurable. 2. The system table is split in to 8 shards. Instead of checking the entire table count limit it to only one shard. This reduces the scope of the check and reduces the chance of serialization issues. This was preivously reverted because of a flakey test because the size check is only done on a single shard. The tests are updated to increase the limit and the number of statements to make sure every shard has data. Fixes: #109619 Release note (sql change): The persistedsqlstats table max size check is now done once an hour instead of every 10 minutes. This reduces the risk of serialization errors on the statistics tables. 110264: c2c: add region constraints replication test r=msbutler a=msbutler This patch adds a test that ensures that a replicating tenant's regional constraints are obeyed in the destination cluster. This test serves as an end to end test of the span config replication work tracked in #106823. This patch also sets the following source system tenant cluster settings in the c2c e2e framework: kv.rangefeed.closed_timestamp_refresh_interval: 200ms, kv.closed_timestamp.side_transport_interval: 50 ms. CDC e2e tests also set these cluster settings. Informs #109059 Release note: None 110334: roachtest: ensure c2c/shutdown tests start destination tenant with online node r=stevendanna a=msbutler An earlier patch #110033 introduced a change that starts the destination tenant from any destination node, but did not consider if that node was shut down. If the driver attempts to connect to the shut down node, the roachtest fails. This patch ensures that the tenant is started on a node that will be online. Fixes #110317 Release note: None 110364: upgrade: remove buggy TTL repair r=rafiss a=ecwall 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. 110431: workflows: stale.yml: update action version r=RaduBerinde a=RaduBerinde The stale bot closes issues as "completed" instead of "not planned". More recent versions have added a configuration setting for this, and it defaults to "not planned". This commit updates the action to the latest version. Epic: none Release note: None 110451: engineccl: skip BenchmarkTimeBoundIterate r=RaduBerinde a=jbowens This benchmark's assertions have recently become flaky. Epic: none Informs: #110299 Release note: none Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: j82w <[email protected]> Co-authored-by: Michael Butler <[email protected]> Co-authored-by: Evan Wall <[email protected]> Co-authored-by: RaduBerinde <[email protected]> Co-authored-by: Jackson Owens <[email protected]>
blathers-crl bot
pushed a commit
that referenced
this issue
Sep 13, 2023
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.
blathers-crl bot
pushed a commit
that referenced
this issue
Sep 13, 2023
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.
rafiss
added
the
release-blocker
Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
label
Oct 26, 2023
rafiss
added
the
branch-release-23.1
Used to mark GA and release blockers, technical advisories, and bugs for 23.1
label
Oct 26, 2023
rytaft
added
C-technical-advisory
Caused a technical advisory
branch-release-22.2
Used to mark GA and release blockers, technical advisories, and bugs for 22.2
and removed
branch-release-22.2
Used to mark GA and release blockers, technical advisories, and bugs for 22.2
labels
Dec 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-cluster-upgrades
A-row-level-ttl
branch-release-23.1
Used to mark GA and release blockers, technical advisories, and bugs for 23.1
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
C-technical-advisory
Caused a technical advisory
release-blocker
Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
During a cluster upgrade TTL is incorrectly removed from table descriptors here because the TTL job schedule ID from the table descriptor is incorrectly being searched for in a set of job IDs here.
Later on when the TTL job runs, it panics (stack trace here https://github.com/cockroachlabs/support/issues/2593#issuecomment-1713899436) and crashes the node.
Remove the TTL "repair" for now: https://cockroachlabs.slack.com/archives/C05RE28U495/p1694456292147979?thread_ts=1694454150.432349&cid=C05RE28U495
See also:
https://github.com/cockroachlabs/support/issues/2593#issuecomment-1714377621
Jira issue: CRDB-31400
The text was updated successfully, but these errors were encountered: