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

fix: add keepalive to manually defined flag resolver channel #156

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nicklasl
Copy link
Member

@nicklasl nicklasl commented Aug 26, 2024

Adding the keep-alive functionality to the manually defined flag resolver grpc channel.

@@ -309,7 +309,10 @@ public Builder(@Nonnull String clientSecret) {

public Builder flagResolverManagedChannel(String host, int port) {
this.flagResolverManagedChannel =
ManagedChannelBuilder.forAddress(host, port).usePlaintext().build();
ManagedChannelBuilder.forAddress(host, port)
.keepAliveTime(Duration.ofMinutes(5).getSeconds(), TimeUnit.SECONDS)
Copy link
Contributor

Choose a reason for hiding this comment

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

I read up a bit on this, and maybe this setting won't hurt, but I don't think it's helping much either. It's the idle timeout that controls how long an inactive connection is kept open, and it defaults to infinity. There might still be an idea to send periodic keep alive to monitor the health of the connection, in our use-case I don't think it's necessary though. We would also need to set permit keep alive without calls as otherwise the keep alive is only active within a long streaming call, which seems to be it's primary purpose.

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.

2 participants