[GitHub] carbondata pull request #1436: [WIP][CARBONDATA-1617] Merging carbonindex fi...

[qiuchenjian-2]

GitHub user ravipesala opened a pull request:

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

    [WIP][CARBONDATA-1617] Merging carbonindex files within segment

    Be sure to do all of the following to help us incorporate your contribution
    quickly and easily:
   
     - [X ] Make sure the PR title is formatted like:
       `[CARBONDATA-<Jira issue #>] Description of pull request`
       
     - [ X] Make sure to add PR description including
           
           Problem :
    The first-time query of carbon becomes very slow. It is because of reading many small carbonindex files and cache to the driver at the first time.
    Many carbonindex files are created in below case
    Loading data in large cluster
    For example, if the cluster size is 100 nodes then for each load 100 index files are created per segment. So after 100 loads, the number of carbonindex files becomes 10000. .
    It will be slower to read all the files from the driver since a lot of namenode calls and IO operations.
    Solution :
    Merge the carbonindex files in two levels.so that we can reduce the IO calls to namenode and improves the read performance.
    Merge within a segment.
    Merge the carbonindex files to single file immediately after load completes within the segment. It would be named as a .carbonindexmerge file. It is actually not a true data merging but a simple file merge. So that the current structure of carbonindex files does not change. While reading we just read one file instead of many carbonindex files within the segment.
   
   
     - [ ] 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/ravipesala/incubator-carbondata carbon-index-merge

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

    https://github.com/apache/carbondata/pull/1436.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 #1436
   
----
commit bbabb705b8dc81a6569b66ee3bb1765de04348c7
Author: ravipesala <[hidden email]>
Date:   2017-10-25T05:43:22Z

    Merging carbonindex files within segment

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1436
 
    restest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1436#discussion_r147673058
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1376,6 +1376,13 @@
     
       public static final String BITSET_PIPE_LINE_DEFAULT = "true";
     
    +  /**
    +   * It will merge the carbon index files with in the segment to single segment.
    --- End diff --
   
    Please mention it is for test purpose only, it should not be set by user


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1436#discussion_r147673357
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datamap/dev/DataMapInfo.java ---
    @@ -0,0 +1,34 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.datamap.dev;
    +
    +/**
    + * Information required to build datamap
    + */
    +public class DataMapInfo {
    +
    +  private String filePath;
    --- End diff --
   
    Can we move it to DataMapMeta. This is another metadata only.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1436#discussion_r147673675
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java ---
    @@ -66,13 +70,17 @@ public BlockletDataMapIndexStore(String carbonStorePath, CarbonLRUCache lruCache
         segmentLockMap = new ConcurrentHashMap<String, Object>();
       }
     
    -  @Override public BlockletDataMap get(TableBlockIndexUniqueIdentifier tableSegmentUniqueIdentifier)
    +  @Override public BlockletDataMap get(TableBlockIndexUniqueIdentifier identifier)
           throws IOException {
    -    String lruCacheKey = tableSegmentUniqueIdentifier.getUniqueTableSegmentIdentifier();
    +    String lruCacheKey = identifier.getUniqueTableSegmentIdentifier();
         BlockletDataMap dataMap = (BlockletDataMap) lruCache.get(lruCacheKey);
         if (dataMap == null) {
           try {
    -        dataMap = loadAndGetDataMap(tableSegmentUniqueIdentifier);
    +        SegmentIndexFileStore indexFileStore = new SegmentIndexFileStore();
    +        indexFileStore.readAllIIndexOfSegment(CarbonTablePath.getSegmentPath(
    --- End diff --
   
    Please move `CarbonTablePath.getSegmentPath(` to next line and make indentation according to each function call


---