[GitHub] carbondata pull request #1851: [WIP]Added block size to DataMap while intial...

[qiuchenjian-2]

GitHub user ravipesala opened a pull request:

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

    [WIP]Added block size to DataMap while intializing

    Added block size to blocklet datamap so that features like small file merge can use them
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [X] Any interfaces changed?
     
     - [X] Any backward compatibility impacted?
     
     - [X] Document update required?
   
     - [X] Testing done
                 
     - [X] 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/ravipesala/incubator-carbondata add-block-size

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

    https://github.com/apache/carbondata/pull/1851.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 #1851
   
----
commit 17c3c106a046e3eeaffd05dc96b7983b1337b675
Author: ravipesala <ravi.pesala@...>
Date:   2018-01-23T13:01:54Z

    Added block size to DataMap while intializing

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/3065/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1833/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1838/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3054/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3059/



---

[qiuchenjian-2]

Github user QiangCai commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    verified on local standalone mode
   
    LGTM


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1851#discussion_r163449480
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---
    @@ -78,17 +78,19 @@ public BlockletDataMap get(TableBlockIndexUniqueIdentifier identifier)
             String segmentPath = CarbonTablePath.getSegmentPath(
                 identifier.getAbsoluteTableIdentifier().getTablePath(),
                 identifier.getSegmentId());
    -        Map<String, String[]> locationMap = new HashMap<>();
    +        Map<String, BlockMetaInfo> blockMetaInfoMap = new HashMap<>();
             CarbonFile carbonFile = FileFactory.getCarbonFile(segmentPath);
             CarbonFile[] carbonFiles = carbonFile.locationAwareListFiles();
             SegmentIndexFileStore indexFileStore = new SegmentIndexFileStore();
             indexFileStore.readAllIIndexOfSegment(carbonFiles);
             PartitionMapFileStore partitionFileStore = new PartitionMapFileStore();
             partitionFileStore.readAllPartitionsOfSegment(carbonFiles, segmentPath);
             for (CarbonFile file : carbonFiles) {
    -          locationMap.put(file.getAbsolutePath(), file.getLocations());
    +          blockMetaInfoMap
    +              .put(file.getAbsolutePath(), new BlockMetaInfo(file.getLocations(), file.getSize()));
    --- End diff --
   
    move `.put` to previous line


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1851#discussion_r163449568
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---
    @@ -244,4 +247,22 @@ public void clearAccessCount(
           cacheable.clear();
         }
       }
    +
    +  public static class BlockMetaInfo {
    --- End diff --
   
    Is it only used in this class? I think it is better to move to outside if not just used in this class


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1851#discussion_r163449593
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---
    @@ -244,4 +247,22 @@ public void clearAccessCount(
           cacheable.clear();
         }
       }
    +
    +  public static class BlockMetaInfo {
    +    private String[] locationInfo;
    +    private long size;
    --- End diff --
   
    please add comment for these two variable


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1851#discussion_r163450757
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---
    @@ -244,4 +247,22 @@ public void clearAccessCount(
           cacheable.clear();
         }
       }
    +
    +  public static class BlockMetaInfo {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1851#discussion_r163450767
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---
    @@ -244,4 +247,22 @@ public void clearAccessCount(
           cacheable.clear();
         }
       }
    +
    +  public static class BlockMetaInfo {
    +    private String[] locationInfo;
    +    private long size;
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/3074/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3064/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1843/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/3078/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1847/



---

[qiuchenjian-2]

Github user QiangCai commented on the issue:

    https://github.com/apache/carbondata/pull/1851
 
    verified CI  successfully on local machine


---