Skip to content

[OCI SDK] Add default retry policy to avoid HTTP_429 - #54

Merged
wy0824 merged 1 commit into
zoom:mainfrom
dannyeuu:dl-add-retry-on-oci-calls
Oct 23, 2025
Merged

wy0824 merged 1 commit into
zoom:mainfrom
dannyeuu:dl-add-retry-on-oci-calls

Conversation

@dannyeuu

@dannyeuu dannyeuu commented Oct 15, 2025 •

Copy link
Copy Markdown
Contributor

Help to improve OCI communication and resolves issue #52

@dannyeuu dannyeuu changed the title [OCI SDK] Add retry default retry policy [OCI SDK] Add default retry policy to avoid HTTP_429 Oct 15, 2025
case 429:
return true
case 409:
return svcErr.GetCode() == "IncorrectState"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check the DefaultRetryPolicy in oci sdk, seems you filter out some 5xx code, do you have any considerations about this part?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To ensure safer behavior, I removed the default retries for 500, 502, 503, and 504 errors from the documentation so that we only retry on TooManyRequests responses. The 50x “Out of Capacity” scenarios will be handled separately on a per-endpoint basis, consistent with the previous PR.

@wy0824

wy0824 commented Oct 16, 2025

Copy link
Copy Markdown
Collaborator

Good PR, I also want to add a retry and ratelimit for oci api call, exclude for the 429 codes, there should be a root cause leading the ratelimit. E.g., we found the "Out of host capacity" and "service limits were exceeded" also trigger karpenter-oci repeatible call create instance api, which may lead to 429. This has been fixed in a previous PR, but not release yet: https://github.com/zoom/karpenter-oci/pull/50/files

@dannyeuu

Copy link
Copy Markdown
Contributor Author

Good PR, I also want to add a retry and ratelimit for oci api call, exclude for the 429 codes, there should be a root cause leading the ratelimit. E.g., we found the "Out of host capacity" and "service limits were exceeded" also trigger karpenter-oci repeatible call create instance api, which may lead to 429. This has been fixed in a previous PR, but not release yet: https://github.com/zoom/karpenter-oci/pull/50/files

Thanks, @wy0824 . I saw your PR, and resolved a lot the shapes that are not avaluable and was still being requested. I have an internal build from the main branch =) but all the 4 clusters was still being facing 429, in all other endpoints like those in the screen:
image

So created this branch and being running this week in my clusters with this branch and reduced a lot the 429 and makes the pipelines not broken or being slow.

@wy0824

wy0824 commented Oct 17, 2025

Copy link
Copy Markdown
Collaborator

Checked your screenshot, all the endpoints in the screenshot meet the 429 response? Most of them are read api, the rate limit should not be so strict. Expect the 429, is there any other error code? And these 429 only happen in one region or all regions?

@dannyeuu

Copy link
Copy Markdown
Contributor Author

Checked your screenshot, all the endpoints in the screenshot meet the 429 response? Most of them are read api, the rate limit should not be so strict. Expect the 429, is there any other error code? And these 429 only happen in one region or all regions?

Yes, all of the endpoints shown in the screenshot can indeed reach the rate limit. After consulting with an Oracle engineer, I confirmed that these read-only endpoints share the same default rate limit configuration for all tenants.

Given this, I believe integrating the default retry mechanism through this PR is important to ensure better resilience against transient 429 responses and to minimize potential disruptions within the Karpenter-OCI control logic.

@wy0824

wy0824 commented Oct 23, 2025

Copy link
Copy Markdown
Collaborator

lgtm

@wy0824
wy0824 merged commit 9ccfa8b into zoom:main Oct 23, 2025
9 of 10 checks passed
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