Github user shahidki31 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/23241#discussion_r239216282
  
    --- Diff: core/src/main/scala/org/apache/spark/io/CompressionCodec.scala ---
    @@ -197,4 +201,8 @@ class ZStdCompressionCodec(conf: SparkConf) extends 
CompressionCodec {
         // avoid overhead excessive of JNI call while trying to uncompress 
small amount of data.
         new BufferedInputStream(new ZstdInputStream(s), bufferSize)
       }
    +
    +  override def zstdEventLogCompressedInputStream(s: InputStream): 
InputStream = {
    +    new BufferedInputStream(new ZstdInputStream(s).setContinuous(true), 
bufferSize)
    --- End diff --
    
    Thanks @srowen for the review.
    Since, the CompressionCodec class used by many classes, we need to see any 
use case for, whether to read open frame for zstd case. As far as the 
eventLoggingListener class is concerned, it needs the open frame data also. So, 
I tried to change as minimal as possible without impacting the other calls.
    
    
    > I think that if we introduce a new method we might try to make it a 
little more general, like: compressedInputStreamForPartialFile or something. It 
would be good to avoid the isInstanceOf below.
    
    Yeah. This is a cleaner solution. Thanks.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to