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

Instantiate backoff strategy per goroutine #496

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

yawangwang
Copy link
Collaborator

@yawangwang yawangwang commented Sep 25, 2024

Instantiate a new backoff object per goroutine since the current retry library is not thread-safe.

This PR also adds those tests related to RefreshToken + FetchContainerSignatures to a test suite with race condition check.

@yawangwang yawangwang force-pushed the replace_retry_library branch from 932d3d1 to f5d7c57 Compare September 25, 2024 00:19
@yawangwang
Copy link
Collaborator Author

/gcbrun

MaxInterval = 3600 sec
MaxElapsedTime = 0 (never stops retrying)
WithCappedDuration = 3600 sec
MaxRetries = 8
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep the unlimited retry?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

my concern is the launcher will hang forever with unlimited retry if refreshToken keeps returning error. We might want the launcher exit in this case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nvm, this PR will use the original library, so we still keep the unlimited retry.

@yawangwang yawangwang force-pushed the replace_retry_library branch 2 times, most recently from 87ef467 to 2780449 Compare September 25, 2024 17:44
@yawangwang yawangwang changed the title Replace retry library with thread-safe implementation Instantiate backoff strategy per goroutine Sep 25, 2024
@yawangwang
Copy link
Collaborator Author

/gcbrun

@yawangwang yawangwang force-pushed the replace_retry_library branch from 2780449 to b668068 Compare September 25, 2024 18:43
@yawangwang yawangwang force-pushed the replace_retry_library branch from b668068 to 342de83 Compare September 25, 2024 19:01

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@yawangwang yawangwang force-pushed the replace_retry_library branch from 342de83 to a3f0425 Compare September 25, 2024 19:21
@yawangwang
Copy link
Collaborator Author

/gcbrun

@yawangwang yawangwang merged commit 78eb710 into google:main Sep 25, 2024
11 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.

None yet

4 participants