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

Change middleware retry intervals #1322

Merged
merged 2 commits into from
Mar 19, 2021
Merged

Conversation

dmanjunath
Copy link
Contributor

@dmanjunath dmanjunath commented Mar 18, 2021

Description

Lower the retry interval in content node middleware from 5s to 1s. This lag affects all write operations like upload/signup etc. Functionally, there's zero impact on middlewares.

Rationale for this change
Libs already polls on 500ms interval to confirm changes from discovery. Also 5s seems like it was picked so the validation can be in lock step with discovery indexing, but we've observed that indexing changes can be applied quite randomly. When compounded, these 5 second waits can add up.

Tests

Ran the system locally and did an upload


// Initial delay before polling
await utils.timeout(1000)
const MaxRetries = 201
Copy link
Contributor Author

Choose a reason for hiding this comment

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

increased the duration by 1 sec because i got rid of the 1 sec initial timeout

@dmanjunath dmanjunath marked this pull request as ready for review March 18, 2021 23:41
@hareeshnagaraj hareeshnagaraj removed their request for review March 18, 2021 23:47
@hareeshnagaraj
Copy link
Contributor

Looks good to me, but I will leave ack to @SidSethi

Copy link
Member

@raymondjacobson raymondjacobson left a comment

Choose a reason for hiding this comment

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

question -- why have a max retries at all?

@SidSethi
Copy link
Contributor

question -- why have a max retries at all?

can't have CN get stuck infinitely retrying each user action right?

@dmanjunath dmanjunath merged commit 78413f0 into master Mar 19, 2021
@dmanjunath dmanjunath deleted the dm-middleware-retry-interval branch March 19, 2021 13:40
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

4 participants