damansingh1313 opened a new pull request, #17965: URL: https://github.com/apache/iceberg/pull/17965
## Why is this change needed? `OSSInputStream` (aliyun) and `EcsSeekableInputStream` (dell) advance `pos` (and `next`, in OSS's case) and increment the `readBytes`/`readOperations` metric counters *before* checking whether the underlying stream has reached EOF (`-1`). When a read call actually returns `-1`: - The tracked position is advanced by one byte (`read()`) or decremented by one byte (`read(byte[], int, int)`, since `-1` is added to `pos`/`next`), desyncing the stream's internal position bookkeeping from the real position in the object. - The `readBytes` counter is incremented by `-1` in the buffered-read path, producing a nonsensical negative byte count in read metrics. `S3InputStream`, `GCSInputStream`, and `ADLSInputStream` already guard against this (fixed in #16055); `OSSInputStream` and `EcsSeekableInputStream` were the two implementations left over from that fix. This PR applies the same guard to both. A previous attempt at this exact fix (#16266) was opened but auto-closed for inactivity rather than rejected on merit — this PR picks that up with tests added for both the single-byte and buffered `read()` overloads on each class. Closes #16062 ## What user-facing changes does this include? None. This only affects internal position tracking and read metrics inside `OSSInputStream` and `EcsSeekableInputStream` when a read reaches EOF; there is no change to any public API, table format, or configuration. ## Are there any user-facing changes? - Fixes metrics reporting a negative byte count when a buffered read hits EOF. - Fixes internal position tracking so a stream that has hit EOF doesn't end up with a position off by one byte relative to the actual object length. ## How was this change tested? Added `testReadSingleEOF` and `testReadBufferedEOF` to both `TestOSSInputStream` and `TestEcsSeekableInputStream`, mirroring the tests added for `S3InputStream`/`GCSInputStream`/`ADLSInputStream` in #16055. Verified locally that all four new tests fail against the pre-fix code and pass after the fix; ran `spotlessCheck` for both modules. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
