-
Notifications
You must be signed in to change notification settings - Fork 940
Fix Apache5 HTTP client retry failures with non-resettable streams #6154
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
Fix Apache5 HTTP client retry failures with non-resettable streams #6154
Conversation
… is implementation of HttpClientConnectionManager
…ring content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams
… Apache 4.x doesnot suports this
|
} | ||
|
||
@Test | ||
@DisplayName("Empty stream should be repeatable") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi just trying to understand, in this circumstance, is it equivalent to create an Entity with InputStream with zero length?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap , that is done here
Lines 151 to 154 in 0bdf878
private static InputStream getContent(Optional<ContentStreamProvider> contentStreamProvider) { | |
return contentStreamProvider.map(ContentStreamProvider::newStream) | |
.orElseGet(() -> new ByteArrayInputStream(new byte[0])); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add tests for concurrent access scenarios to ensure concurrent access to writeTo method is safe, or that has already been covered in other places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reason why writeTo_ConcurrentWrites_HandlesCorrectly
was removed is because concurrent reads from an inputStream is not thread safe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, concurrent reads from an InputStream are not thread-safe, and if we need to implement this functionality, we would need to add proper synchronization mechanisms. Since the Apache 4.x client doesn't have support for this, I removed this test case.
Note that I added this test case in my previous PR where I included several test cases to RepeatableInputStreamRequestEntityTest to verify all possible scenarios in both 4.x and 5.x versions to ensure consistent functionality. However, at that time, I didn't test for flakiness by running it in loop mode. When I later ran it as a @Repeated test for 1000 iterations, it started failing.
So initially I thought it was fine doing concurrent access for Apache 4.x since it was passing (it fails intermittently )
* Add initial empty module for Apache5x for seting up package (#6075) * Add initial module for Apache5x for seting up package * Add based on new module checklist * Baseline or Copy all the ApacheSDKHttpClient classes to newly added Apache5SDKHttpClient (#6088) * Add initial module for Apache5x for seting up package * Add based on new module checklist * Baseline all the classes from Apache4 SDK client to the new Apache5 module * Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues (#6100) * Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues * Handle comments from review * Handle comments from Zoe * Update the snap shot * Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability (#6132) * Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability * Upated test cases * Handled comments * Updated snap shot after merge from master * Revert "Updated snap shot after merge from master" This reverts commit 64f7d33. * Updated snap shot after merge from master * Fix architecture test failures for apache5.x (#6140) * Fix architecture test failures for apache5.x * Checkstyle issues * Updated snap shot after merge from master * Use reference of PoolingHttpClientConnectionManager instead of HttpClientConnectionManager for Connection Manager (#6147) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix Apache5 HTTP client retry failures with non-resettable streams (#6154) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Merge PR#6165 #6165 * Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Update snapshots * Do not buffer the Response stream using BufferedHttpEntity (#6200) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Merge from master * Apache5x SDkBenhmark Tests (#6206) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Clean up unused APIs and add test to make sure it can be handled with alternatives (#6211) * Clean up unused APIs and add test to make sure it can be handled with alternatives * Added NTCredentials to keep backward compatibilty with Apache4.x * Upgrade Apache5 org.apache.httpcomponents.client5 to latest available version (#6214) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview API annotation added for Public APIs and TODOs addressed (#6215) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview ready , addressing open TODOs * Added PublicApi since checkstyle was failing * Updated the snapshot * Updated thr Brazil package nma e to have preview as suffix * Updated Version as -PREVIEW to release apache5 as preview release (#6219) * Updated Version as -PREVIEW * japi cmp needs to be disables since this is a new version and we dont have old version to compare * Handled Surface API review comments (#6224) * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * update pom.xml for apache5.x * Handled commed for merge to master PR 6220 (#6240) * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * Handled comment for merge to master PR for apache 5.x * Added change logs * Review comment * Handled Review comments
* Add initial empty module for Apache5x for seting up package (#6075) * Add initial module for Apache5x for seting up package * Add based on new module checklist * Baseline or Copy all the ApacheSDKHttpClient classes to newly added Apache5SDKHttpClient (#6088) * Add initial module for Apache5x for seting up package * Add based on new module checklist * Baseline all the classes from Apache4 SDK client to the new Apache5 module * Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues (#6100) * Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues * Handle comments from review * Handle comments from Zoe * Update the snap shot * Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability (#6132) * Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability * Upated test cases * Handled comments * Updated snap shot after merge from master * Revert "Updated snap shot after merge from master" This reverts commit 64f7d33. * Updated snap shot after merge from master * Fix architecture test failures for apache5.x (#6140) * Fix architecture test failures for apache5.x * Checkstyle issues * Updated snap shot after merge from master * Use reference of PoolingHttpClientConnectionManager instead of HttpClientConnectionManager for Connection Manager (#6147) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix Apache5 HTTP client retry failures with non-resettable streams (#6154) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Merge PR#6165 #6165 * Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Update snapshots * Do not buffer the Response stream using BufferedHttpEntity (#6200) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Merge from master * Apache5x SDkBenhmark Tests (#6206) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Clean up unused APIs and add test to make sure it can be handled with alternatives (#6211) * Clean up unused APIs and add test to make sure it can be handled with alternatives * Added NTCredentials to keep backward compatibilty with Apache4.x * Upgrade Apache5 org.apache.httpcomponents.client5 to latest available version (#6214) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview API annotation added for Public APIs and TODOs addressed (#6215) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview ready , addressing open TODOs * Added PublicApi since checkstyle was failing * Updated the snapshot * Updated thr Brazil package nma e to have preview as suffix * Updated Version as -PREVIEW to release apache5 as preview release (#6219) * Updated Version as -PREVIEW * japi cmp needs to be disables since this is a new version and we dont have old version to compare * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * Handled Surface API review comments (#6224) * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * update pom.xml for apache5.x * Handled comment for merge to master PR for apache 5.x * Handled commed for merge to master PR 6220 (#6240) * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * Handled comment for merge to master PR for apache 5.x * Added change logs * Add Http client bench marks with S3 Get/Put * Removed some dependencies * updated performance comparison logic * updated readme * updated warmup time and iterations * remove performance comparison since we already have printed performamnce and one sent to cw * Review from github * Review comment * updatded prints * Handled Review comments * Added performance comparison for Apache * updated README * remove preview buid * added changed to skpi http client benchmark * updated the snapshot version * updated to run on jdk8 * merge from master * updated README.md * fixed checkbuild issues * fixed checksyle issues * fixed issue where secondary benchmarks were not found * Revert "fixed issue where secondary benchmarks were not found" This reverts commit b536571. * Revert "fixed checksyle issues" This reverts commit 99e697e. * Fixed checkstyle issues * Handled review comments * Updated snapshot * Add skipping of http-client-benchmarks in validate-brazil-config.yml * updated snapshot
…ted code (#6312) * Add initial empty module for Apache5x for seting up package (#6075) * Add initial module for Apache5x for seting up package * Add based on new module checklist * Baseline or Copy all the ApacheSDKHttpClient classes to newly added Apache5SDKHttpClient (#6088) * Add initial module for Apache5x for seting up package * Add based on new module checklist * Baseline all the classes from Apache4 SDK client to the new Apache5 module * Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues (#6100) * Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues * Handle comments from review * Handle comments from Zoe * Update the snap shot * Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability (#6132) * Fix HTTP authentication retry failures by improving RepeatableInputStreamRequestEntity repeatability * Upated test cases * Handled comments * Updated snap shot after merge from master * Revert "Updated snap shot after merge from master" This reverts commit 64f7d33. * Updated snap shot after merge from master * Fix architecture test failures for apache5.x (#6140) * Fix architecture test failures for apache5.x * Checkstyle issues * Updated snap shot after merge from master * Use reference of PoolingHttpClientConnectionManager instead of HttpClientConnectionManager for Connection Manager (#6147) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix Apache5 HTTP client retry failures with non-resettable streams (#6154) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Merge PR#6165 #6165 * Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Update snapshots * Do not buffer the Response stream using BufferedHttpEntity (#6200) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Merge from master * Apache5x SDkBenhmark Tests (#6206) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Clean up unused APIs and add test to make sure it can be handled with alternatives (#6211) * Clean up unused APIs and add test to make sure it can be handled with alternatives * Added NTCredentials to keep backward compatibilty with Apache4.x * Upgrade Apache5 org.apache.httpcomponents.client5 to latest available version (#6214) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview API annotation added for Public APIs and TODOs addressed (#6215) * Fix architecture test failures for apache5.x * Checkstyle issues * Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager * Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams * writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this * Fix connectionPoolingWorks by setting skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x * disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json * Added Test case for Async , handled review ocmments * Donot do buffer the response using BufferedHttpEntity since it might cause memory issue, this behaviour is same as Apache4.x * Fix compilation issues * Fix checkstyle issues * Remove test which are specific to apache http * Add benchmark for Apache5 and add Streaming Api test cases * Update Apache5 to 5.5 * Preview ready , addressing open TODOs * Added PublicApi since checkstyle was failing * Updated the snapshot * Updated thr Brazil package nma e to have preview as suffix * Updated Version as -PREVIEW to release apache5 as preview release (#6219) * Updated Version as -PREVIEW * japi cmp needs to be disables since this is a new version and we dont have old version to compare * Handled Surface API review comments (#6224) * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * update pom.xml for apache5.x * Handled commed for merge to master PR 6220 (#6240) * Handled Surface API review comments * Added a single test for localaddress , handled review comments * Removing internal package name as -preview after internal discussion * Fix transient text case failures * Handled comment for merge to master PR for apache 5.x * Added change logs * Review comment * Handled Review comments * Move connection timeout configuration from RequestConfig to ConnectionConfig in Apache HttpClient 5 (#6293) * Replacing deprecated API like connectionTimeout on RequestConfig and passing it via defaultconnectionconfigs * Handle review comments * Replace httpclient.execute call with httpclient.executeOpen as mentioned in deprecation notes of httpclient.execute (#6298) * Replacing deprecated API like connectionTimeout on RequestConfig and passing it via defaultconnectionconfigs * Handle review comments * replace httpclient.execute call with httpclient.executeOpen as mentioned in deprecation notes of httpclient.execute * nit updates * fixed sonar quebe issues * Replace deprecated SSLConnectionSocketFactory with recommended API (#6281) * Replace deprecated SSLConnectionSocketFactory with recommended API * Fixed checkstyle issues * Changed name tlsSocketStrategy on builder * Removed warning log * added more test cases * updated after review * Added ConnectionSocketFactory to Apache5Client builder same as Apache4 * handled PR comments * Removed unused classes after moving to SSL sockets * Added change logs * removed old change logs * remove unused imports * inermittent port used in wiremock fixed * update change logs
Motivation and Context
When using S3Client with Apache5 HTTP client, PutObject/GetObject requests fail immediately with the following error:
Root Cause:
Modifications
Added EntityCreationResult helper class
Stored content stream reference as a field
Fixed resource management
Testing
License