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

client: support a 1:1 mapping with acbws and addrConns #6302

Merged
merged 3 commits into from
May 23, 2023

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented May 19, 2023

In the process of making SubConn have a String method (for debug printing), I noticed that, since the addrConn below a SubConn can change out from underneath theSubConn, there would be a race if I tried to print the addrConn's channelz ID. But, in reality, the "subchannel" that exists in channelz corresponds to our SubConns, not our addrConns, so I moved the channelz ID over to the SubChannel.

RELEASE NOTES: none

@dfawley dfawley added this to the 1.56 Release milestone May 19, 2023
@dfawley dfawley requested a review from easwars May 19, 2023 17:59
@dfawley dfawley changed the title client: make channelz a feature of a SubConn, not an addrConn client: support a 1:1 mapping with acbws and addrConns May 19, 2023
@dfawley
Copy link
Member Author

dfawley commented May 19, 2023

It seems the thing we tried really hard to avoid doing was considerably easier than dealing with the hacks put in place to avoid doing it.

I.e. it's easier to make addrConn able to be updated than move the subchannel stats (channelz)/etc into acbw and ignore spurious updates from old addrConns once attached to acbws.

The main idea of the strategy for this is to use ac.ctx more often: previously it was canceled only when the whole addrConn was shut down. We now cancel it and recreate it whenever we want a connection attempt to stop. And since it is also canceled when the addrConn is shut down, we can replace most of the checks for ac.state == shutdown with ac.ctx.Err() != nil, because many of those uses also needed to exit when the current connection attempt was aborted.

clientconn.go Outdated Show resolved Hide resolved
balancer_conn_wrappers.go Show resolved Hide resolved
clientconn.go Outdated Show resolved Hide resolved
balancer_conn_wrappers.go Show resolved Hide resolved
clientconn.go Outdated Show resolved Hide resolved
clientconn.go Show resolved Hide resolved
@easwars easwars assigned dfawley and unassigned easwars May 19, 2023
@easwars easwars assigned easwars and dfawley and unassigned dfawley and easwars May 19, 2023
@dfawley dfawley merged commit e9799e7 into grpc:master May 23, 2023
1 check passed
@dfawley dfawley deleted the racefix branch May 23, 2023 16:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants