[FLINK-39533][s3][backport] Use abort() instead of drain on close/seek when remaining bytes exceed threshold in NativeS3InputStream#28052
Merged
gaborgsomogyi merged 1 commit intoApr 28, 2026
Conversation
…aining bytes exceed threshold in NativeS3InputStream * [FLINK-39533][s3] Use abort() instead of drain on close/seek when remaining bytes exceed threshold in NativeS3InputStream * [FLINK-39533][s3] Address to review comments. Bug at the end of stream and addres to s3 returning 416 (cherry picked from commit 3d6dff7)
Contributor
Author
|
@gaborgsomogyi PTAL |
Collaborator
Contributor
Author
|
@gaborgsomogyi / @RocMarshal can you help merging the changes |
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
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.
What is the purpose of the change
Backport of #28012 (master, merged as
3d6dff7f76ee4da1bc9e095a311d9055780222da) torelease-2.3.NativeS3InputStreamcallsResponseInputStream.close()when releasing streams duringseek(),skip(), andclose()operations. Apache HttpClient'sclose()implementation drains all remaining bytes from the response body to enable HTTP connection reuse. For large S3 objects where only a small portion was read (e.g. checkpoint metadata from a multi-GB state file), this drains potentially gigabytes of data over the network — causing severe latency during checkpoint restore and seek-heavy read patterns.The AWS SDK v2
ResponseInputStreamJavaDoc explicitly recommends callingabort()when remaining data is not needed. This change replacesclose()withabort()in the stream release path, plus the follow-ups from review:lazyInitialize()annotated@GuardedBy("lock")with the same runtime precondition as the other lock-guarded helpers, so a future lock-move refactor cannot silently break the invariants.position >= contentLength) is now evaluated beforelazyInitialize()in bothread()andread(byte[], int, int).positionandcontentLengthare authoritative, so we never need a stream just to return-1. This is also a prerequisite for (3).seek(contentLength)andskip()to EOF used to issuebytes=contentLength-on the open stream — RFC 7233 declares this range unsatisfiable and S3 returns HTTP 416. Both paths now release the stream instead of reopening it; subsequent reads short-circuit via the EOF check above. Factored into a singlerepositionOpenStream()helper used by both call sites.Brief change log
NativeS3InputStream:releaseStream()aborts before closing so subsequent SDKclose()does not drain.lazyInitialize()is now@GuardedBy("lock")with a runtime lock-held assertion.lazyInitialize()inread()/read(byte[], int, int).repositionOpenStream()helper called fromseek()/skip(); releases instead of reopening at EOF.seek()throwsEOFException(matching the Hadoop S3A contract) for negative positions and seeks pastcontentLength.NativeS3InputStreamTest(new): 13 tests covering abort-before-close ordering, position tracking, EOF semantics, error paths, and the seek/skip-to-EOF no-reopen invariant.Verifying this change
This change added tests and was verified as follows:
3d6dff7f76ee4da1bc9e095a311d9055780222da(git diffis empty).mvn -pl flink-filesystems/flink-s3-fs-native test -Dtest=NativeS3InputStreamTest→ 13/13 passing on this branch.ap-south-1). Triggeredstop-with-savepoint, restored from the savepoint, ran further checkpoints. Confirmed:chk-15, 3.2 GB) completed successfully.416/InvalidRange, norequires lock to be heldassertions, noError aborting S3/Error closingwarnings, no leaked-stream notices, and zero ERROR-level entries.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
Was generative AI tooling used to co-author this PR?