Skip to content

release-2.1: backport 7 rangefeed PRs#29500

Merged
craig[bot] merged 15 commits intocockroachdb:release-2.1from
nvb:backport2.1-28855-28970-29076-28912-29134-29219-28974
Sep 4, 2018
Merged

release-2.1: backport 7 rangefeed PRs#29500
craig[bot] merged 15 commits intocockroachdb:release-2.1from
nvb:backport2.1-28855-28970-29076-28912-29134-29219-28974

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Sep 4, 2018

Backport:

Please see individual PRs for details.

All cherry-picks were clean.

/cc @cockroachdb/release

nvb added 15 commits September 4, 2018 00:11
These aren't needed anymore.

Release note: None
This no longer needs to be thread-safe.

Release note: None
This state is already in ReplicaDescriptor.

Release note: None
This was not needed anymore. This should provide a small
but real perf win because we can avoid the context.WithCancel
call for every RPC.

Release note: None
The `OrigTimestamp` was being used to give a timestamp to the
txn in the logical op log because that is the timestamp the
intent is written at. However, the intent can't actually
commit until its `Timestamp`. This was causing issues with
closed timestamps, which forwards the `Txn.Timestamp` but don't
affect the `OrigTimestamp`. Without this change we were hitting
an assertion in the rangefeed package where a new intent looked
like it was below the resolved timestamp.

Release note: None
This change adds a new `EventChanTimeout` configuration to
`rangefeed.Processor`. The config specifies the maximum duration
that `ConsumeLogicalOps` and `ForwardClosedTS` will block on the
Processor's input channel. Once the duration is exceeded, these
methods will stops the processor with an error and tear it down.
This prevents slow consumers from making callers of the two methods
block.

The first method is used downstream of Raft, where blocking due to
slow rangefeed consumers for an unbounded amount of time is unacceptable.
It may require that we resize the processor's input buffer.

A TODO is left about reducing the impact of a single slow consumer
on an entire rangefeed.Processor. This can be left as a future
improvement.

Release note: None
The previous contract for `rangefeed.Stream.Send` was that its
events were unsafe for use after it returned, mandating a clone
if they were stored somewhere. This added extra complication and
will force local RangeFeed rpcs to perform clones even when they're
not necessary. Remove this "optimization" and strengthen the contract.

Release note: None
This commit adds a RangeFeed method to DistSender and teaches it how
to split rangefeeds across ranges and how to manage their individual
lifecycle events.

Release note: None
This commit adjusts the changefeed poller to use RangeFeed
requests when the corresponding cluster setting is enabled.

Release note: None
This PR includes two small perf tweak that came up
which running cdc/w=100/nodes=3/init=true.

Release note: None
These seemed to get stuck without this.

Release note: None
Ranges with expiration-based leases will not function properly
with closed timestamps, which means that we can't establish a
resolved timestamp on ranges that use them. Because of this,
we disallow rangefeeds on these ranges entirely.

This restriction may be lifted in the future.

Release note: None
This change hooks up closed timestamps into rangefeed. It does
so by creating a new closed timestamp subscription and notifying
all ranges with active rangefeed when new closed timestamp entries
arrive. This triggers each rangefeed to check for the maximum closed
timestamp on its range and use this to attempt to move its resolved
timestamp forward.

Eventually this will need some testing tied in with the closedts
package. That will have to wait until the closedts package is set
up to be more easily used in tests from the storage package (right
now `NoopContainer`s are used in tests everywhere). For now, we can
wait for integration testing with cdc.

Release note: None
@nvb nvb requested review from a team and danhhz September 4, 2018 04:12
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@tbg
Copy link
Copy Markdown
Member

tbg commented Sep 4, 2018 via email

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Sep 4, 2018

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 4, 2018

Build failed

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Sep 4, 2018

Flake due to #29271.

bors r+

craig bot pushed a commit that referenced this pull request Sep 4, 2018
29500: release-2.1: backport 7 rangefeed PRs r=nvanbenschoten a=nvanbenschoten

Backport:
  * 5/5 commits from "kv: give Transport a haircut" (#28855)
  * 1/1 commits from "engine: use Txn.Timestamp instead of OrigTimestamp for LogLogicalOp" (#28970)
  * 1/1 commits from "rangefeed: add release valve to logical op consumption" (#29076)
  * 3/3 commits from "kv: teach DistSender about RangeFeeds, use for changefeeds" (#28912)
  * 1/1 commits from "rangefeed: small perf-related changes" (#29134)
  * 2/2 commits from "kv: truncate RangeFeed span to range descriptor " (#29219)
  * 2/2 commits from "storage: hook closed timestamps into rangefeed" (#28974)

Please see individual PRs for details.

All cherry-picks were clean.

/cc @cockroachdb/release


Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 4, 2018

Build succeeded

@craig craig bot merged commit fb4afa1 into cockroachdb:release-2.1 Sep 4, 2018
@nvb nvb deleted the backport2.1-28855-28970-29076-28912-29134-29219-28974 branch September 11, 2018 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants