Fix fd remaining open longer than necessary #2616
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dagnir
force-pushed
the
file-publisher-fd-fix
branch
3 times, most recently
from
d505415 to
12c4559
Compare
zoewangg
reviewed
Jul 22, 2021
.../sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/FileAsyncRequestBody.java
Show resolved
Hide resolved
.../sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/FileAsyncRequestBody.java
Show resolved
Hide resolved
|
Integ tests failure looks unrelated (issue with S3 buckets). Will clean up and rerun |
Done |
zoewangg
approved these changes
Jul 22, 2021
In FileAsyncRequestBody, the file was not being closed until a request() read past the end of the file without ready any bytes; i.e. if a read() call reads up to the end of the file, it would not complete the subscription on the subsequent request() signal. This fixes this issue by signaling complete if we know we've reached the end.
dagnir
force-pushed
the
file-publisher-fd-fix
branch
from
4bf5c4d to
a39f2fb
Compare
|
Kudos, SonarCloud Quality Gate passed! |
joviegas
approved these changes
Jul 22, 2021
zoewangg
added a commit
that referenced
this pull request
Aug 10, 2021
* Add download implementation for API requests * Add S3CrtAsyncClient skeleton * Add support for adapting the sdk configurations to crt configurations. * Implement getObject in S3CrtAsyncClient * Implement S3 CRT upload * Fix checkstyle errors, add stability tests * Upgrade crt to 0.11.5 * Update APIs * Fix CRT S3 Client builder - Return the correct type from builder methods - Make max throughput and part size nullable * Implement S3TransferManager#upload * Implement TransferManager download and add s3-benchmarks module * Move s3 crt related classes to s3-transfermanager * Add requestLogger and requestIdLogger * Various fixes and refactoring * Add user-agent header in transfer manager * Refactor UserAgentUtils and make it a protected API * Fix sdk-benchmarks version * Rename module and package for transfermanager * Fix stability tests to not create a new client per request * Various renaming * Remove the option to pass bucket and key to DownloadRequest and UploadRequest and remove unused files * Add README for S3TransferManager * Add validation in S3ClientConfiguration * Fix CRT/SDK POJO conversion and add more tests * Update to use CRT DelegateCredentialsProvider * Provide HTTP request-level data to PutObjectResponse * Update Javadocs * Fix the race condition in the FileAsyncRequestBody which causes the request to hang * Add request cancellation logic and tests * Fixed an issue where the future gets stuck when upload fails * Offload completion of execution future to a separate thread * Wrap CrtS3RuntimeException to SdkServiceException * Revert "Wrap CrtS3RuntimeException to SdkServiceException" * Update s3 crt client tests to verify resources are closed properly * Wrap CrtS3RuntimeException to S3ServiceException * Update version * Bump up CRT version, clean up resources in tests, fix credential adapter TODO (#2594) * Remove buffer copy since it's no longer needed (#2611) * Fix fd remaining open longer than necessary (#2616) In FileAsyncRequestBody, the file was not being closed until a request() read past the end of the file without ready any bytes; i.e. if a read() call reads up to the end of the file, it would not complete the subscription on the subsequent request() signal. This fixes this issue by signaling complete if we know we've reached the end. * Cancel sub on finish and exception (#2619) * Bump up crt version (#2640) Bump up crt version * Fix merge conflict * Merge 'origin/master' into transfer manager branch (#2647) Co-authored-by: Quanzzzz <[email protected]> Co-authored-by: John Viegas <[email protected]> Co-authored-by: Dongie Agnir <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
In FileAsyncRequestBody, the file was not being closed until a request()
read past the end of the file without ready any bytes; i.e. if a read()
call reads up to the end of the file, it would not complete the
subscription on the subsequent request() signal. This fixes this issue
by signaling complete if we know we've reached the end.
Description
This PR fixes the behavior by tracking how much of the file is has been read, closing the file as soon as we've read it entirely.
Testing
New tests, ran integ tests.
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsLicense