gaborgsomogyi commented on code in PR #25509:
URL: https://github.com/apache/flink/pull/25509#discussion_r1804812888
##########
flink-runtime/src/main/java/org/apache/flink/runtime/state/CompressibleFSDataInputStream.java:
##########
@@ -31,19 +31,21 @@ public class CompressibleFSDataInputStream extends
FSDataInputStream {
private final FSDataInputStream delegate;
private final InputStream compressingDelegate;
+ private final boolean compressed;
public CompressibleFSDataInputStream(
FSDataInputStream delegate, StreamCompressionDecorator
compressionDecorator)
throws IOException {
this.delegate = delegate;
this.compressingDelegate =
compressionDecorator.decorateWithCompression(delegate);
+ this.compressed = compressionDecorator !=
UncompressedStreamCompressionDecorator.INSTANCE;
}
@Override
public void seek(long desired) throws IOException {
- final int available = compressingDelegate.available();
- if (available > 0) {
- if (available != compressingDelegate.skip(available)) {
+ if (compressed) {
+ final int available = compressingDelegate.available();
+ if (available > 0 && available !=
compressingDelegate.skip(available)) {
throw new IOException("Unable to skip buffered data.");
}
}
Review Comment:
I agree that this area has improvement possibility but I've the following
reasons not to do that (at least in this PR):
* This is a blocker and as such I would keep the fix as small as possible
not to break things
* The main issue here is that we don't have separate classes for
uncompressed and compressed `FSDataInputStream`, so if you ask me I would do
that step. I've tried that already and came to the conclusion that it would
lead far because this is a pattern in Flink like
`CompressibleFSDataOutputStream` and all the streams which are in `has-a`
context instead of being a real decorator around each other. So all in all
introducing another level of layer as an interface would be overkill here.
--
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]
