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

increase test coverage for migration of older versions #107451

Closed
maryliag opened this issue Jul 24, 2023 · 2 comments
Closed

increase test coverage for migration of older versions #107451

maryliag opened this issue Jul 24, 2023 · 2 comments
Assignees
@jayshrivastava
Labels
A-cdc Change Data Capture C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-postmortem Originated from a Postmortem action item. P-3 Issues/test failures with no fix SLA T-cdc

Comments

@maryliag
Copy link
Contributor

maryliag commented Jul 24, 2023 •
edited by exalate-issue-sync bot
Loading

Increase test coverage that would also test migrations from older versions (e.g. 22.1) to a new one (e.g. 23.1)
that would have caught errors such as the one here: #106359

Jira issue: CRDB-30049

Epic CRDB-9179
@maryliag maryliag added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-cdc O-postmortem Originated from a Postmortem action item. labels Jul 24, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jul 24, 2023

cc @cockroachdb/cdc

@blathers-crl blathers-crl bot added the A-cdc Change Data Capture label Jul 24, 2023
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 2, 2023
@jayshrivastava
roachtest/cdc-mixed-versions: add multi-step upgrade test
769e66b 
This change adds a new roachtest "cdc/mixed-versions-multiple-upgrades"
which starts a changefeed and ensures that it keeps running through
multiple cluster upgrades and rollbacks. This test is an extension
of a previous test "cdc/mixed-versions" which only tests upgrading
from one previous version to the present version. "cdc/mixed-versions"
is now renamed to "cdc/mixed-versions-single-upgrade".

"cdc/mixed-versions-multiple-upgrades" performs 4 upgrades starting
from 4 versions in the past. As seen in cockroachdb#107293,
changefeeds running on versions prior to 22.2 can be very brittle,
mainly due to them not retrying transient errors. To address this
problem, this new roachtest will ensure that 22.2 is the minimum
binary version. So if this test is running on 23.1, it will only test
the upgrade path from 22.2-23.1. Once 23.2 releases, it will test the
upgrade path from 22.2 -> 23.1 -> 23.2 and so on, keeping a running
window of size 4.

Closes: cockroachdb#107451
Epic: None
Release note: None
@jayshrivastava jayshrivastava mentioned this issue Aug 2, 2023
Closed
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 2, 2023
@jayshrivastava
roachtest/cdc-mixed-versions: add multi-step upgrade test
65a10e5 
This change adds a new roachtest "cdc/mixed-versions-multiple-upgrades"
which starts a changefeed and ensures that it keeps running through
multiple cluster upgrades and rollbacks. This test is an extension
of a previous test "cdc/mixed-versions" which only tests upgrading
from one previous version to the present version. "cdc/mixed-versions"
is now renamed to "cdc/mixed-versions-single-upgrade".

"cdc/mixed-versions-multiple-upgrades" performs 4 upgrades starting
from 4 versions in the past. As seen in cockroachdb#107293,
changefeeds running on versions prior to 22.2 can be very brittle,
mainly due to them not retrying transient errors. To address this
problem, this new roachtest will ensure that 22.2 is the minimum
binary version. So if this test is running on 23.1, it will only test
the upgrade path from 22.2-23.1. Once 23.2 releases, it will test the
upgrade path from 22.2 -> 23.1 -> 23.2 and so on, keeping a running
window of size 4.

More notes:

Can we remove the restriction of starting at version 22.2?
In [cockroachdb#107293](cockroachdb#107293), we added some code
to restart the changefeed in case it fails to reduce flakes when running 22.1. It does
defeat the purpose of the test to do so, so it would not make sense to do something similar
in this change. It also does not make sense to change 22.1 because it is EOL'd.

Why perform the upgrade through multiple versions?
As seen in cockroachdb#107451, there are cases where,
for example, a 22.1 changefeed survives an upgrade to 22.2 and a 22.2 changefeed survives
and upgrade to 23.1, but a 22.1 does NOT survive an upgrade from 22.1 -> 22.2 -> 23.1.
This non-transitive relationship is the reason that I added this new test. We (and our
users) expect that arbitrarily old jobs can be resumed in new versions without error (unless we
explicitly add code to fail these jobs in certain cases). Because of this expectation,
we should have a test which asserts a changefeed stays running through multiple version
upgrades.

Why limit the test at 4 versions?
We do not want the test to run for an unbounded number of upgrades as we have
many more releases in the future.

Closes: cockroachdb#107451
Epic: None
Release note: None
@jayshrivastava jayshrivastava mentioned this issue Aug 15, 2023
Merged
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 15, 2023
@jayshrivastava
roachtest/cdc/mixed-versions: use mixed version framework
3b0a059 
This change updates the `cdc/mixed-versions` roachtest to use
the mixed version testing framework. This mixed version testing
framework is better than the previous framework because it offers
testing for multiple upgrades. It will also make it easier to
maintain and expand this cdc-specific test.

Informs: cockroachdb#107451
Epic: None
Release note: None
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 16, 2023
@jayshrivastava
roachtest/cdc/mixed-versions: use mixed version framework
b583882 
This change updates the `cdc/mixed-versions` roachtest to use
the mixed version testing framework. This mixed version testing
framework is better than the previous framework because it offers
testing for multiple upgrades. It will also make it easier to
maintain and expand this cdc-specific test.

Informs: cockroachdb#107451
Epic: None
Release note: None
@jayshrivastava jayshrivastava mentioned this issue Aug 17, 2023
Open
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 17, 2023
@jayshrivastava
roachtest/cdc/mixed-versions: use mixed version framework
fc25ef0 
This change updates the `cdc/mixed-versions` roachtest to use
the mixed version testing framework. This mixed version testing
framework is better than the previous framework because it offers
testing for multiple upgrades. It will also make it easier to
maintain and expand this cdc-specific test.

Informs: cockroachdb#107451
Epic: None
Release note: None
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 21, 2023
@jayshrivastava
roachtest/cdc/mixed-versions: use mixed version framework
6517700 
This change updates the `cdc/mixed-versions` roachtest to use
the mixed version testing framework. This mixed version testing
framework is better than the previous framework because it offers
testing for multiple upgrades. It will also make it easier to
maintain and expand this cdc-specific test.

Informs: cockroachdb#107451
Epic: None
Release note: None
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Aug 21, 2023
@jayshrivastava
roachtest/cdc/mixed-versions: use mixed version framework
d944ff8 
This change updates the `cdc/mixed-versions` roachtest to use
the mixed version testing framework. This mixed version testing
framework is better than the previous framework because it offers
testing for multiple upgrades. It will also make it easier to
maintain and expand this cdc-specific test.

Informs: cockroachdb#107451
Epic: None
Release note: None
craig bot pushed a commit that referenced this issue Aug 22, 2023
@jayshrivastava @herkolategan
@arulajmani @knz
Merge #108802 #109160 #109177 #109236 #109239
33d6552 
108802: roachtest/cdc/mixed-versions: use mixed version framework r=renatolabs a=jayshrivastava

This change updates the `cdc/mixed-versions` roachtest to use the mixed version testing framework. This mixed version testing framework is better than the previous framework because it offers testing for multiple upgrades. It will also make it easier to maintain and expand this cdc-specific test.

Informs: #107451
Epic: None
Release note: None

109160: roachtest: simplify address functions r=renatolabs,smg260 a=herkolategan

Combine the duplicated logic for internal and external address functions in cluster into one function. This prevents changing one and forgetting to update the other.

Epic: None
Release note: None

109177: concurrency: misc cleanup related to multiple transactions holding locks on a single key r=nvanbenschoten a=arulajmani

Cleanup following-on from #109049. See individual commits for details.



109236: enginepb,concurrency: clean up {,the usage of} TxnSeqIsIgnored r=nvanbenschoten a=arulajmani

Prior to this patch, we were rolling out bespoke logic to determine whether a sequence number was ignored or not when acquiring/updating locks in the lock table. This patch switches that usage to call into enginepb.TxnSeqIsIgnored instead.

While here, we also change the implementation of TxnSeqIsIgnored to use a binary search and update some commentary.

Epic: none

Release note: None

109239: authserver: clarify a skip r=ericharmeling a=knz

Release note: None
Epic: CRDB-28893

Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
@blathers-crl blathers-crl bot mentioned this issue Aug 22, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Aug 22, 2023
@jayshrivastava
roachtest/cdc/mixed-versions: use mixed version framework
0ef2e3f 
This change updates the `cdc/mixed-versions` roachtest to use
the mixed version testing framework. This mixed version testing
framework is better than the previous framework because it offers
testing for multiple upgrades. It will also make it easier to
maintain and expand this cdc-specific test.

Informs: #107451
Epic: None
Release note: None
@miretskiy
Copy link
Contributor

I think we're good here. Going to close; @jayshrivastava feel free to reopen if you feel strongly.

@exalate-issue-sync exalate-issue-sync bot added the P-3 Issues/test failures with no fix SLA label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jayshrivastava jayshrivastava

Labels
A-cdc Change Data Capture C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-postmortem Originated from a Postmortem action item. P-3 Issues/test failures with no fix SLA T-cdc
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

roachtest/cdc-mixed-versions: add multi-step upgrade test jayshrivastava/cockroach
3 participants
@maryliag @jayshrivastava @miretskiy