Skip to content

Commit

Permalink
Merge cockroachdb#42650 cockroachdb#42746
Browse files Browse the repository at this point in the history 
42650: sql: stop observing the CommitTimestamp in TRUNCATE r=ajwerner a=ajwerner

In cockroachdb#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 cockroachdb#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 cockroachdb#41177.
Fixes cockroachdb#42690.

These were both broken by cockroachdb#41793 because prior versions of crdb didn't support the `WITH diff` option.

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
  • Loading branch information
@ajwerner @nvanbenschoten
3 people committed Nov 25, 2019
3 parents 579db62 + 3003a79 + 7044348 commit 5f3e659
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 56 deletions.
68 changes: 50 additions & 18 deletions pkg/cmd/roachtest/cdc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,16 @@ func runCDCBank(ctx context.Context, t *test, c *cluster) {
); err != nil {
t.Fatal(err)
}

// NB: the WITH diff option was not supported until v20.1.
withDiff := t.IsBuildVersion("v20.1.0")
var opts = []string{`updated`, `resolved`}
if withDiff {
opts = append(opts, `diff`)
}
var jobID string
if err := db.QueryRow(
`CREATE CHANGEFEED FOR bank.bank INTO $1 WITH updated, resolved, diff`, kafka.sinkURL(ctx),
`CREATE CHANGEFEED FOR bank.bank INTO $1 WITH `+strings.Join(opts, `, `), kafka.sinkURL(ctx),
).Scan(&jobID); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -314,19 +321,22 @@ func runCDCBank(ctx context.Context, t *test, c *cluster) {
}

const requestedResolved = 100
baV, err := cdctest.NewBeforeAfterValidator(db, `bank.bank`)
if err != nil {
return err
}
fprintV, err := cdctest.NewFingerprintValidator(db, `bank.bank`, `fprint`, tc.partitions, 0)
if err != nil {
return err
}
v := cdctest.MakeCountValidator(cdctest.Validators{
validators := cdctest.Validators{
cdctest.NewOrderValidator(`bank`),
baV,
fprintV,
})
}
if withDiff {
baV, err := cdctest.NewBeforeAfterValidator(db, `bank.bank`)
if err != nil {
return err
}
validators = append(validators, baV)
}
v := cdctest.MakeCountValidator(validators)

for {
m := tc.Next(ctx)
Expand Down Expand Up @@ -387,10 +397,16 @@ func runCDCSchemaRegistry(ctx context.Context, t *test, c *cluster) {
if _, err := db.Exec(`CREATE TABLE foo (a INT PRIMARY KEY)`); err != nil {
t.Fatal(err)
}

// NB: the WITH diff option was not supported until v20.1.
withDiff := t.IsBuildVersion("v20.1.0")
var opts = []string{`updated`, `resolved`, `format=experimental_avro`, `confluent_schema_registry=$2`}
if withDiff {
opts = append(opts, `diff`)
}
var jobID string
if err := db.QueryRow(
`CREATE CHANGEFEED FOR foo INTO $1`+
`WITH updated, resolved, diff, format=experimental_avro, confluent_schema_registry=$2`,
`CREATE CHANGEFEED FOR foo INTO $1 WITH `+strings.Join(opts, `, `),
kafka.sinkURL(ctx), kafka.schemaRegistryURL(ctx),
).Scan(&jobID); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -448,14 +464,30 @@ func runCDCSchemaRegistry(ctx context.Context, t *test, c *cluster) {
}
sort.Strings(updated)

expected := []string{
`{"before":null,"after":{"foo":{"a":{"long":1}}},"updated":{"string":""}}`,
`{"before":null,"after":{"foo":{"a":{"long":2},"b":{"string":"2"}}},"updated":{"string":""}}`,
`{"before":null,"after":{"foo":{"a":{"long":3},"b":{"string":"3"},"c":{"long":3}}},"updated":{"string":""}}`,
`{"before":null,"after":{"foo":{"a":{"long":4},"c":{"long":4}}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":1},"c":null}},"after":{"foo":{"a":{"long":1},"c":null}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":2},"c":null}},"after":{"foo":{"a":{"long":2},"c":null}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":3},"c":{"long":3}}},"after":{"foo":{"a":{"long":3},"c":{"long":3}}},"updated":{"string":""}}`,
var expected []string
if withDiff {
expected = []string{
`{"before":null,"after":{"foo":{"a":{"long":1}}},"updated":{"string":""}}`,
`{"before":null,"after":{"foo":{"a":{"long":2},"b":{"string":"2"}}},"updated":{"string":""}}`,
`{"before":null,"after":{"foo":{"a":{"long":3},"b":{"string":"3"},"c":{"long":3}}},"updated":{"string":""}}`,
`{"before":null,"after":{"foo":{"a":{"long":4},"c":{"long":4}}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":1},"b":null,"c":null}},"after":{"foo":{"a":{"long":1},"c":null}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":1},"c":null}},"after":{"foo":{"a":{"long":1},"c":null}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":2},"b":{"string":"2"},"c":null}},"after":{"foo":{"a":{"long":2},"c":null}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":2},"c":null}},"after":{"foo":{"a":{"long":2},"c":null}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":3},"b":{"string":"3"},"c":{"long":3}}},"after":{"foo":{"a":{"long":3},"c":{"long":3}}},"updated":{"string":""}}`,
`{"before":{"foo_before":{"a":{"long":3},"c":{"long":3}}},"after":{"foo":{"a":{"long":3},"c":{"long":3}}},"updated":{"string":""}}`,
}
} else {
expected = []string{
`{"updated":{"string":""},"after":{"foo":{"a":{"long":1},"c":null}}}`,
`{"updated":{"string":""},"after":{"foo":{"a":{"long":1}}}}`,
`{"updated":{"string":""},"after":{"foo":{"a":{"long":2},"b":{"string":"2"}}}}`,
`{"updated":{"string":""},"after":{"foo":{"a":{"long":2},"c":null}}}`,
`{"updated":{"string":""},"after":{"foo":{"a":{"long":3},"b":{"string":"3"},"c":{"long":3}}}}`,
`{"updated":{"string":""},"after":{"foo":{"a":{"long":3},"c":{"long":3}}}}`,
`{"updated":{"string":""},"after":{"foo":{"a":{"long":4},"c":{"long":4}}}}`,
}
}
if strings.Join(expected, "\n") != strings.Join(updated, "\n") {
t.Fatalf("expected\n%s\n\ngot\n%s\n\n",
Expand Down
Loading

0 comments on commit 5f3e659

Please sign in to comment.