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

Endpoint::lazy_connect always tries to reconnect #460

Closed
wants to merge 1 commit into from

Conversation

boxdot
Copy link

@boxdot boxdot commented Sep 22, 2020

When a connection could not be established on first connection
attempt, an error was reported and the connection never tried to reconnect.
This behaviour is kept for Endpoint::connect and changed for
Endpoint::lazy_connect.

Motivation

The behaviour to fail on the first attempt was introduced in #413.
This makes tonic clients behave two-fold:

  1. If a client fails at the first connection attempt, the connection is broken
    and it will never try to reconnect.
  2. If a client connected successfully at the first attempt, but later the connection
    breaks, the client will resiliently try to reconnect.

As explained in #413 and #403, the behaviour in 1 is sometimes needed, however
there is another use case where the server we are connecting to is not yet up.
When connecting lazily, we expect the connection to
be always resilient, since we move the actual connection attempt to a later
point in time.

Solution

  • Keep the error reporting behaviour in Endpoint::connect.
  • Keep the resilient behaviour in Endpoint::connect_lazy.

Note

I would appreciate a proposal if there is a better way to achieve resilient initial reconnect. Also, I am not very happy to propagate the always_reconnect flag through several layers, but do not see a better way to do it.

When a connection could not be established on first connection
attempt, an error was reported and it never tried to  reconnect.
This behaviour is kepts for `Endpoint::connect` and changed for
`Endpoint::lazy_connect`.

Rationale: When connecting lazily, we expect the connection to
be always resilient, since we move the actual connection to a later
point in time. This is also useful, when connecting to services
that are not yet started.
@boxdot
Copy link
Author

boxdot commented Sep 22, 2020

After opening the PR saw #458. Sorry for not checking. Feel free to close in case you prefer the other solution.

@LucioFranco
Copy link
Member

Thanks for opening this, likely to go with #458

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

2 participants