[GitHub] carbondata pull request #2996: [WIP] Fix Rename-Fail & Datamap-creation-Fail

[qiuchenjian-2]

GitHub user NamanRastogi opened a pull request:

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

    [WIP] Fix Rename-Fail & Datamap-creation-Fail

    Fixed negative scenarios:
    1. Alter Table Rename Table Fail
    2. Create Preagregate-Datamap Fail
   
    ### Alter Table Rename Table Fail
    * Changed <code>CarbonAlterTableRenameCommand</code>
    * When tabe rename is success in hive, for failed in carbon data store, it would throw exception, but would not go back and undo rename in hive.
   
    **Solution**: A flag to keep check if hive rename has already executed, and of the code breaks after hive rename is done, go back and undo the hive rename.
   
    ### Create-Preagregate-Datamap Fail
    * Changed <code>CarbonDropDataMapCommand</code>
    * When (preaggregate) datamap schema is written, but table updation is failed
        -> call CarbonDropDataMapCommand.processMetadata()
        -> call dropDataMapFromSystemFolder() -> this is  supposed to delete the folder on disk, but doesnt as the datamap is not yet updated in table, and throws NoSuchDataMapException
   
    **Solution**: Call CarbonDropTableCommand.run() instead of CarbonDropTableCommand.processDatamap(). As CarbonDropTableCommand.processData() deletes actual folders from disk.
           
   
     - [x] Any interfaces changed?   --->   No
     - [ ] Any backward compatibility impacted?
     - [x] Document update required?   --->   No
     - [ ] Testing done
     - [ ] 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/NamanRastogi/carbondata hdfs_quota_fix

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

    https://github.com/apache/carbondata/pull/2996.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 #2996
   
----
commit c67b61eef376d9218d5b67f1946cef5406fb3712
Author: namanrastogi <naman.rastogi.52@...>
Date:   2018-12-13T10:39:58Z

    Fixed Alter Table Rename

commit 4b97c01045431e25f8b5bbe11f9cc49e7bda23ec
Author: namanrastogi <naman.rastogi.52@...>
Date:   2018-12-18T06:21:02Z

    Fixed no-deletion of datamap folder, when table updation is failed

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1822/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10080/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2031/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2996#discussion_r242847958
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala ---
    @@ -165,15 +167,22 @@ private[sql] case class CarbonAlterTableRenameCommand(
           case e: ConcurrentOperationException =>
             throw e
           case e: Exception =>
    +        if (hiveRenameSuccess) {
    +          sparkSession.sessionState.catalog.asInstanceOf[CarbonSessionCatalog].alterTableRename(
    --- End diff --
   
    If rename to old table name is fail, how should this scene be handled?


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1963/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10215/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2236/



---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    @NamanRastogi Please fixed the build failure


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2996#discussion_r244906489
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala ---
    @@ -165,15 +167,22 @@ private[sql] case class CarbonAlterTableRenameCommand(
           case e: ConcurrentOperationException =>
             throw e
           case e: Exception =>
    +        if (hiveRenameSuccess) {
    +          sparkSession.sessionState.catalog.asInstanceOf[CarbonSessionCatalog].alterTableRename(
    +            TableIdentifier(newTableName, Some(oldDatabaseName)),
    +            TableIdentifier(oldTableName, Some(oldDatabaseName)),
    --- End diff --
   
    Can we reuse `oldTableIdentifier` and `newTableIdentifier` in `alterTableRenameModel` ?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2996#discussion_r244906592
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala ---
    @@ -165,15 +167,22 @@ private[sql] case class CarbonAlterTableRenameCommand(
           case e: ConcurrentOperationException =>
             throw e
           case e: Exception =>
    +        if (hiveRenameSuccess) {
    +          sparkSession.sessionState.catalog.asInstanceOf[CarbonSessionCatalog].alterTableRename(
    --- End diff --
   
    This is a revert operation when L139-142(hive update) is success but L145-146(carbon meta store update) is failed


---

[qiuchenjian-2]

Github user Indhumathi27 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2184/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2400/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10440/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2190/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2406/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10446/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2996#discussion_r245669672
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala ---
    @@ -165,15 +167,22 @@ private[sql] case class CarbonAlterTableRenameCommand(
           case e: ConcurrentOperationException =>
             throw e
           case e: Exception =>
    +        if (hiveRenameSuccess) {
    +          sparkSession.sessionState.catalog.asInstanceOf[CarbonSessionCatalog].alterTableRename(
    +            TableIdentifier(newTableName, Some(oldDatabaseName)),
    +            TableIdentifier(oldTableName, Some(oldDatabaseName)),
    --- End diff --
   
    Done


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2996
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2200/



---