[GitHub] carbondata pull request #3053: [WIP]JVM crash issue in snappy compressor

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/3053
 
     @manishgupta88 @xuchuanyin I think if it's really a problem with snappy then whether any performance impact is there or not we have to merge as its a functional issue. :)
    @akashrn5 May be this issue is coming because of offheap to onheap fallback in UnsafeMemoryManager can u please verify once. Please try discuss with snappy community also.



---

[qiuchenjian-2]

Github user akashrn5 commented on the issue:

    https://github.com/apache/carbondata/pull/3053
 
    @kumarvishal09 i have tested the fallback scenario by changing code, it is even failing with that also and i have raised discussion in snappy community also [https://groups.google.com/forum/#!topic/snappy-compression/4noNVKCMBqM](url)


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/3053
 
    @kumarvishal09 ...I agree with you that it is a functional issue and we need to merge it. My point was before merging we can do one load performance test to see if there is any performance degrade and if there is any then we can update the benchmark results


---

[qiuchenjian-2]

Github user akashrn5 commented on the issue:

    https://github.com/apache/carbondata/pull/3053
 
    > Does this PR fix two problems?
    > If it is yes, better to separate it into two.
    >
   
    the one line change of rowId to rowId + 1 is coupled with this, when i removed the compress method in unSafeFixLengthColumnPage, i got this issue and fixed in this, so this is required in this PR only


---