Skip to content

Conversation

@SanjayMarreddi
Copy link
Contributor

@SanjayMarreddi SanjayMarreddi commented Jun 25, 2025

Description
This PR is a follow-up to the existing PRs #13347, #13348, #13361 in the series addressing feature gaps in the stream powered by analytics-accelerator-s3. It's part of our ongoing effort towards default-on integration.

This PR specifically adds support for S3 Async Clients to have similar configurations like S3 Sync Clients. Since the CRT clients have some feature gaps, we have raised an issue here and disabled CRT client. Java Async Client ( Netty ) will be the default Async client used now.

Note that there is one feature gap between Apache (sync) and Netty (async) clients: the APACHE_EXPECT_CONTINUE_ENABLED property is not supported by Netty. All other configuration options have equivalent implementations.

Testing

  • When s3.analytics-accelerator.enabled is set to true, previously skipped integration tests testCrossRegionAccessEnabled, testNewInputStreamWithCrossRegionAccessPoint work now.
  • Added relevant unit tests.

@SanjayMarreddi SanjayMarreddi force-pushed the update_s3_async_clients branch 3 times, most recently from f9b0fb4 to 7be4137 Compare July 8, 2025 15:39
@SanjayMarreddi SanjayMarreddi force-pushed the update_s3_async_clients branch from 7be4137 to da17914 Compare July 9, 2025 15:36

private S3AsyncClientWrapper flakyStreamAsyncClient(AtomicInteger counter, IOException failure) {
S3AsyncClientWrapper flakyClient = spy(new S3AsyncClientWrapper(s3AsyncClient()));
doAnswer(invocation -> new FlakyInputStream(invocation.callRealMethod(), counter, failure))
Copy link
Contributor

Choose a reason for hiding this comment

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

[from testing] I'm not sure if this wrapper is 100% right. I am seeing cast exceptions will look in to this.

Choose a reason for hiding this comment

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

Hi @stubz151 . I fixed the stubs with async client related changes. Now all tests with Async client are working as expected.

@github-actions
Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Aug 21, 2025
@guizmaii
Copy link

not stale

@github-actions github-actions bot removed the stale label Aug 22, 2025
@github-actions
Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 21, 2025
@guizmaii
Copy link

not stale

@github-actions github-actions bot removed the stale label Sep 23, 2025
@github-actions
Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 23, 2025
@guizmaii
Copy link

not stale

@github-actions github-actions bot removed the stale label Oct 24, 2025
@github-actions
Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 23, 2025
@guizmaii
Copy link

not stale

@github-actions github-actions bot removed the stale label Nov 24, 2025
@github-actions
Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants