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

handle session timout #3

Closed
wants to merge 1 commit into from
Closed

handle session timout #3

wants to merge 1 commit into from

Conversation

roibaron
Copy link

retries the get request whenever it fails, using a backoff parameter to avoid server refuses

@yuvadm
Copy link
Owner

yuvadm commented May 13, 2021

Thanks for the PR @roibaron! I'm curious how useful you think this will be given that we already poll the API endpoint every second. Why do we need any retries here? Wouldn't it suffice to just drop any unsuccessful connection and wait for the next poll?

@roibaron
Copy link
Author

Hi @yuvadm, after ~30 mins of running the code crashes on my machine, due to multiple rejections. I saw you added an exception handler in the latest commit, however, due to multiple response with the same timeout, the server keeps rejecting the requests (for ~1 minute).
Adding exponential backoff solved the problem and allowed a smaller interval between failing request to response.

@yuvadm
Copy link
Owner

yuvadm commented May 14, 2021

Can you paste some logs of how it looks when that edge case is encountered? I'd be surprised if the server treats requests any differently if you exponentially backoff as opposed to just continuing to poll every second.

@roibaron
Copy link
Author

Sure, I added a counter and time to check the duration of the retries, which resets after a succesfull request. I got the following:
2021-05-14 12:19:56 Exception: HTTP request timed out,
number of retries: 7
time of failing: 12.229

While using Retry, I didn't encounter more than 2 retries, which is 3 seconds total.

@yuvadm
Copy link
Owner

yuvadm commented May 14, 2021

I've added your suggestion (in a12a455) for using an HTTP session object in a way that re-uses existing TCP connections, I believe that alone will provide significant improvement on potential timeouts.

@roibaron
Copy link
Author

you are probably right

@roibaron roibaron closed this May 14, 2021
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