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

fix: Retry on database wait operation requests #790

Merged

Conversation

lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Apr 12, 2023

📝 Description

This change alters the status polling logic in database-related resources to retry on 502 responses from the API within a certain tolerance. For the databases service, these errors do not necessarily reflect an internal error and instead may be the result of a transient error. This should significantly improve E2E test reliability.

Blocked by linode/linodego#319

✔️ How to Test

make PKG_NAME=linode/databasemysql testacc

@lgarber-akamai lgarber-akamai requested a review from a team April 12, 2023 15:59
@lgarber-akamai lgarber-akamai marked this pull request as draft April 12, 2023 16:05
@lgarber-akamai lgarber-akamai marked this pull request as ready for review April 12, 2023 18:17
@lgarber-akamai
Copy link
Contributor Author

/acctest sha=cc0f58708740afaef8cfc9a8727511604f8a8b97 pkg=linode/databasemysql

@lgarber-akamai
Copy link
Contributor Author

/acctest sha=cc0f58708740afaef8cfc9a8727511604f8a8b97 pkg=linode/databasepostgresql

@lgarber-akamai
Copy link
Contributor Author

/acctest sha=cc0f58708740afaef8cfc9a8727511604f8a8b97 pkg=linode/databaseaccesscontrols

@lgarber-akamai
Copy link
Contributor Author

/acctest sha=cc0f58708740afaef8cfc9a8727511604f8a8b97 pkg=linode/...

Copy link
Member

@zliang-akamai zliang-akamai left a comment

Choose a reason for hiding this comment

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

Nice work!

@zliang-akamai zliang-akamai requested a review from a team April 12, 2023 20:26
@zliang-akamai
Copy link
Member

/acctest sha=0202d41a6742d78f56845b1f04f73916221bf6f8 pkg=linode/...

@zliang-akamai
Copy link
Member

zliang-akamai commented Apr 13, 2023

Wondering if we should use MinRetryDelayMilliseconds and MaxRetryDelayMilliseconds from the config.

MinRetryDelayMilliseconds: d.Get("min_retry_delay_ms").(int),

"min_retry_delay_ms": {

But since I can't find where these config values were used anywhere else, maybe we can just remove these config values from config schema instead? Was there any historic context info regarding these config?

image

@lgarber-akamai
Copy link
Contributor Author

lgarber-akamai commented Apr 13, 2023

@zliang-akamai

Wondering if we should use MinRetryDelayMilliseconds and MaxRetryDelayMilliseconds from the config.

That's a good point! It'd probably be a good idea to keep retry timings configurable.

But since I can't find where these config values were used anywhere else, maybe we can just remove these config values from config schema instead?

These configs are passed into the linodego client and are used for retrying on 408 responses, etc.:

They're generally used to fine-tune API load for operations that might run into a lot of 408s (instance provisioning, etc.)

Hope that clears things up!

@zliang-akamai
Copy link
Member

@lgarber-akamai Thank you for the context info!

@zliang-akamai zliang-akamai requested a review from a team April 13, 2023 18:31
@lgarber-akamai
Copy link
Contributor Author

/acctest sha=c618f5abff8b851ae1703940ff75e04640bd24e1 pkg=linode/...

@zliang-akamai zliang-akamai requested a review from a team April 13, 2023 20:29
@lgarber-akamai lgarber-akamai merged commit cd36f28 into linode:dev Apr 17, 2023
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.

3 participants