Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add retry #6437

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

Open
RanVaknin wants to merge 2 commits into master
base: master
Choose a base branch
Loading
from rvaknin/fix-flakey-test-s3Multipart
Open

Add retry #6437

RanVaknin wants to merge 2 commits into master from rvaknin/fix-flakey-test-s3Multipart

Conversation

@RanVaknin
Copy link
Contributor

@RanVaknin RanVaknin commented Sep 22, 2025
edited
Loading

The mpuDefaultSplitImpl_partsFailOfRetryableError_shouldFail test intermittently fails in CI environments following PR #6368 which replaced the direct SplittingPublisher constructor with a builder pattern, which likely changed the threading or timing behavior of when the NonRetryableException gets thrown versus when WireMock processes and counts the HTTP requests.

This seems to cause a race condition in the test where sometimes the test sees the expected single request and sometimes it sees multiple requests before the exception occurs:

[ERROR] software.amazon.awssdk.services.s3.internal.multipart.S3MultipartClientPutObjectWiremockTest.mpuDefaultSplitImpl_partsFailOfRetryableError_shouldFail(String, Long, ResponseDefinitionBuilder)[1] -- Time elapsed: 0.544 s <<< FAILURE!
com.github.tomakehurst.wiremock.client.VerificationException:
Expected exactly 1 requests matching the following pattern but received 2:

or

[ERROR] software.amazon.awssdk.services.s3.internal.multipart.S3MultipartClientPutObjectWiremockTest.mpuDefaultSplitImpl_partsFailOfRetryableError_shouldFail(String, Long, ResponseDefinitionBuilder)[2] -- Time elapsed: 0.151 s <<< FAILURE
java.lang.AssertionError:
Expecting throwable message:
"software.amazon.awssdk.core.exception.NonRetryableException: Multiple subscriptions detected. This could happen due to a retry attempt. The AsyncRequestBody implementation provided does not support splitting to retryable/resubscribable AsyncRequestBody. If you need retry capability or multiple subscriptions, consider using BufferedSplittableAsyncRequestBody to wrap your AsyncRequestBody. (SDK Attempt Count: 2)"
to contain:
"A retry was attempted, but"
but did not.

When testing this locally, I ran a parameterizedTest with 1000 iterations and saw 100% success rate even when running in parallel. This is probably failing in our CI build for some environment specific reasons. As first low effort measure, I added a simple retryer that will retry each parameterized test 3 times before failing.

@RanVaknin RanVaknin requested a review from a team as a code owner September 22, 2025 07:06
@RanVaknin RanVaknin added changelog-not-required Indicate changelog entry is not required for a specific PR no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required labels Sep 22, 2025
Copy link

return Optional.ofNullable(contentLength);
for (int i = 0; i < 3; i++) {
try {
stubUploadPartFailsInitialAttemptSucceedsUponRetryCalls(responseDefinitionBuilder);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe consider put this line out of the for loop to prevent multiple stubs. I'm not sure if this will just overwrite the previous stub or cause multiple stub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fred1155 Fred1155 Fred1155 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

changelog-not-required Indicate changelog entry is not required for a specific PR no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /