Skip to content

rangefeed: small perf-related changes#29134

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/rangefeedOpt
Aug 27, 2018
Merged

rangefeed: small perf-related changes#29134
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/rangefeedOpt

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Aug 27, 2018

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

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
@nvb nvb requested review from a team and danhhz August 27, 2018 18:06
@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:

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


pkg/storage/rangefeed/processor.go, line 40 at r1 (raw file):

	// defaultCheckStreamsInterval is the default interval at which a Processor
	// will check all streams to make sure they have not been canceled.
	defaultCheckStreamsInterval = 1 * time.Second

maybe note in the comment why this got bumped up

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! 1 of 0 LGTMs obtained


pkg/storage/rangefeed/processor.go, line 40 at r1 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

maybe note in the comment why this got bumped up

Gave this a shot but nothing seemed very useful to document and keep around. The reason boils down to "I saw this come up as a blip on a profile and realized it was unnecessarily low".

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 27, 2018

Build failed (retrying...)

craig bot pushed a commit that referenced this pull request Aug 27, 2018
28721: sql: fix two bugs with TRUNCATE r=vivekmenezes a=vivekmenezes

sql: correctly resolve schema mutations during TRUNCATE
related to #27687

sql: mark schema change job as successful when table DROP/TRUNCATE
The job is modified in the same transaction as the user's DROP/TRUNCATE transaction.


29134: rangefeed: small perf-related changes r=nvanbenschoten a=nvanbenschoten

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

Release note: None

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

craig bot commented Aug 27, 2018

Build succeeded

@craig craig bot merged commit 2284f33 into cockroachdb:master Aug 27, 2018
@nvb nvb deleted the nvanbenschoten/rangefeedOpt branch August 27, 2018 23:12
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>
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.

4 participants