[GitHub] carbondata pull request #2482: [CARBONDATA-2714] Support merge index files f...

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2482#discussion_r202298843
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/events/AlterTableMergeIndexEventListener.scala ---
    @@ -0,0 +1,95 @@
    +/*
    + * 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.spark.sql.events
    +
    +import java.util
    +
    +import scala.collection.JavaConverters._
    +import scala.collection.mutable
    +
    +import org.apache.spark.internal.Logging
    +import org.apache.spark.sql.util.CarbonException
    +
    +import org.apache.carbondata.common.logging.{LogService, LogServiceFactory}
    +import org.apache.carbondata.core.datamap.Segment
    +import org.apache.carbondata.core.locks.{CarbonLockFactory, LockUsage}
    +import org.apache.carbondata.core.statusmanager.SegmentStatusManager
    +import org.apache.carbondata.events.{AlterTableMergeIndexEvent, Event, OperationContext, OperationEventListener}
    +import org.apache.carbondata.processing.merger.CarbonDataMergerUtil
    +import org.apache.carbondata.spark.util.CommonUtil
    +
    +class AlterTableMergeIndexEventListener extends OperationEventListener with Logging {
    --- End diff --
   
    This listener also merge with `MergeIndexEventListener` as another `case `


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2482#discussion_r202301535
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java ---
    @@ -488,6 +485,49 @@ private void readIndexFiles(SegmentStatus status, boolean ignoreStatus) throws I
         }
       }
     
    +  /**
    +   * Reads all merge index / index files as per the status of the file.
    +   * In case of @ignoreStatus is true it just reads all merge index / index files
    +   *
    +   * @param status
    +   * @param ignoreStatus
    +   * @return
    +   * @throws IOException
    +   */
    +  private List<String> readIndexOrMergeFiles(SegmentStatus status, boolean ignoreStatus)
    --- End diff --
   
    why do you need this method, Already merge files are available in `SegmentIndexFileStore.getCarbonMergeFileToIndexFilesMap`


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

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


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/2482
 
    @dhatchayani  can you provide the detail, this pr including both(level1 and level2) ?   as per the mailing list discussion : http://apache-carbondata-dev-mailing-list-archive.1130556.n5.nabble.com/Discussion-Merging-carbonindex-files-for-each-segments-and-across-segments-td24441.html



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2482#discussion_r202602660
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1871,6 +1868,16 @@
        */
       public static final String CACHE_LEVEL_DEFAULT_VALUE = "BLOCK";
     
    +  /**
    +   * It is internal configuration and used only for test purpose.
    --- End diff --
   
    Internal ?  then for users, how to enable and disable this feature ?  


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2482#discussion_r202608397
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1871,6 +1868,16 @@
        */
       public static final String CACHE_LEVEL_DEFAULT_VALUE = "BLOCK";
     
    +  /**
    +   * It is internal configuration and used only for test purpose.
    --- End diff --
   
    We are not giving this to user. For user, this will be always TRUE. User should not change it.


---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/2482
 
    @chenliang613 We are supporting only level 1 (within segment) as discussed in the community. level 2 (across segments) is not supported as it will be already taken care through compaction


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---