tests: new roachtest for job backward compatibility in new schema changer#86950
Conversation
|
This change is |
d39e78b to
80329c2
Compare
We can gladly reuse some existing functions from `backup.go` for our mixed-version declarative schema changer job backward compatible roachtest. This commit moves them from function variables inside another function to be standalone functions.
80329c2 to
986fb92
Compare
|
I think it's RFAL :) |
fqazi
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 2 of 3 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @fqazi and @Xiang-Gu)
pkg/cmd/roachtest/tests/mixed_version_job_backward_compatibility_in_declarative_schema_changer.go line 29 at r2 (raw file):
db := c.Conn(ctx, t.L(), 1) setUpQuery := ` SET use_declarative_schema_changer = off;
Don't think we need to force the legacy one here?
pkg/cmd/roachtest/tests/mixed_version_job_backward_compatibility_in_declarative_schema_changer.go line 116 at r2 (raw file):
// Run schema change stmts, chosen from those supported in `predV` version. planAndRunSchemaChange(c, upgradedNodes.RandNode(), `DROP SEQUENCE testdb.testsc.s`),
Longer term do we want these tests to be datadriven somehow? Think this is more of a future thing.
Xiang-Gu
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/tests/mixed_version_job_backward_compatibility_in_declarative_schema_changer.go line 29 at r2 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
Don't think we need to force the legacy one here?
We don't necessarily; I think I was just being paranoid when I wrote tests about declarative schema changer -- I want all the setup stmts to run in legacy schema changer and all to-be-tested stmst to run in declarative schema changer.
I don't really think this is reasonable or convincing at all -- I've removed it
pkg/cmd/roachtest/tests/mixed_version_job_backward_compatibility_in_declarative_schema_changer.go line 116 at r2 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
Longer term do we want these tests to be datadriven somehow? Think this is more of a future thing.
yes, I thought about it and don't know the answer -- Indeed, such a roachtest will need to be modified for each (future) release because what's tested in all supported stmts in release v_N-1 (in this case, it's only all the DROPs). I don't like that. What you suggested, a datadriven test, sounds like a way. Let's discuss this during our next 1:1 so I can gain more clarity about what to do.
For this release, let's keep it as is.
…n state Previously, we don't have a test for declarative schema changer job compatibility, both backford and forward, in a mixed version state. This PR added such one. Release note: None
986fb92 to
c15a899
Compare
|
TFTR! bors r+ |
|
Build succeeded: |
Commit 1: Moved a few functions so that we can reuse them in the next commit.
Commit 2: We added a roachtest that tests job backward compatibility in the
declarartive schema changer in a mixed-version cluster. Namely, we test that
DDL stmts supported in v22.1 can be planed in nodes running in v22.2 and the
jobs created can be adopted and finished by nodes running in v22.1.
Fixes: #79840
Release justification: test only changes.
Release note: None