roachtest/cdc: fix cdc/bank and cdc/schemareg #42746
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Partial fix for cockroachdb#42690. I don't think this ever worked. The schema change backfill for the dropped column `b` results in a record for each row that was previously inserted.
|
This change is |
danhhz
reviewed
Nov 25, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @danhhz and @nvanbenschoten)
pkg/cmd/roachtest/cdc.go, line 280 at r2 (raw file):
// NB: the WITH diff option was not supported until v20.1. withDiff := t.IsBuildVersion("v19.2.1")
Is this right? We're not going to backport, so wouldn't this start failing again when we release 19.2.2?
Fixes cockroachdb#42690. Fixes cockroachdb#41177. This was broken by cockroachdb#41793.
nvanbenschoten
force-pushed
the
nvanbenschoten/fixCDC
branch
from
416d073
to
7044348
Compare
nvanbenschoten
commented
Nov 25, 2019
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:
pkg/cmd/roachtest/cdc.go, line 280 at r2 (raw file):
Previously, danhhz (Daniel Harrison) wrote…
Is this right? We're not going to backport, so wouldn't this start failing again when we release 19.2.2?
Nope, it's not right. I was thinking that this was the cluster version (which hasn't been bumped on master), but it's actually the build version (which has). Fixed.
danhhz
approved these changes
Nov 25, 2019
|
TFTR! bors r+ |
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Nov 25, 2019
42650: sql: stop observing the CommitTimestamp in TRUNCATE r=ajwerner a=ajwerner In #40581 we stopped observing the commit timestamp to write it into table descriptors. In this change I overlooked (rather forgot) about this additional place in the code where we observed the commit timestamp. As far as I can tell we don't read this field anywhere ever. Furthermore we know that the the table descriptor in question to which we are referring must be alive and equal to the provided value at the timestamp at which it was read due to serializability. In short, this minor change continues to populate the field with a sensible value and will permit TRUNCATE to be pushed. Fixes #41566. Release note (bug fix): Long running transactions which attempt to TRUNCATE can now be pushed and will commit in cases where they previously could fail or retry forever. 42746: roachtest/cdc: fix cdc/bank and cdc/schemareg r=nvanbenschoten a=nvanbenschoten Fixes #41177. Fixes #42690. These were both broken by #41793 because prior versions of crdb didn't support the `WITH diff` option. Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #41177.
Fixes #42690.
These were both broken by #41793 because prior versions of crdb didn't support the
WITH diffoption.