Skip to content

rpc: remove DRPC dependency from Peer and Connection generics #148609

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

Merged
merged 2 commits into from
Jun 24, 2025

Conversation

cthumuluru-crdb
Copy link
Contributor

@cthumuluru-crdb cthumuluru-crdb commented Jun 20, 2025

Although Peer and Connection are generic and intended to support both gRPC and DRPC connections, the current implementation has a hardcoded dependency on DRPC. As a result, a DRPC connection is always dialed, regardless of the intended type. This PR removes the direct dependency on DRPC, allowing the appropriate connection type (gRPC or DRPC) to be used based on the generic parameter.

Epic: CRDB-48923
Fixes: none
Release note: none

@cthumuluru-crdb cthumuluru-crdb changed the title rpc: remove DRPC dependency from Peer and Connection generics rpc: remove DRPC dependency from Peer and Connection generics Jun 20, 2025
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@cthumuluru-crdb cthumuluru-crdb added the do-not-merge bors won't merge a PR with this label. label Jun 20, 2025
@cthumuluru-crdb cthumuluru-crdb force-pushed the drpc-peer-pr2 branch 2 times, most recently from 4da8d25 to 0ee4f1e Compare June 23, 2025 18:23
Copy link

blathers-crl bot commented Jun 23, 2025

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cthumuluru-crdb cthumuluru-crdb marked this pull request as ready for review June 24, 2025 04:25
@cthumuluru-crdb cthumuluru-crdb requested a review from a team as a code owner June 24, 2025 04:25
Copy link
Contributor

@shubhamdhama shubhamdhama left a comment

Choose a reason for hiding this comment

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

LGTM, left some comments. I'll take another look and approve it.

@cthumuluru-crdb cthumuluru-crdb removed the do-not-merge bors won't merge a PR with this label. label Jun 24, 2025
@cthumuluru-crdb
Copy link
Contributor Author

@shubhamdhama, if you have any additional comments on this PR drop a comment here and I'll address them in a separate commit.

@cthumuluru-crdb
Copy link
Contributor Author

bors r+

craig bot pushed a commit that referenced this pull request Jun 24, 2025
148609: rpc: remove DRPC dependency from `Peer` and `Connection` generics r=cthumuluru-crdb a=cthumuluru-crdb

Although `Peer` and `Connection` are generic and intended to support both gRPC and DRPC connections, the current implementation has a hardcoded dependency on DRPC. As a result, a DRPC connection is always dialed, regardless of the intended type. This PR removes the direct dependency on DRPC, allowing the appropriate connection type (gRPC or DRPC) to be used based on the generic parameter.

Epic: CRDB-48923
Fixes: none
Release note: none

Co-authored-by: Chandra Thumuluru <chandra.thumuluru@cockroachlabs.com>
This is a follow-up to cockroachdb#145195, where we extracted KVBatch from the
Internal service. Here, we do the same with RangeFeed. The TL;DR is
that smaller services are easier to maintain and can be more smoothly
migrated to DRPC.

We also enable this service on the DRPC server. This is controlled by
`rpc.experimental_drpc.enabled` (off by default).

This change is part of a series and is similar to: cockroachdb#146926

Note: This only registers the service; the client is not updated to use the
DRPC client, so this service will not have any functional effect.

Epic: CRDB-48925
Release note: None
Although `Peer` and `Connection` are generic and intended to support both
gRPC and DRPC connections, the current implementation has a hardcoded
dependency on DRPC. As a result, a DRPC connection is always dialed,
regardless of the intended type. This PR removes the direct dependency
on DRPC, allowing the appropriate connection type (gRPC or DRPC) to be
used based on the generic parameter.

Epic: CRDB-48923
Fixes: none
Release note: none
@craig
Copy link
Contributor

craig bot commented Jun 24, 2025

Canceled.

@cthumuluru-crdb
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 24, 2025

@craig craig bot merged commit 7ca9db8 into cockroachdb:master Jun 24, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants