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

release-19.1: storage/rangefeed: strip incompatible information from RangeFeedEvents #44082

Merged

Conversation

@nvanbenschoten
Copy link
Member

nvanbenschoten commented Jan 16, 2020

Backport 2/2 commits from #44035.

/cc @cockroachdb/release


Fixes #43967.

This commit adds a step before publishing RangeFeedEvents to RangeFeed registrations where incompatible information in the events is stripped from the event. There are two cases where this comes up at the moment:

The first is with previous values. If no registrations for the current Range are requesting previous values, then we won't even retrieve them on the Raft goroutine. However, if any are and they overlap with an update then the previous value on the corresponding events will be populated. If we're in this case and any other registrations don't want previous values then we'll need to strip them.

The second is with the key spans in checkpoint events. Checkpoint events are always created spanning the entire Range. However, a registration might not be listening on updates over the entire Range. If this is the case then we need to constrain the checkpoint events published to that registration to just the span that it's listening on. This is more than just a convenience to consumers - it would be incorrect to say that a rangefeed has observed all values up to the checkpoint timestamp over a given key span if any updates to that span have been filtered out.

Release note (bug fix): CDC is no longer susceptible to a bug where a resolved timestamp might be published before all events that precede it have been published in the presence of a Range merge.

This commit fixes a bug in these tests where multiple RangeFeedEvents
that were meant to be distinguishable were assigned the same keys. This
should have been fine because the test was assuming pointer equality
throughout, but `require.Equal` actually performs deep equality, so the
different events were actually considered equal.

Release note: None
Fixes #43967.

This commit adds a step before publishing RangeFeedEvents to RangeFeed
registrations where incompatible information in the events is stripped
from the event. There are two cases where this comes up at the moment:

The first is with previous values. If no registrations for the current
Range are requesting previous values, then we won't even retrieve them
on the Raft goroutine. However, if any are and they overlap with an
update then the previous value on the corresponding events will be
populated. If we're in this case and any other registrations don't want
previous values then we'll need to strip them.

The second is with the key spans in checkpoint events. Checkpoint events
are always created spanning the entire Range. However, a registration
might not be listening on updates over the entire Range. If this is the
case then we need to constrain the checkpoint events published to that
registration to just the span that it's listening on. This is more than
just a convenience to consumers - it would be incorrect to say that a
rangefeed has observed all values up to the checkpoint timestamp over
a given key span if any updates to that span have been filtered out.

Release note (bug fix): CDC is no longer susceptible to a bug where
a resolved timestamp might be published before all events that precede
it have been published in the presence of a Range merge.
@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Jan 16, 2020

This change is Reviewable

@danhhz
danhhz approved these changes Jan 16, 2020
@nvanbenschoten nvanbenschoten merged commit 265263f into cockroachdb:release-19.1 Jan 16, 2020
2 checks passed
2 checks passed
GitHub CI (Cockroach) TeamCity build finished
Details
license/cla Contributor License Agreement is signed.
Details
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten:backport19.1-44077 branch Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.