deniskuzZ commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r447046016



##########
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##########
@@ -229,102 +239,168 @@ public int repair(MsckInfo msckInfo) {
             throw new MetastoreException(e);
           }
         }
+        if (transactionalTable && !MetaStoreServerUtils.isPartitioned(table)) {
+          if (result.getMaxWriteId() > 0) {
+            if (txnId < 0) {
+              // We need the txnId to check against even if we didn't do the 
locking
+              txnId = getMsc().openTxn(getUserName());
+            }
+
+            validateAndAddMaxTxnIdAndWriteId(result.getMaxWriteId(), 
result.getMaxTxnId(),
+                table.getDbName(), table.getTableName(), txnId);
+          }
+        }
       }
       success = true;
     } catch (Exception e) {
       LOG.warn("Failed to run metacheck: ", e);
       success = false;
-      ret = 1;
     } finally {
-      if (msckInfo.getResFile() != null) {
-        BufferedWriter resultOut = null;
-        try {
-          Path resFile = new Path(msckInfo.getResFile());
-          FileSystem fs = resFile.getFileSystem(getConf());
-          resultOut = new BufferedWriter(new 
OutputStreamWriter(fs.create(resFile)));
-
-          boolean firstWritten = false;
-          firstWritten |= writeMsckResult(result.getTablesNotInMs(),
-            "Tables not in metastore:", resultOut, firstWritten);
-          firstWritten |= writeMsckResult(result.getTablesNotOnFs(),
-            "Tables missing on filesystem:", resultOut, firstWritten);
-          firstWritten |= writeMsckResult(result.getPartitionsNotInMs(),
-            "Partitions not in metastore:", resultOut, firstWritten);
-          firstWritten |= writeMsckResult(result.getPartitionsNotOnFs(),
-            "Partitions missing from filesystem:", resultOut, firstWritten);
-          firstWritten |= writeMsckResult(result.getExpiredPartitions(),
-            "Expired partitions (retention period: " + partitionExpirySeconds 
+ "s) :", resultOut, firstWritten);
-          // sorting to stabilize qfile output (msck_repair_drop.q)
-          Collections.sort(repairOutput);
-          for (String rout : repairOutput) {
-            if (firstWritten) {
-              resultOut.write(terminator);
-            } else {
-              firstWritten = true;
-            }
-            resultOut.write(rout);
-          }
-        } catch (IOException e) {
-          LOG.warn("Failed to save metacheck output: ", e);
-          ret = 1;
-        } finally {
-          if (resultOut != null) {
-            try {
-              resultOut.close();
-            } catch (IOException e) {
-              LOG.warn("Failed to close output file: ", e);
-              ret = 1;
-            }
-          }
+      if (result!=null) {
+        logResult(result);
+        if (msckInfo.getResFile() != null) {
+          success = writeResultToFile(msckInfo, result, repairOutput, 
partitionExpirySeconds) && success;
         }
       }
 
-      LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());
-      LOG.info("Tables missing on filesystem: {}", result.getTablesNotOnFs());
-      LOG.info("Partitions not in metastore: {}", 
result.getPartitionsNotInMs());
-      LOG.info("Partitions missing from filesystem: {}", 
result.getPartitionsNotOnFs());
-      LOG.info("Expired partitions: {}", result.getExpiredPartitions());
-      if (acquireLock && txnId > 0) {
-          if (success) {
-            try {
-              LOG.info("txnId: {} succeeded. Committing..", txnId);
-              getMsc().commitTxn(txnId);
-            } catch (Exception e) {
-              LOG.warn("Error while committing txnId: {} for table: {}", 
txnId, qualifiedTableName, e);
-              ret = 1;
-            }
-          } else {
-            try {
-              LOG.info("txnId: {} failed. Aborting..", txnId);
-              getMsc().abortTxns(Lists.newArrayList(txnId));
-            } catch (Exception e) {
-              LOG.warn("Error while aborting txnId: {} for table: {}", txnId, 
qualifiedTableName, e);
-              ret = 1;
-            }
-          }
+      if (txnId > 0) {
+        success = closeTxn(qualifiedTableName, success, txnId) && success;
       }
       if (getMsc() != null) {
         getMsc().close();
         msc = null;
       }
     }
+    return success ? 0 : 1;
+  }
 
+  private boolean closeTxn(String qualifiedTableName, boolean success, long 
txnId) {
+    boolean ret = true;
+    if (success) {
+      try {
+        LOG.info("txnId: {} succeeded. Committing..", txnId);
+        getMsc().commitTxn(txnId);
+      } catch (Exception e) {
+        LOG.warn("Error while committing txnId: {} for table: {}", txnId, 
qualifiedTableName, e);
+        ret = false;
+      }
+    } else {
+      try {
+        LOG.info("txnId: {} failed. Aborting..", txnId);
+        getMsc().abortTxns(Lists.newArrayList(txnId));
+      } catch (Exception e) {
+        LOG.warn("Error while aborting txnId: {} for table: {}", txnId, 
qualifiedTableName, e);
+        ret = false;
+      }
+    }
     return ret;
   }
 
-  private LockRequest createLockRequest(final String dbName, final String 
tableName) throws TException {
-    UserGroupInformation loggedInUser = null;
-    String username;
+  private void logResult(CheckResult result) {
+    LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());

Review comment:
       why calling log.info so many times?




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to