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

Create timeout and onidle clients #1092

Open
Bolodya1997 opened this issue Sep 30, 2021 · 5 comments
Open

Create timeout and onidle clients #1092

Bolodya1997 opened this issue Sep 30, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@Bolodya1997
Copy link

Issue

Currently we have timeout for the initial Request, but we don't have any timeout for the retries to reestablishing the existing Connection. Imagine such case:

  1. NSC requests NSM for some service.
  2. NSM provides such service.
  3. Some time goes.
  4. NSM starts failing to provide such service.
  5. NSC starts requesting service again in infinite loop ([qfix] Add refresh retries on failure #1081 (comment)).

Possible solution

We can create timeout and completed client chain elements:

  1. timeout - just the same as a server timeout chain element, but it has some internal timeout instead of Conn.GetPrevPathSegment() expiration time.
  2. onidle - just the same as a server onidle chain element.

In such case client will close all failed Connections on some timeout and after close itself.

@Bolodya1997
Copy link
Author

@edwarnicke @denis-tingaikin
cc

@Bolodya1997 Bolodya1997 added the enhancement New feature or request label Sep 30, 2021
@denis-tingaikin
Copy link
Member

Can we just use attempts for refresh?

@Bolodya1997
Copy link
Author

Can we just use attempts for refresh?

We can use it instead of timeout, not instead of onidle.
I suppose that having 30 seconds timeout is more clear and predictable than 10 times refresh attempts.

@edwarnicke
Copy link
Member

@Bolodya1997 Could we simply not have the refresh loop check for expiration and terminate the loop locally if it is past expire?

@Bolodya1997
Copy link
Author

@Bolodya1997 Could we simply not have the refresh loop check for expiration and terminate the loop locally if it is past expire?

Yes, it should be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants