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

resolver/manual: support restarts, required for channel idleness #6638

Merged
merged 3 commits into from
Sep 15, 2023

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Sep 15, 2023

RELEASE NOTES: none

@easwars easwars requested a review from dfawley September 15, 2023 02:07
@easwars easwars added the Type: Feature New features or improvements in behavior label Sep 15, 2023
@easwars easwars added this to the 1.59 Release milestone Sep 15, 2023
resolver/manual/manual.go Outdated Show resolved Hide resolved
r.UpdateState(*r.bootstrapState)
if r.lastSeenState != nil {
err := r.CC.UpdateState(*r.lastSeenState)
go func(err error) { r.UpdateStateCallback(err) }(err)
Copy link
Member

Choose a reason for hiding this comment

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

What's the concern about using go r.UpdateStateCallback(err)? err can't be modified after this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I was paranoid beyond repair :P

@@ -26,7 +26,7 @@ import (
"google.golang.org/grpc/resolver"
)

// NewBuilderWithScheme creates a new test resolver builder with the given scheme.
// NewBuilderWithScheme creates a new manual resolver builder with the given scheme.
Copy link
Member

Choose a reason for hiding this comment

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

Should we document that this may only be used with a single ClientConn? Otherwise bad things happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@dfawley dfawley assigned easwars and unassigned dfawley Sep 15, 2023
// NewBuilderWithScheme creates a new manual resolver builder with the given scheme.
// NewBuilderWithScheme creates a new manual resolver builder with the given
// scheme. Every instance of the manual resolver may only ever be used with a
// single grpc.ClientConn. Otherwise, bad things will happen.
Copy link
Member

Choose a reason for hiding this comment

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

LMAO.

@easwars easwars merged commit 1880bd6 into grpc:master Sep 15, 2023
10 checks passed
@easwars easwars deleted the support_idle_in__manual_resolver branch September 15, 2023 17:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants