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.

Sign up for GitHub

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

Jump to bottom

tests: Revive a skipped roachtest when master is 23.1 #89345

Closed
Xiang-Gu opened this issue Oct 4, 2022 · 6 comments · Fixed by #98403
Closed

tests: Revive a skipped roachtest when master is 23.1 #89345

Xiang-Gu opened this issue Oct 4, 2022 · 6 comments · Fixed by #98403
Assignees
@Xiang-Gu
Labels
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. GA-blocker T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@Xiang-Gu
Copy link
Contributor

Xiang-Gu commented Oct 4, 2022 •
edited
Loading

We decided to skip the roachtest declarative_schema_changer/job-compatibility-mixed-version
because it hangs on master with error

"running execution from '2022-10-04 21:11:14.507359' to '2022-10-04 21:11:15.273009' on 4 failed: non-cancelable: failed to satisfy [[ColumnType:{DescID: 175, ColumnFamilyID: 0, ColumnID: 1}, ABSENT], ABSENT] -dep-SameStagePrecedence-> [[Column:{DescID: 175, ColumnID: 1}, ABSENT], ABSENT] rule "column type removed right before column when not dropping relation"

when testing job-backward compatibility for a DROP VIEW stmt.

One can easily reproduce it with

roachtest run declarative_schema_changer/job-compatibility-mixed-version --local

The reason for this is we recently merged a PR to master that got rid of

  • the isRelationBeingDropped field, which is used in several dep-rules, and
  • the hacky logic that sets it.

This means, in a mixed version state, if we DROP VIEW on an upgraded node and later let an old node adopt the schema changer job, that old node will see this field as being false (even though it would have been true before that PR). Consequently, it activates a dep rule (which was supposed to be suppressed exactly by this field) in the old node, causing a unsatisfiable error, and thus retries indefinitely.

Instead of reverting the abovementioned PR, we will skip this roachtest for now until master is on 23.1, at which time we can close this issue. More specifically, we will need to wait until predecessor_version.json has an entry for 23.1 : 22.2.* AND *t.BuildVersion() is v23.1xxx.

Jira issue: CRDB-20212
@Xiang-Gu Xiang-Gu added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Oct 4, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Oct 4, 2022
@Xiang-Gu Xiang-Gu changed the title tests: Skip a roachtest until master is v23.1 tests: Revive a skipped roachtest when master is 23.1 Oct 4, 2022
@Xiang-Gu Xiang-Gu mentioned this issue Oct 4, 2022
Merged
@ajwerner ajwerner added branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Oct 5, 2022
craig bot pushed a commit that referenced this issue Oct 6, 2022
@Xiang-Gu @chengxiong-ruan
Merge #89348 #89438
06d3121 
89348: tests: Skip a roachtest test until master is on 23.1 r=Xiang-Gu a=Xiang-Gu

Commit 1: Fixed a bug where we forgot to check whether
job adoption is disabled before resuming a job. It also uncovered
a flaw in another test which we fix.

Commit 2: Skip a roachtest that we recently see failure until
master is on 23.1. The reason is detailed in issue #89345.

Fixes: #88921
Backport commit 1 will also fix #89091

Release note: None

89438: sql: benchmark for function expression type checking r=chengxiong-ruan a=chengxiong-ruan

Release note: None

Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
@ajwerner
Copy link
Contributor

I think the time has come.

@Xiang-Gu
Copy link
Contributor Author

Maybe not? We still do not have an entry for 23.1 in predecessor_version.json

@ajwerner
Copy link
Contributor

Ah, yeah, I guess that's fair.

@ajwerner
Copy link
Contributor

ajwerner commented Nov 8, 2022

now finally I think the time has come

@Xiang-Gu Xiang-Gu mentioned this issue Nov 15, 2022
Closed
@Xiang-Gu
Copy link
Contributor Author

It turns out it's still not time (even if we have an entry in predecessor_version.json for 23.1:22.2xxx), because *t.BuildVersion(), which is implemented as git describe --abbrev=0 --tags --match='v[0-9]*', is still v22.2.0-alpha.00000000.

We have to wait until *t.BuildVersion() gives us v23.1.xx.

@Xiang-Gu Xiang-Gu mentioned this issue Dec 5, 2022
Merged
@rimadeodhar
Copy link
Collaborator

@Xiang-Gu: Can we unskip this test now to close out the issue?

@ajwerner ajwerner added GA-blocker and removed release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Mar 8, 2023
@Xiang-Gu Xiang-Gu mentioned this issue Mar 13, 2023
Merged
@craig craig bot closed this as completed in 25c00d4 Mar 14, 2023
@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
Assignees

@Xiang-Gu Xiang-Gu

Labels
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. GA-blocker T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
3 participants
@ajwerner @Xiang-Gu @rimadeodhar