[GitHub] carbondata pull request #3000: [CARBONDATA-3181][BloomDataMap] Fix access fi...

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #3000: [CARBONDATA-3181][BloomDataMap] Fix access fi...

qiuchenjian-2
GitHub user kevinjmh opened a pull request:

    https://github.com/apache/carbondata/pull/3000

    [CARBONDATA-3181][BloomDataMap] Fix access field error for BitSet in bloom filter

    **Problem**
    java.lang.IllegalAccessError is thrown when query on bloom filter without compress on CarbonThriftServer. Detailed log ss pasted at bottom.
   
    **Analyse**
    similar problem was occur when get/set BitSet in CarbonBloomFilter, it uses reflection to solve. We can do it like this. Since we have set the BitSet already, another easier way is to call super class method to avoid accessing it from CarbonBloomFilter
   
    **Solution**
    if bloom filter is not compressed, call super method to test membership
   
    ---
    *Detail log*
    ```
    18/12/19 11:16:07 ERROR thriftserver.SparkExecuteStatementOperation: Error executing query, currentState RUNNING,
    java.lang.IllegalAccessError: tried to access field org.apache.hadoop.util.bloom.BloomFilter.bits from class org.apache.hadoop.util.bloom.CarbonBloomFilter
            at org.apache.hadoop.util.bloom.CarbonBloomFilter.membershipTest(CarbonBloomFilter.java:70)
            at org.apache.carbondata.datamap.bloom.BloomCoarseGrainDataMap.prune(BloomCoarseGrainDataMap.java:202)
            at org.apache.carbondata.core.datamap.TableDataMap.pruneWithFilter(TableDataMap.java:185)
            at org.apache.carbondata.core.datamap.TableDataMap.prune(TableDataMap.java:160)
            at org.apache.carbondata.core.datamap.dev.expr.DataMapExprWrapperImpl.prune(DataMapExprWrapperImpl.java:53)
            at org.apache.carbondata.hadoop.api.CarbonInputFormat.getPrunedBlocklets(CarbonInputFormat.java:517)
            at org.apache.carbondata.hadoop.api.CarbonInputFormat.getDataBlocksOfSegment(CarbonInputFormat.java:412)
            at org.apache.carbondata.hadoop.api.CarbonTableInputFormat.getSplits(CarbonTableInputFormat.java:529)
            at org.apache.carbondata.hadoop.api.CarbonTableInputFormat.getSplits(CarbonTableInputFormat.java:220)
            at org.apache.carbondata.spark.rdd.CarbonScanRDD.internalGetPartitions(CarbonScanRDD.scala:127)
            at org.apache.carbondata.spark.rdd.CarbonRDD.getPartitions(CarbonRDD.scala:66)
            at org.apache.spark.rdd.RDD$$anonfun$partitions$2.apply(RDD.scala:252)
            at org.apache.spark.rdd.RDD$$anonfun$partitions$2.apply(RDD.scala:250)
    ```
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kevinjmh/carbondata bloom_bits_access

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/3000.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3000
   
----
commit b7df6abe73a633c0df79c5b426df9416d7a4c1bc
Author: Manhua <kevinjmh@...>
Date:   2018-12-19T03:30:46Z

    fix access error for bits in bloom filter

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #3000: [CARBONDATA-3181][BloomDataMap] Fix access field err...

qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3000
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1841/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #3000: [CARBONDATA-3181][BloomDataMap] Fix access field err...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3000
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10098/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #3000: [CARBONDATA-3181][BloomDataMap] Fix access field err...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3000
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2049/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #3000: [CARBONDATA-3181][BloomDataMap] Fix access fi...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3000#discussion_r242851840
 
    --- Diff: datamap/bloom/src/main/java/org/apache/hadoop/util/bloom/CarbonBloomFilter.java ---
    @@ -49,27 +49,23 @@ public CarbonBloomFilter(int vectorSize, int nbHash, int hashType, boolean compr
     
       @Override
       public boolean membershipTest(Key key) {
    -    if (key == null) {
    -      throw new NullPointerException("key cannot be null");
    -    }
    -
    -    int[] h = hash.hash(key);
    -    hash.clear();
         if (compress) {
           // If it is compressed check in roaring bitmap
    +      if (key == null) {
    --- End diff --
   
    I think "key == null" can put out of the branch, because it can reduce one function call. Although super.membershipTest also judge it


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #3000: [CARBONDATA-3181][BloomDataMap] Fix access fi...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user kevinjmh commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3000#discussion_r243130074
 
    --- Diff: datamap/bloom/src/main/java/org/apache/hadoop/util/bloom/CarbonBloomFilter.java ---
    @@ -49,27 +49,23 @@ public CarbonBloomFilter(int vectorSize, int nbHash, int hashType, boolean compr
     
       @Override
       public boolean membershipTest(Key key) {
    -    if (key == null) {
    -      throw new NullPointerException("key cannot be null");
    -    }
    -
    -    int[] h = hash.hash(key);
    -    hash.clear();
         if (compress) {
           // If it is compressed check in roaring bitmap
    +      if (key == null) {
    --- End diff --
   
    The reason I code it like this is for consistent procedure. And I don't think it can reduce call for GENERAL cases. Null is only one special case, and all the others are not-null cases. If change it as you advised, all non-null cases will have to do double null check when bloom filter is not compressed.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #3000: [CARBONDATA-3181][BloomDataMap] Fix access fi...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/3000#discussion_r243132507
 
    --- Diff: datamap/bloom/src/main/java/org/apache/hadoop/util/bloom/CarbonBloomFilter.java ---
    @@ -49,27 +49,23 @@ public CarbonBloomFilter(int vectorSize, int nbHash, int hashType, boolean compr
     
       @Override
       public boolean membershipTest(Key key) {
    -    if (key == null) {
    -      throw new NullPointerException("key cannot be null");
    -    }
    -
    -    int[] h = hash.hash(key);
    -    hash.clear();
         if (compress) {
           // If it is compressed check in roaring bitmap
    +      if (key == null) {
    --- End diff --
   
    got it


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #3000: [CARBONDATA-3181][BloomDataMap] Fix access field err...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/3000
 
    LGTM


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #3000: [CARBONDATA-3181][BloomDataMap] Fix access fi...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/3000


---