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/schemachanger: drop jobs from 22.1 can fail trying to plan on RowLevelTTL #86672

Closed
fqazi opened this issue Aug 23, 2022 · 1 comment · Fixed by #87196
Closed

sql/schemachanger: drop jobs from 22.1 can fail trying to plan on RowLevelTTL #86672

fqazi opened this issue Aug 23, 2022 · 1 comment · Fixed by #87196
Assignees
Labels
branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. 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

@fqazi
Copy link
Collaborator

fqazi commented Aug 23, 2022

Any job generated with a RowLevelTTL element generated on 22.1 and resumed on 22.2 can fail. With the following error: failed to satisfy [[Table:{DescID: 130}, ABSENT], DROPPED] -dep-SameStagePrecedence-> [[RowLevelTTL:{DescID: 130}, ABSENT], ABSENT] rule "descriptor drop right before dependent element removal"

A simple repro is to pull down a 22.1 corpus: gsutil cp gs://cockroach-corpus/corpus-86575/corpus . , then run ./cockroach debug declarative-corpus-validate corpus

@fqazi fqazi added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-schema-deprecated Use T-sql-foundations instead branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 labels Aug 23, 2022
@Xiang-Gu Xiang-Gu self-assigned this Aug 29, 2022
Xiang-Gu added a commit to Xiang-Gu/cockroach that referenced this issue Aug 31, 2022
Release justification: fixed a release blocker bug.

Previously, in a mixed version state (v22.1 and v22.2), if a old node
drops a rowLevelTTL table and the new shcema changer job was adopted
by a new node, then there is a dep rule that cannot satisfied and hence
causes forward incompatibility. This PR fixes this by suppressing this
dep rule if this particular case happens (namely, it suppress the rule
if `from` is Table element, `to` is a RowLevelTTL element, and there is
no `PUBLIC` status of the table element anywhere in the graph).

Fixes: cockroachdb#86672

Release note: None
Xiang-Gu added a commit to Xiang-Gu/cockroach that referenced this issue Sep 6, 2022
Release justification: fixed a release blocker bug.

Previously, in a mixed version state (v22.1 and v22.2), if a old node
drops a rowLevelTTL table and the new shcema changer job was adopted
by a new node, then there is a dep rule that cannot satisfied and hence
causes forward incompatibility. This PR fixes this by suppressing this
dep rule if this particular case happens (namely, it suppress the rule
if `from` is Table element, `to` is a RowLevelTTL element, and there is
no `PUBLIC` status of the table element anywhere in the graph).

Fixes: cockroachdb#86672

Release note: None
@ajwerner
Copy link
Contributor

ajwerner commented Sep 6, 2022

This is really hard to hit, it requires dropping a table with row-level TTL and then crashing the gateway node and then having it resume on a node with the older version.

Xiang-Gu added a commit to Xiang-Gu/cockroach that referenced this issue Sep 7, 2022
Release justification: fixed a release blocker bug.

Previously, in a mixed version state (v22.1 and v22.2), if a old node
drops a rowLevelTTL table and the new shcema changer job was adopted
by a new node, then there is a dep rule that cannot satisfied and hence
causes forward incompatibility. This PR fixes this by suppressing this
dep rule if this particular case happens (namely, it suppress the rule
if `from` is Table element, `to` is a RowLevelTTL element, and there is
no `PUBLIC` status of the table element anywhere in the graph).

Fixes: cockroachdb#86672

Release note: None
craig bot pushed a commit that referenced this issue Sep 7, 2022
84509: cdc: add builtin function to emulate json encoding r=[miretskiy,rafiss,chengxiong-ruan] a=HonoreDB

There's been a [longstanding debugging request](#75730) to be able to see how a row would get encoded by a changefeed with certain options. The main obstacle was removing the need for the builtins package to depend on any package implementing a builtin, which got easier thanks to the introduction of the builtinsregistry. This PR builds on that work to make everything that references builtin functions load-order independent. The first commit removes the load order effect on function OIDs, and the second changes everything that iterates over all builtins to instead register a hook to be called on each builtin as it gets registered. Finally, we add a crdb_internal function that's registered and implemented outside of the builtins package.

Release note (sql change): Added the crdb_internal.to_json_as_changefeed_with_flags function to help
debug json changefeeds. 

Release justification: Improves stability by ensuring that version changes don't alter OIDs and helping with debugging.

86757: build: disable release justification r=kpatron-cockroachlabs a=kpatron-cockroachlabs

Once the release-22.2 branch is cut we can disable release justification
githook check and let blathers justification check take over.

Notes:
* to be merged after the release 22.2 branch cut
* `blathers/release-justification-check` will pass after `STABILITY_PERIOD`
is turned off [as part of the branch cut playbook]

Release note: None
Release justification: internal-only / non-production
/ release-process code change

87196: rules: suppress a dep-rule in special case r=Xiang-Gu a=Xiang-Gu

Commit 1: very minor comment, function location, code changes;
Commit 2: suppress a dep-rule that causes forward incompatibility
for dropping a table with rowLevelTTL in the mixed version state.

Previously, in a mixed version state (v22.1 and v22.2), if a old node
    drops a rowLevelTTL table and the new shcema changer job was adopted
    by a new node, then there is a dep rule that cannot satisfied and hence
    causes forward incompatibility. This PR fixes this by suppressing this
    dep rule if this particular case happens (namely, it suppress the rule
    if `from` is Table element, `to` is a RowLevelTTL element, and there is
    no `PUBLIC` status of the table element anywhere in the graph).

Fixes: #86672

Release note: None

Release justification: fixed a release blocker bug.


87203: sql: make UDF more robust with db/schema renaming r=chengxiong-ruan a=chengxiong-ruan

There are 5 commits:
1. disallow cross-db references in udf. Also a side effect to serialize user defined type names in udf body as type OIDs.
2. check udf dependencies before renaming a db.
3. check dependencies before renaming a schema.
4. deserialize sequence OID for `SHOW CREATE FUNCTION`.
5. deserialize type OID for `SHOW CREATE FUNCTION`.

Release justification: necessary and low risk bug fixes to make UDF more robust with schema/db renaming.

87287: sql,backupccl: allow owners to also control schedules r=benbardin a=adityamaru

This change relaxes the admin only check that was enforced
when resuming, pasuing, dropping and altering a backup schedule
to also allow non-admin owners of the schedules to perform
these control operations.

Release note (sql change): Owners of a backup schedule can now
control their schedule using the supported pause, resume, drop
and alter queries.

Release justification: high impact change to introduce finer grained
permission control to disaster recovery operations

87298: cli: don't show password in \c metacommand r=knz a=rafiss

fixes #87294

Release note (cli change): The \c metacommand no longer shows the
password in plaintext.

Release justification: low risk change

Co-authored-by: Aaron Zinger <[email protected]>
Co-authored-by: Kyle Patron <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: adityamaru <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
@craig craig bot closed this as completed in 660efc7 Sep 7, 2022
blathers-crl bot pushed a commit that referenced this issue Oct 3, 2022
Release justification: fixed a release blocker bug.

Previously, in a mixed version state (v22.1 and v22.2), if a old node
drops a rowLevelTTL table and the new shcema changer job was adopted
by a new node, then there is a dep rule that cannot satisfied and hence
causes forward incompatibility. This PR fixes this by suppressing this
dep rule if this particular case happens (namely, it suppress the rule
if `from` is Table element, `to` is a RowLevelTTL element, and there is
no `PUBLIC` status of the table element anywhere in the graph).

Fixes: #86672

Release note: None
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. 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)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants