-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Adding server grpc netty permit-keep-alive-time and permit-keep-alive-without-calls #31287
Conversation
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
Hi, i am migrating a couple of plain grpc java services to use quarkus grpc. Those services have some custom configurations that were recommended by grpc-java maintainers (grpc/grpc-java#8995) to solve a connection reset issue caused by the infrastructure where we are running our services (Google Cloud Run). We need to establish smaller keep alive values that netty server would allow, and indicate that they can be done when the service is not receiving any RPC call. For this we need to configure these 2 properties: -permit keep alive time But unfortunately those options are not exposed by quarkus configuration, so after checking how the existing configurable option for the netty server was done (https://github.com/quarkusio/quarkus/pull/14957/files) i decided to do similar and open this pr. Would you consider taking a look? thanks! |
224ae5f
to
6d42e34
Compare
Thanks! That's a nice addition! |
This comment has been minimized.
This comment has been minimized.
Make sure you apply the formatter? Can you build, and rebase? |
@cescoffier omw |
6d42e34
to
20b5268
Compare
|
This comment has been minimized.
This comment has been minimized.
@cescoffier could you merge it? or i should rebase |
Can you squash the commits? |
20b5268
to
357ba30
Compare
done! |
…ive-without-calls as configuration options
357ba30
to
1a716df
Compare
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
as configuration options