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

fixes #807 - changes map for grpc connections to be a string key #816

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

nathanejohnson
Copy link
Member

@nathanejohnson nathanejohnson commented Feb 23, 2021

This should be a more complete fix for #807 , inspired by #808

@nathanejohnson
Copy link
Member Author

@qqwx1986 @cmvoicu can you take a look at this PR and see if this addresses your issue?

@qqwx1986
Copy link

qqwx1986 commented Feb 24, 2021

@qqwx1986 @cmvoicu can you take a look at this PR and see if this addresses your issue?

thanks, it's addresses the second problem. i need wait finish processing grpc requests although this target are uninstalled but not disconnected

if !hasTarget(tKey, table) {
   if cs.GetState() != connectivity.Ready {  // it will make sure finished all processing grpc requests  before remove
        log.Println("[DEBUG] grpc: cleaning up connection to", tKey)
        cs.Close()
        delete(p.connections, tKey)
   }
}

@Vetrenik
Copy link

Is there some progress with merging this pull request?

@yuriy-yarosh
Copy link

Yup, we need this...

@aaronhurt
Copy link
Member

We're just waiting on confirmation from @qqwx1986. If someone else in a similar situation could test and confirm that would be appreciated.

@qqwx1986
Copy link

qqwx1986 commented May 3, 2021

We're just waiting on confirmation from @qqwx1986. If someone else in a similar situation could test and confirm that would be appreciated.

i think this pr just fix one problem, and another i had reply up floor

@nathanejohnson nathanejohnson merged commit 5026127 into master Jul 12, 2022
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.

None yet

5 participants