[GitHub] carbondata pull request #1082: [CARBONDATA-1218] In case of data-load failur...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1082
 
    Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/837/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user mohammadshahidkhan commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1082
 
    Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/871/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1082
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/914/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user mohammadshahidkhan commented on the issue:

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


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1082
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1246/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1082
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1249/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1082#discussion_r159584326
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/badrecordloger/BadRecordLoggerTest.scala ---
    @@ -247,6 +248,33 @@ class BadRecordLoggerTest extends QueryTest with BeforeAndAfterAll {
         }
       }
     
    +  test(
    +    "test if first load failed due to bad record then second load should not failed if there is " +
    +    "no bad record") {
    +    sql("drop table IF EXISTS loadIssue")
    +    sql("""CREATE TABLE IF NOT EXISTS loadIssue(ID BigInt, item String) STORED BY 'carbondata'""")
    +    CarbonProperties.getInstance().addProperty(CarbonCommonConstants.CARBON_BAD_RECORDS_ACTION,
    +      "FAIL").addProperty(CarbonCommonConstants.LOAD_SORT_SCOPE, "GLOBAL_SORT")
    +    try {
    +      sql("insert into loadIssue select 'x','Book'");
    +    } catch {
    +      case ex: Exception =>
    +        assert(true)
    +    }
    +    try {
    +      sql("insert into loadIssue select 1,'Book'");
    +    } catch {
    +      case ex: Exception =>
    +        assert(false)
    +    } finally {
    +      CarbonProperties.getInstance()
    +        .addProperty(CarbonCommonConstants.CARBON_BAD_RECORDS_ACTION, "FORCE")
    +        .addProperty(CarbonCommonConstants.LOAD_SORT_SCOPE, "LOCAL_SORT")
    +    }
    +    assert(true)
    +    sql("drop table IF EXISTS loadIssue")
    +  }
    +
    --- End diff --
   
    1. Use only one try and finally block inside test code..try should start from first line and end at last and finally should only reset the carbon property
    2. Instead of try catch use intercept[Exception] only for the 1st case where exception is expected. For 2nd case try catch is not required...if exception comes anyways test case will fail


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1082#discussion_r159586543
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -145,6 +147,15 @@ public static void renameBadRecordsFromInProgressToNormal(
         }
       }
     
    +  public static void renameBadRecord(CarbonDataLoadConfiguration configuration) {
    +    // rename the bad record in progress to normal
    +    CarbonTableIdentifier identifier =
    +        configuration.getTableIdentifier().getCarbonTableIdentifier();
    +    CarbonDataProcessorUtil.renameBadRecordsFromInProgressToNormal(configuration,
    --- End diff --
   
    1. Avoid calling the method using classname. Method is present in the same class.
    2. I think we can introduce a new class called BadRecordsUtil which will have all the functions related to bad records like
    renameBadRecord, hasBadRecord, renameBadRecordsFromInProgressToNormal (make it private in the new util class)


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1082#discussion_r159621817
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -145,6 +147,15 @@ public static void renameBadRecordsFromInProgressToNormal(
         }
       }
     
    +  public static void renameBadRecord(CarbonDataLoadConfiguration configuration) {
    +    // rename the bad record in progress to normal
    +    CarbonTableIdentifier identifier =
    +        configuration.getTableIdentifier().getCarbonTableIdentifier();
    +    CarbonDataProcessorUtil.renameBadRecordsFromInProgressToNormal(configuration,
    --- End diff --
   
    Refactored


---