changefeedccl: verify MVCC range tombstones are never visible to CDC #82943
Comments
|
cc @cockroachdb/cdc |
samiskin
added a commit
to samiskin/cockroach
that referenced
this issue
Oct 3, 2022
Resolves cockroachdb#82943 A new test verifies that even if an import was reverted the changefeed still fails on a table offline error. This is now needed to ensure we don't have undesired behavior during a case where rangefeeds would emit range tombstones. Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 3, 2022
…89200 #89211 85346: builtins: recategorize some builtins for docs r=rafiss a=jordanlewis Updates cockroachdb/docs#11842 This commit moves the json generator functions into the JSON documentation category rather than the more generically named "set-returning functions" category. Also, move the "levenshtein" function into the fuzzy text search category. Also, move another couple of internal functions into "system info". Release note: None 88844: *: Use CPut when writing to `system.descriptor` table r=Xiang-Gu a=Xiang-Gu Each individual commit has a rather thorough message, so I encourage reviewers to read them as well. Commit 1: code pattern, non-functional changes Commit 2: Added a `[]byte` field in descriptor builder and immutable struct. We also modified existing functions to carry over this field between descriptor and Mutable/immutable descriptor, and vice versa. Commit 3: Upgrade `WriteDescToBatch` to use `CPut` rather than `Put` to prevent clobbering of the `system.descriptor` table. We get the expected value for the `CPut` when first reading the descriptor into collection from storage, as well as keep booking the descriptor's latest change inside the `uncommitted` descriptor set, in case more we write to the same descriptor more than once in the same transaction. I didn't add any tests in this PR since this changed function `WriteDescToBatch` is heavily invoked for many sql stmts, so I think we should be pretty confident about its correctness if it passes all existing tests (unit, logic, and roachtest). Release note: None 88859: cluster-ui: add helper to determine empty sql results and export sql api wrapper r=xinhaoz a=xinhaoz ### Commit 1 This commit adds a helper function, `sqlResultsAreEmpty` to the sql api that determines whether there are execution results in the request response. Release note: None ### Commit 2 Release note: None 88875: sql/sqlutil, descs: move `TxnWithExecutor()` and methods to `sql.InternalExecutorFactory` r=ajwerner a=ZhouXing19 Having `TxnWithExecutor()` in `descs.CollectionFactory` is unnatural, and will bring dependency loop headaches. This commit is to move the same logic under `sql.InternalExecutorFactory`. Release note: None 88966: build: support running extra checks on ARM r=rail a=healthy-pod This code change changes CI scripts for acceptance, bench, and roachtest to be runnable on ARM machines. Release note: None 89081: builtins: fix doc formatting r=rafiss a=aliher1911 Some builtins had docs incorrectly quoted resulting in wrong formatting applied on generated web page docs. Release note: None 89131: stmtdiagnostics: remove conditional request from registry after completion r=yuzefovich a=yuzefovich Previously, we had a minor bug in how we handle the conditional diagnostics requests when we got a bundle that satisfied the condition - we correctly updated the corresponding system table, but we forgot to remove the request from the local registry. As a result, we would continue collecting conditional bundles until the local node polls the system table and updates its registry (every 10 seconds by default). This commit fixes that issue. Additionally, this commit updates the tests to enforce that the in-memory registry doesn't contain completed requests. Release note: None 89169: changefeedccl: verify changefeed failure for reverted import r=samiskin a=samiskin Resolves #82943 A new test verifies that even if an import was reverted the changefeed still fails on a table offline error. This is now needed to ensure we don't have undesired behavior during a case where rangefeeds would emit range tombstones. Release note: None 89195: storage: correctly check that a value is a tombstone r=erikgrinaker a=sumeerbhola We can't rely on the byte slice being of length 0. This was not a correctness bug, but limits a wider MVCC range tombstone. Release note: None 89200: roachtest: disable decimal columns in costfuzz and unoptimized tests r=rytaft a=rytaft The use of decimal columns was making costfuzz and unoptimized-query-oracle tests flaky. This commit disables generation of decimal columns as a temporary mitigation for these flakes. Fixes #88547 Release note: None 89211: roachtest: skip cdc/bank on ARM64 r=miretskiy a=healthy-pod This code change skips `cdc/bank` on ARM64 because Confluent CLI is not available on `linux/arm64`. Release note: None Co-authored-by: Jordan Lewis <[email protected]> Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Xin Hao Zhang <[email protected]> Co-authored-by: Jane Xing <[email protected]> Co-authored-by: healthy-pod <[email protected]> Co-authored-by: Oleg Afanasyev <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Shiranka Miskin <[email protected]> Co-authored-by: sumeerbhola <[email protected]> Co-authored-by: Rebecca Taft <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We will shortly emit MVCC range deletion tombstones (#70412) across rangefeeds (#82449). We don't expect these to be emitted across live spans with changefeeds across them, so erroring on these events is fine. However, we must verify that an
IMPORT INTOcorrectly takes the span offline, and that once the import has been cancelled and rolled back, the catchup scan won't pick up the range tombstones. We should write an integration test to verify that this cannot happen.Jira issue: CRDB-16745
Epic CRDB-2624
The text was updated successfully, but these errors were encountered: