Skip to content

Fix failures in s3-regression-tests in merge queue + fix flakey s3 TM integ test #6343

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

Merged
merged 8 commits into from
Aug 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/s3-regression-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ jobs:
- name: Check for changes related to s3
id: check-changes
run: |
git fetch origin ${{ github.base_ref || github.event.merge_group.base_ref || github.ref }} --depth 1
CHANGED_FILES=$(git diff remotes/origin/${{ github.base_ref || github.event.merge_group.base_ref || github.ref }} --name-only)
BASE_REF=${{ github.base_ref || github.event.merge_group.base_ref || github.ref }}
BASE_REF=${BASE_REF#refs/heads/}
git fetch origin "$BASE_REF" --depth 1
CHANGED_FILES=$(git diff origin/"$BASE_REF" --name-only)
if echo "$CHANGED_FILES" | grep -q -E '^core/|^services/s3/|^services-custom/s3-transfer-manager/|^http-client-spi/|^http-clients/'; then
echo "Detected changes in S3, HTTP client, or core modules"
echo "has_s3_related_changes=true" >> $GITHUB_OUTPUT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public static void setUpForAllIntegTests() throws Exception {
s3CrtAsync = S3CrtAsyncClient.builder()
.credentialsProvider(CREDENTIALS_PROVIDER_CHAIN)
.region(DEFAULT_REGION)
.maxConcurrency(1)
.build();
tmCrt = S3TransferManager.builder()
.s3Client(s3CrtAsync)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public class S3TransferManagerDownloadPauseResumeIntegrationTest extends S3Integ
private static final Logger log = Logger.loggerFor(S3TransferManagerDownloadPauseResumeIntegrationTest.class);
private static final String BUCKET = temporaryBucketName(S3TransferManagerDownloadPauseResumeIntegrationTest.class);
private static final String KEY = "key";
// 24 * MB is chosen to make sure we have data written in the file already upon pausing.
private static final long OBJ_SIZE = 24 * MB;
// 50 * MB is chosen to make sure we have data written in the file already upon pausing.
private static final long OBJ_SIZE = 50 * MB;
private static File sourceFile;

@BeforeAll
Expand Down Expand Up @@ -152,8 +152,7 @@ void pauseAndResume_ObjectNotChanged_shouldResumeDownload(S3TransferManager tm)
assertThat(resumableFileDownload.s3ObjectLastModified()).hasValue(testDownloadListener.getObjectResponse.lastModified());
assertThat(bytesTransferred).isEqualTo(path.toFile().length());
assertThat(resumableFileDownload.totalSizeInBytes()).hasValue(sourceFile.length());

assertThat(bytesTransferred).isLessThan(sourceFile.length());
assertThat(bytesTransferred).isLessThanOrEqualTo(sourceFile.length());
assertThat(download.completionFuture()).isCancelled();

log.debug(() -> "Resuming download ");
Expand Down Expand Up @@ -242,7 +241,7 @@ private static void waitUntilFirstByteBufferDelivered(FileDownload download) {
.addAcceptor(WaiterAcceptor.retryOnResponseAcceptor(r -> true))
.overrideConfiguration(o -> o.waitTimeout(Duration.ofMinutes(1))
.maxAttempts(Integer.MAX_VALUE)
.backoffStrategy(FixedDelayBackoffStrategy.create(Duration.ofMillis(100))))
.backoffStrategy(FixedDelayBackoffStrategy.create(Duration.ofMillis(10))))
.build();
waiter.run(() -> download.progress().snapshot());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ void uploadDirectory_customMaxConcurrency_shouldLimitConcurrentOperations() thro
assertThat(observedPeak)
.as("Implementation allowed %d concurrent operations but was configured for %d",
observedPeak, configuredMaxConcurrency)
.isEqualTo(configuredMaxConcurrency);
.isLessThanOrEqualTo(configuredMaxConcurrency);

// Release the phase to let operations complete
phaser.arriveAndDeregister();
Expand Down