[GitHub] carbondata pull request #1346: Optimize memory and fix nosort queries

[qiuchenjian-2]

GitHub user ravipesala opened a pull request:

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

    Optimize memory and fix nosort queries

    1. Use UnsafeManager for dimension chunks as well to avoid leaks
    2. Fix filters on nosort columns.
    3. Optimize scanRDD


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

    $ git pull https://github.com/ravipesala/incubator-carbondata use-unsafe-manager

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

    https://github.com/apache/carbondata/pull/1346.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 #1346
   
----
commit 1a5e722a77a095a9620c55dffe384c5239e7bf2a
Author: Ravindra Pesala <[hidden email]>
Date:   2017-09-10T09:27:09Z

    Use UnsafeManager for dimension chunks as well.

commit 8439a9bfcb4fea473d9c366fb4571f17d8ed062b
Author: Ravindra Pesala <[hidden email]>
Date:   2017-09-10T09:44:19Z

    Fix nosort filter issue

commit dbbdbe85ad0621c7b5ac7a24182b32e29514b210
Author: Ravindra Pesala <[hidden email]>
Date:   2017-09-10T09:58:50Z

    Optimize ScanRDD

----


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/645/



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/658/



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/681/



---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/683/



---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/687/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/697/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/701/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/704/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1346#discussion_r138253487
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/store/impl/unsafe/UnsafeAbstractDimensionDataChunkStore.java ---
    @@ -68,10 +72,9 @@
        * @param numberOfRows   total number of rows
        */
       public UnsafeAbstractDimensionDataChunkStore(long totalSize, boolean isInvertedIdex,
    -      int numberOfRows) {
    +      int numberOfRows) throws MemoryException {
         // allocating the data page
    -    this.dataPageMemoryBlock =
    -        MemoryAllocatorFactory.INSATANCE.getMemoryAllocator().allocate(totalSize);
    +    this.dataPageMemoryBlock = UnsafeMemoryManager.allocateMemoryWithRetry(taskId, totalSize);
    --- End diff --
   
    From this class it self we can throw runtime exception if Unsafesafe memory manager not able to allocate requested memory


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1346#discussion_r138254259
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanFiterExecuterImpl.java ---
    @@ -267,11 +267,15 @@ private BitSet getFilteredIndexes(DimensionColumnDataChunk dimensionColumnDataCh
           DirectDictionaryGenerator directDictionaryGenerator = DirectDictionaryKeyGeneratorFactory
    --- End diff --
   
    Now as we are not adding one in default value, while searching for last index of null value in binary search, we need to add +1 in search result as search will be last index of null value inclusive


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1346#discussion_r138254137
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanEqualFilterExecuterImpl.java ---
    @@ -265,11 +265,15 @@ private BitSet getFilteredIndexes(DimensionColumnDataChunk dimensionColumnDataCh
           DirectDictionaryGenerator directDictionaryGenerator = DirectDictionaryKeyGeneratorFactory
    --- End diff --
   
    Now as we are not adding one in default value, while searching for last index of null value in binary search, we need to add +1 in search result as search will be last index of null value inclusive


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1346#discussion_r138338482
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/store/impl/unsafe/UnsafeAbstractDimensionDataChunkStore.java ---
    @@ -68,10 +72,9 @@
        * @param numberOfRows   total number of rows
        */
       public UnsafeAbstractDimensionDataChunkStore(long totalSize, boolean isInvertedIdex,
    -      int numberOfRows) {
    +      int numberOfRows) throws MemoryException {
         // allocating the data page
    -    this.dataPageMemoryBlock =
    -        MemoryAllocatorFactory.INSATANCE.getMemoryAllocator().allocate(totalSize);
    +    this.dataPageMemoryBlock = UnsafeMemoryManager.allocateMemoryWithRetry(taskId, totalSize);
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1346#discussion_r138339130
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanEqualFilterExecuterImpl.java ---
    @@ -265,11 +265,15 @@ private BitSet getFilteredIndexes(DimensionColumnDataChunk dimensionColumnDataCh
           DirectDictionaryGenerator directDictionaryGenerator = DirectDictionaryKeyGeneratorFactory
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1346#discussion_r138339157
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanFiterExecuterImpl.java ---
    @@ -267,11 +267,15 @@ private BitSet getFilteredIndexes(DimensionColumnDataChunk dimensionColumnDataCh
           DirectDictionaryGenerator directDictionaryGenerator = DirectDictionaryKeyGeneratorFactory
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1346
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4/



---