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

feat: Eager connect flag for Netty based client #1931

Merged
merged 4 commits into from
Apr 30, 2024

Conversation

johanandren
Copy link
Member

@johanandren johanandren commented Apr 17, 2024

Will cause the client creation to trigger connection to server rather than first on initial request, can help avoiding latency for the first requests caused by waiting for connection to complete.

References #1930

Will casue the client creation to trigger connection to server rather than
first on initial request, can help avoiding latency for the first requests
caused by waiting for connection to complete.
Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

looking good, maybe some more docs

# Request that the client try to connect the service immediately when the client is created
# rather than on the first request. Only supported for the Netty client backend, the akka-http client backend
# is always eager.
eager-connection = off
Copy link
Member

Choose a reason for hiding this comment

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

Shall we add something to the client reference docs about this? Seems like something you want to enable most of the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure it is something you want enabled most of the time, it since it would also means you might create a bunch of connections for clients that don't see any traffic so wastes resource usage (at least I'm guessing that is why it is lazy by default). For the record clients will also idle timeout after a default 5 minutes without any requests and switch to an "IDLE" state and only reconnect when a new request is made.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mentioning in docs seems good though.

Copy link
Member

Choose a reason for hiding this comment

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

ok, yes we should not change the default

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in 6222128

Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

runtime/src/main/resources/reference.conf Outdated Show resolved Hide resolved
runtime/src/main/resources/reference.conf Outdated Show resolved Hide resolved
Co-authored-by: Enno Runne <458526+ennru@users.noreply.github.com>
@johanandren johanandren merged commit 6ff0492 into main Apr 30, 2024
12 checks passed
@johanandren johanandren deleted the wip-eager-client-connect-1930 branch April 30, 2024 11:11
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.

3 participants