[GitHub] [carbondata] akashrn5 opened a new pull request #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table

[GitBox]

akashrn5 opened a new pull request #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548
 
 
    ### Why is this PR needed?
    the code to clean up the stale datamap folders during session initialization causes problem, if some other user tries to access the datamap table, we might get permission exception.
   
    ### What changes were proposed in this PR?
   We need to clean up only if the table does not exists in hive metastore, but the schema exists, we get exception incase the table exists but other user, so in such case, no need to go ahead, we can just catch exception and go ahead. Same changes are proposed here.
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - No (Not required as tested in cluster and fix is for clean up issue)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-569682663
 
 
   Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1360/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-569694886
 
 
   Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1370/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-569723185
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1381/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

kunal642 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570121640
 
 
   LGTM

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

MakKind commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570218679
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

MakKind commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570220449
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

MakKind removed a comment on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570220449
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

MakKind removed a comment on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570218679
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

MakKind commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570222595
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

MakKind removed a comment on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570222595
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

akashrn5 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570504339
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570515558
 
 
   Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1422/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570541723
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1442/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570542887
 
 
   Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1430/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

akashrn5 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570565475
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570585082
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1447/
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#discussion_r363073041
 
 

 ##########
 File path: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonEnv.scala
 ##########
 @@ -135,17 +135,25 @@ class CarbonEnv {
       dataMapSchema =>
         if (null != dataMapSchema.getRelationIdentifier &&
             !dataMapSchema.isIndexDataMap) {
-          if (!sparkSession.sessionState
-            .catalog
-            .tableExists(TableIdentifier(dataMapSchema.getRelationIdentifier.getTableName,
-              Some(dataMapSchema.getRelationIdentifier.getDatabaseName)))) {
+          val isTableExists = try {
+            sparkSession.sessionState
+              .catalog
+              .tableExists(TableIdentifier(dataMapSchema.getRelationIdentifier.getTableName,
+                Some(dataMapSchema.getRelationIdentifier.getDatabaseName)))
+          } catch {
+            // we need to take care of cleanup when the table does not exists, if table exists and
+            // some other user tries to access the table, it might fail, that time no need to handle
+            case ex: Exception =>
 
 Review comment:
   Should catch NoSuchTableException?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

akashrn5 commented on a change in pull request #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#discussion_r363078785
 
 

 ##########
 File path: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonEnv.scala
 ##########
 @@ -135,17 +135,25 @@ class CarbonEnv {
       dataMapSchema =>
         if (null != dataMapSchema.getRelationIdentifier &&
             !dataMapSchema.isIndexDataMap) {
-          if (!sparkSession.sessionState
-            .catalog
-            .tableExists(TableIdentifier(dataMapSchema.getRelationIdentifier.getTableName,
-              Some(dataMapSchema.getRelationIdentifier.getDatabaseName)))) {
+          val isTableExists = try {
+            sparkSession.sessionState
+              .catalog
+              .tableExists(TableIdentifier(dataMapSchema.getRelationIdentifier.getTableName,
+                Some(dataMapSchema.getRelationIdentifier.getDatabaseName)))
+          } catch {
+            // we need to take care of cleanup when the table does not exists, if table exists and
+            // some other user tries to access the table, it might fail, that time no need to handle
+            case ex: Exception =>
 
 Review comment:
   no, basically this will happen only when the table not exists in hive metastore and schema is present in carbon store. So we just need to proceed only when the tableexists is false, else for anything its fine.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on issue #3548: [CARBONDATA-3600]Fix the cleanup failure issue if user fails to access table
URL: https://github.com/apache/carbondata/pull/3548#issuecomment-570899969
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services