Skip to content

Conversation

@markdroth
Copy link
Member

@markdroth markdroth commented Nov 4, 2025

This is a prerequisite for queuing calls in the subchannel, which will be needed for A105 (grpc/proposal#516).

Today, callers that need to start a call on a subchannel (primarily the client channel, but there are a couple of others) need to first ask the subchannel for the ConnectedSubchannel and then start the call on the ConnectedSubchannel. That works fine today, but when we add call queuing in the subchannel, this API would require us to acquire and release the subchannel's lock twice: once to get the ConnectedSubchannel, and again to add the call to the queue.

To avoid that, I have removed the ConnectedSubchannel from the subchannel's API and instead made it an internal implementation detail. Instead, the subchannel itself has methods for starting a call and doing pings.

This PR predominantly affects the v1 stack. There are essentially no changes to the v3 stack, since that code was already grabbing the UnstartedCallDestination out of the ConnectedSubchannel within a method on the subchannel.

This change is not completely trivial, but it should not cause any behavior changes, and it is sufficiently cross-cutting that I don't see an easy way to make it an experiment.

@markdroth markdroth changed the title [subchannel] restructure APIs for starting a subchannel call [subchannel] restructure APIs for starting a subchannel call in the v1 stack Nov 7, 2025
@markdroth markdroth changed the title [subchannel] restructure APIs for starting a subchannel call in the v1 stack [subchannel] restructure APIs for starting a subchannel call Nov 7, 2025
@markdroth markdroth marked this pull request as ready for review November 8, 2025 01:02
@markdroth markdroth requested a review from ctiller November 8, 2025 01:02
Copy link
Member

@ctiller ctiller left a comment

Choose a reason for hiding this comment

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

A couple of nits to take care of... let's discuss landing timing offline

watchers_;
};

class ConnectedSubchannel;
Copy link
Member

Choose a reason for hiding this comment

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

Describe what these are in a comment

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


void StartCallLocked()
// Returns false if there was no connection to start a call on.
bool StartCallLocked()
Copy link
Member

Choose a reason for hiding this comment

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

Recommend GPR_MUST_USE_RESULT here

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, done.

@grpc-checks grpc-checks bot added bloat/none and removed bloat/low labels Dec 2, 2025
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.

2 participants