Skip to content

kv: truncate RangeFeed span to range descriptor #29219

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
nvb:nvanbenschoten/rangefeedDistSenderFix
Aug 28, 2018
Merged

kv: truncate RangeFeed span to range descriptor #29219
craig[bot] merged 2 commits intocockroachdb:masterfrom
nvb:nvanbenschoten/rangefeedDistSenderFix

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Aug 28, 2018

Small fix for RangeFeed's interaction with DistSender.

@nvb nvb requested review from a team and danhhz August 28, 2018 21:30
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@danhhz danhhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: but sciencedog on the RSpan().AsRawSpanWithNoLocals() stuff

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


pkg/ccl/changefeedccl/changefeed_test.go, line 383 at r2 (raw file):

func TestChangefeedTruncateRenameDrop(t *testing.T) {
	defer leaktest.AfterTest(t)()
	t.Skip("here")

debugging?

These seemed to get stuck without this.

Release note: None
@nvb nvb force-pushed the nvanbenschoten/rangefeedDistSenderFix branch from 7a5e56a to 1056bdc Compare August 28, 2018 21:45
Copy link
Copy Markdown
Contributor Author

@nvb nvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


pkg/ccl/changefeedccl/changefeed_test.go, line 383 at r2 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

debugging?

Woops, removed! Thanks for catching that.

craig bot pushed a commit that referenced this pull request Aug 28, 2018
29135: opt: directive that asserts a rule was used r=justinj a=justinj

Brought up in #29050 as a possibility to avoid the case of certain rules
no longer applying.

Kind of a strawman right now because I'm not sure
* if this is the syntax we want,
* how widely we want to use it, I've just added it to one file for the
  purposes of this PR.

I think this is probably valuable in that it asserts we're testing what
we want to test, but I do think it's a little invasive as far as
cluttering up the test directives goes. Open to suggestions for how to
possibly improve.

Release note: None

29219: kv: truncate RangeFeed span to range descriptor  r=nvanbenschoten a=nvanbenschoten

Small fix for RangeFeed's interaction with DistSender.

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

craig bot commented Aug 28, 2018

Build succeeded

@craig craig bot merged commit 1056bdc into cockroachdb:master Aug 28, 2018
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>
@nvb nvb deleted the nvanbenschoten/rangefeedDistSenderFix branch September 11, 2018 15:42
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