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

(maint) - Add connect-timeout to transport #216

Merged
merged 1 commit into from
May 4, 2023

Conversation

jordanbreen28
Copy link
Contributor

@jordanbreen28 jordanbreen28 commented May 4, 2023

This PR adds a connect-timeout flag to each provisioned node in the litmus_inventory, to help prevent transient timeout errors during testing.

Initially, this defaulted to 10s which caused numerous false errors due to timeouts, this increases the default to 2 minutes.

note - machines provisioned with the provision_service will be updated in a separate PR in provision_service

@jordanbreen28 jordanbreen28 force-pushed the maint-add_connect-timeout branch from 07e754b to 2db5d9b Compare May 4, 2023 10:29
@GSPatton
Copy link
Contributor

GSPatton commented May 4, 2023

this is great! I'm wondering if 2 mins is a bit long though with one retry?

I was thinking something like a maximum of 3 retries but each retry has a longer timeout.

E.g. 1st retry has timeout of 30s, 2nd of 1min and 3rd of 2mins? What do you think?

@jordanbreen28
Copy link
Contributor Author

jordanbreen28 commented May 4, 2023

this is great! I'm wondering if 2 mins is a bit long though with one retry?

I was thinking something like a maximum of 3 retries but each retry has a longer timeout.

E.g. 1st retry has timeout of 30s, 2nd of 1min and 3rd of 2mins? What do you think?

I get ya and this would be niche!
But this isn’t a if connection drops retry, but more of a maximum number of seconds that the client will wait for a response from the node before dropping the connection.

So before, the node had a maximum of 10 seconds to complete the process and return the result to the client, where as now it has two minutes.

So this essentially means, the client will wait up to two minutes for a response from the node before terminating the process and dropping the connection (thus causing the test suite to fail).
Hope that clears it up :-)

@GSPatton GSPatton merged commit caf25e5 into main May 4, 2023
@GSPatton GSPatton deleted the maint-add_connect-timeout branch May 4, 2023 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants