[GitHub] carbondata pull request #1573: [CARBONDATA-1815] Add AtomicRunnableCommand a...

[qiuchenjian-2]

GitHub user jackylk opened a pull request:

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

    [CARBONDATA-1815] Add AtomicRunnableCommand abstraction

    Some CarbonData command need to process both metadata and data which should be in an atomic fashion. These commands need to support undo if any failure. So this PR adds AtomicRunnableCommand to enforce this behavior.
   
     - [X] Any interfaces changed?
     No
     - [X] Any backward compatibility impacted?
     No
     - [X] Document update required?
    No
     - [X] Testing done
    Retest all testcases
     - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA

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

    $ git pull https://github.com/jackylk/incubator-carbondata undo

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

    https://github.com/apache/carbondata/pull/1573.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 #1573
   
----
commit c2e7843e65a4f68b8e6f657bf89d90ea7965438e
Author: Jacky Li <[hidden email]>
Date:   2017-11-27T08:48:09Z

    add AtomicRunnableCommand

commit b6939d0708ddbf65e7f4d29679dec4a5ee3b6bf4
Author: Jacky Li <[hidden email]>
Date:   2017-11-27T08:57:56Z

    fix comment

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1573#discussion_r153162075
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonEnv.scala ---
    @@ -138,4 +141,35 @@ object CarbonEnv {
           .asInstanceOf[CarbonRelation]
           .carbonTable
       }
    +
    +  def getTablePath(
    +      databaseNameOp: Option[String],
    +      tableName: String
    +  )(sparkSession: SparkSession): String = {
    +    val dbLocation = GetDB.getDatabaseLocation(
    +      databaseNameOp.getOrElse(sparkSession.sessionState.catalog.getCurrentDatabase),
    +      sparkSession,
    +      CarbonProperties.getStorePath)
    +    dbLocation + CarbonCommonConstants.FILE_SEPARATOR + tableName
    +  }
    +
    +  def getDatabaseName(
    +      databaseNameOp: Option[String]
    +  )(sparkSession: SparkSession): String = {
    +    databaseNameOp.getOrElse(sparkSession.sessionState.catalog.getCurrentDatabase)
    +  }
    +  
    +  def getMetastorePath(
    +      databaseNameOp: Option[String],
    +      tableName: String
    +  )(sparkSession: SparkSession): String = {
    +    val absoluteTableIdentifier = AbsoluteTableIdentifier.from(
    +      getTablePath(databaseNameOp, tableName)(sparkSession),
    +      getDatabaseName(databaseNameOp)(sparkSession),
    +      tableName)
    +    val carbonTablePath = CarbonStorePath
    +      .getCarbonTablePath(absoluteTableIdentifier)
    --- End diff --
   
    move to last row


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1573#discussion_r153166206
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/execution/command/CarbonHiveCommands.scala ---
    @@ -51,7 +51,7 @@ case class CarbonDropDatabaseCommand(command: DropDatabaseCommand)
         // DropHiveDB command will fail if cascade is false and one or more table exists in database
         if (command.cascade && tablesInDB != null) {
           tablesInDB.foreach { tableName =>
    -        CarbonDropTableCommand(true, tableName.database, tableName.table).run(sparkSession)
    +          CarbonDropTableCommand(true, tableName.database, tableName.table).run(sparkSession)
    --- End diff --
   
    not required


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1573#discussion_r153166422
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala ---
    @@ -150,7 +149,7 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
         DROP ~> DATAMAP ~> opt(IF ~> EXISTS) ~ ident ~ (ON ~ TABLE) ~
         (ident <~ ".").? ~ ident <~ opt(";")  ^^ {
           case ifexists ~ dmname ~ ontable ~ dbName ~ tableName =>
    -        CarbonDropDataMapCommand(dmname, ifexists.isDefined, dbName, tableName)
    +      CarbonDropDataMapCommand(dmname, ifexists.isDefined, dbName, tableName)
    --- End diff --
   
    not required


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1573
 
    Build Failed with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/389/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---