[GitHub] carbondata pull request #1734: [CARBONDATA-1912] Handling lock issues for al...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1734
 
    @ManoharVanam...what is need of releasing the locks in catch block when finally block assures that lock is released?


---

[qiuchenjian-2]

Github user ManoharVanam commented on the issue:

    https://github.com/apache/carbondata/pull/1734
 
    @manishgupta88  If operation is success, unlock will try to delete lock file from old table ( which doesn't exists) so error will be printed on spark sql console. So release old table lock only in case of exception.


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1734
 
    @ManoharVanam ...In that case can we change the logger error to warning in case of non existence of file and check whether still the warning log is getting printed. If warning logs are not getting printed in the console then we can make change in the logic instead changing the logic


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1734#discussion_r159394213
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala ---
    @@ -156,18 +156,21 @@ private[sql] case class CarbonAlterTableRenameCommand(
         } catch {
           case e: Exception =>
             LOGGER.error(e, "Rename table failed: " + e.getMessage)
    -        if (carbonTable != null) {
    -          AlterTableUtil
    -            .revertRenameTableChanges(oldTableIdentifier,
    +        try {
    --- End diff --
   
    Still it is unknown, when exception is raised before rename or after rename.
    So better handle unlocking at common place and logic and change table path if rename is success.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1734#discussion_r159605348
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala ---
    @@ -156,18 +156,21 @@ private[sql] case class CarbonAlterTableRenameCommand(
         } catch {
           case e: Exception =>
             LOGGER.error(e, "Rename table failed: " + e.getMessage)
    -        if (carbonTable != null) {
    -          AlterTableUtil
    -            .revertRenameTableChanges(oldTableIdentifier,
    +        try {
    --- End diff --
   
    done


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1734
 
    LGTM....will merge after all builds run


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1734
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---