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.

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

kvserver: C2C handling of DeleteRange aka MVCC range tombstones #70433

Closed
erikgrinaker opened this issue Sep 20, 2021 · 3 comments · Fixed by #84553
Closed

kvserver: C2C handling of DeleteRange aka MVCC range tombstones #70433

erikgrinaker opened this issue Sep 20, 2021 · 3 comments · Fixed by #84553
Assignees
Labels
A-cdc Change Data Capture A-tenant-streaming Including cluster streaming C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) sync-me-8 T-cdc
Milestone

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Sep 20, 2021

Once MVCC range deletion tombstones (#70412) are emitted across rangefeeds (#82449), they need to be handled by cluster-to-cluster (C2C) streaming. Initially, these will only be used for schema GC (e.g. dropping tables) and rolling back a cancelled import. When that happens, the DeleteRange operation must be replayed on the target cluster using an MVCC range tombstone.

NB: MVCC range tombstones will also be included in AddSSTable events emitted across rangefeeds, and these must be handled via specialized range key functionality on the SST iterator, e.g. HasPointAndRange() and RangeKeys(). See SimpleMVCCIterator documentation for details.

#CRDB-7188
Jira issue: CRDB-10074
Epic CRDB-7188

@erikgrinaker erikgrinaker added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-transactions Relating to MVCC and the transactional model. A-cdc Change Data Capture T-kv KV Team labels Sep 20, 2021
@blathers-crl blathers-crl bot added the T-cdc label Sep 20, 2021
@erikgrinaker erikgrinaker changed the title kvserver: RangeFeed and CDC support for DeleteRange kvserver: rangefeed and CDC support for DeleteRange Sep 20, 2021
@lunevalex lunevalex added A-kv-replication Relating to Raft, consensus, and coordination. and removed A-kv-transactions Relating to MVCC and the transactional model. labels Oct 1, 2021
@exalate-issue-sync exalate-issue-sync bot removed the T-kv KV Team label Oct 17, 2021
@ajwerner
Copy link
Contributor

It seems to me like the RangeFeed ought to opt into receiving these (much like SSTs) and regular changefeeds should not except them and should fail if they see one.

@gh-casper gh-casper self-assigned this Apr 19, 2022
@erikgrinaker erikgrinaker changed the title kvserver: rangefeed and CDC support for DeleteRange kvserver: C2C and CDC handling of DeleteRange Jun 5, 2022
@erikgrinaker
Copy link
Contributor Author

I split out the rangefeed implementation to #82449, and repurposed this for any C2C and CDC work. Working on the rangefeed bits now, and expect to have a PR ready shortly -- there's still work remaining on MVCC range tombstones themselves though.

@erikgrinaker erikgrinaker removed A-kv-replication Relating to Raft, consensus, and coordination. T-kv-replication labels Jun 5, 2022
@gh-casper gh-casper added the A-tenant-streaming Including cluster streaming label Jun 9, 2022
miretskiy pushed a commit to miretskiy/cockroach that referenced this issue Jun 10, 2022
Informs cockroachdb#70433
Permanet fix is being tracked by the above issue.
This is a temporary fix to ensure that change aggregators
cancel their context when the server quiesces so that the
server shutdown is not inhibited by the running changefeeds.
@erikgrinaker erikgrinaker changed the title kvserver: C2C and CDC handling of DeleteRange kvserver: C2C handling of DeleteRange aka MVCC range tombstones Jun 15, 2022
@shermanCRL shermanCRL added this to the 22.2 milestone Jun 28, 2022
craig bot pushed a commit that referenced this issue Jun 28, 2022
82352: server, sql: surface session txnCount, recent txn fingerprints, active time r=xinhaoz a=xinhaoz

Finishing up Gerardo's PR, original review here: #80717

--------------------------------------------------
Partially addresses #74257.

Previously, the status server did not provide session details such as
total number of transactions executed, transaction fingerprint
IDs, and total active time. This change adds the aforementioned session
details to the `serverpb.Session` struct.

To track recently executed transaction fingerprint IDs, a FIFO cache
`TxnFingerprintIDCache` is introduced with its corresponding cluster
setting `TxnFingerprintIDBufferCapacity` to control the capacity. The
default capacity is set at 100 fingerprints.

The total number of transactions executed is filled using the existing
`txnCounter` from the `extraTxnState` in `connExecutor`. The total active
time is calculated by introducing a `timeutil.StopWatch` to the connection
executor, which is started and stopped when a transaction is started and
finished respectively.

Release note (api change): the `serverpb.Session` struct now has three
new fields: number of transactions executed, transaction fingerprint
IDs, and total active time.

82623: backupinfo: introduce a backupinfo package r=stevendanna a=adityamaru

The backupinfo package contains logic related to interacting
with information and metadata describing the backup. After this
change we have `backupdest` depending on `backupinfo`.

Release note: None

82718: kvserver: emit MVCC range tombstones over rangefeeds r=aliher1911 a=erikgrinaker

This patch adds MVCC range tombstone support in rangefeeds. Whenever an
MVCC range tombstone is written, a new `MVCCDeleteRangeOp` logical op
is recorded and emitted across the rangefeed as a `RangeFeedDeleteRange`
event. MVCC range tombstones will only be written when the
`MVCCRangeTombstones` version gate has been enabled.

Changefeeds will emit an error for these events. We do not expect to see
these in online spans with changefeeds, since they are initially only
planned for use with schema GC and import rollbacks.

The rangefeed client library has been extended with support for these
events, but no existing callers handle them for the same reason as
changefeeds. Initial scans do not emit regular tombstones, and thus not
range tombstones either, but catchup scans will emit them if
encountered.

This patch has rudimentary testing of MVCC range tombstones in
rangefeeds. A later patch will add a data-driven test harness for
rangefeeds with more exhaustive tests.

Resolves #82449.
Touches #70433.

Release note: None

82936: sql/schemachanger: implement DROP OWNED BY r=jasonmchan a=jasonmchan

Previously, we did not support the DROP OWNED BY statement (#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.

83229: ui: remove option 10/30 min from SQL Activity page r=maryliag a=maryliag

Note to reviewers: only 2nd commit is relevant to this PR

Previously we had the options for 10 and 30min on
SQL Activity pages, which created some confusion, since
we would always show the last 1h info.
This commit remove those 2 options.
If the user select any of those options on the Metrics
page, it will get updated to 1h on the SQL Activity
pages.

<img width="444" alt="Screen Shot 2022-06-22 at 5 43 53 PM" src="https://user-images.githubusercontent.com/1017486/175144243-2f084e0b-5e09-4874-9640-e7eea6179343.png">

https://www.loom.com/share/226e54322df6456aa2039b5c54f72eb1


Fixes #82914

Release note (ui change): Removal of the 10 and 30min options
on the SQL Activity page.

83420: ui: improve tooltip UX with text updates r=ericharmeling a=ericharmeling

Fixes #81374.
Fixes #83256.
Fixes #81248.
Fixes #79018.

Note the following:

- The updates resolving #79018 effectively revert the tooltip text for Rows Read to the original wording (which [was updated for accuracy](e379e9d#diff-492398441e971e355a687a4ce333a9766e2195287d0227682444d5dc0eb7ee1a)). I assume this is okay. `@kevin-v-ngo`
- The updates resolving #81248 do not in fact refer to the time intervals as date ranges, as this language is misleading (a 1h interval is an interval and not a date range). Instead, this update just removes the anchor and the link to the non-existent Interval Range section of https://www.cockroachlabs.com/docs/stable/ui-statements-page.html. We may want to consider updating the docs to call the "time picker" data type a time interval and not a date range. This appears to have been the case in previous releases (https://www.cockroachlabs.com/docs/v21.1/ui-statements-page#time-interval). `@stbof` 

Release note (ui change): Updated tooltips on the Statements and
Transactions pages in the DB Console for improved UX.

83428: sql: rename anonymizedStmt in sqlstats pkg to stmtNoConstants r=ericharmeling a=ericharmeling

Note that this commit does not change any files outside the sqlstats package.

Fixes #80725.

Release note: None

83468: ui: update all dates to use same format r=maryliag a=maryliag

Update all dates to use the same format.

Fixes #81159

Release note: None

83520: kv: don't try to reject lease transfer when flushing proposal buffer r=nvanbenschoten a=nvanbenschoten

Fixes #83498.
Fixes #83402.
Fixes #83308.

This was fallout from #82758.

This commit adds logic to `propBuf.maybeRejectUnsafeProposalLocked` to avoid
trying to reject proposals based on the state of the raft group when the group
is not provided (e.g. when flushing the buffer). We already had this logic for
`RequestLease` (indirectly), but did not for `TransferLease`.

Co-authored-by: Gerardo Torres <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Jason Chan <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@shermanCRL
Copy link
Contributor

Addresses #83810

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cdc Change Data Capture A-tenant-streaming Including cluster streaming C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) sync-me-8 T-cdc
Projects
None yet
7 participants