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

changefeedccl: Add options to enable the use of MuxRangeFeed RPC #86448

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

miretskiy
Copy link
Contributor

@miretskiy miretskiy commented Aug 19, 2022

Enable changefeed to use MuxRangeFeed RPC if
changefeed.mux_rangefeed.enabled is set to true.

Release note (enterprise change): Changefeeds may opt in
via changefeed.mux_rangefeed.enabled setting to use
MuxRangeFeed RPC which multiplexes multipe range feed streams
onto a single RPC stream per node.

Release justification: add option to enable functionality
that's disabled by default.

@miretskiy miretskiy requested review from a team as code owners August 19, 2022 13:31
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@miretskiy miretskiy force-pushed the usemux branch 4 times, most recently from 82c2604 to bf97c61 Compare August 20, 2022 12:13
@miretskiy miretskiy removed request for ajwerner and a team August 20, 2022 12:13
@miretskiy
Copy link
Contributor Author

@jayshrivastava I decided to remove 1 commit (adding similar option to rangefeed library). This one is now purely for changefeeds.

@miretskiy miretskiy changed the title changefeedccl,kv: Add options to enable the use of MuxRangeFeed RPC changefeedccl: Add options to enable the use of MuxRangeFeed RPC Aug 20, 2022
@@ -74,8 +75,13 @@ func (p rangefeedFactory) Run(ctx context.Context, sink kvevent.Writer, cfg rang
}
g := ctxgroup.WithContext(ctx)
g.GoCtx(feed.addEventsToBuffer)
var rfOpts []kvcoord.RangeFeedOption
if cfg.UseMux {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that this is ever set to true. Please add UseMux: f.useMux, to the struct initialization in kvevent.runUntilTableEvent.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume what I said above is the intended behavior. As in, the changefeed should pass the setting down to the underlying rangefeed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh. Nice catch.

Enable changefeed to use `MuxRangeFeed` RPC if
`changefeed.mux_rangefeed.enabled` is set to true.

Release note (enterprise change): Changefeeds may opt in
via `changefeed.mux_rangefeed.enabled` setting to use
`MuxRangeFeed` RPC which multiplexes multipe range feed streams
onto a single RPC stream per node.

Release justification: add option to enable functionality
that's disabled by default.
@miretskiy
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 22, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 22, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 23, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 23, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 23, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 23, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 23, 2022

Build succeeded:

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