aturoczy commented on code in PR #4410:
URL: https://github.com/apache/hive/pull/4410#discussion_r1227686607


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -694,21 +694,21 @@ public void markCleaned(CompactionInfo info) throws 
MetaException {
           pStmt.setLong(1, info.id);
           LOG.debug("Going to execute update <{}> for CQ_ID={}", s, info.id);
           pStmt.executeUpdate();
+        }
 
-          s = "DELETE FROM \"COMPACTION_QUEUE\" WHERE \"CQ_ID\" = ?";
-          pStmt = dbConn.prepareStatement(s);
-          pStmt.setLong(1, info.id);
-          LOG.debug("Going to execute update <{}>", s);
-          int updCount = pStmt.executeUpdate();
-          if (updCount != 1) {
-            LOG.error("Unable to delete compaction record: {}.  Update 
count={}", info, updCount);
-            LOG.debug("Going to rollback");
-            dbConn.rollback();
-          }
+        /* Remove compaction queue record corresponding to the compaction 
which has been successful as well as
+         * remove all abort retry associated metadata of table/partition in 
the COMPACTION_QUEUE both when compaction
+         * or abort cleanup is successful. We don't want a situation wherein 
we have an abort retry entry for a table
+         * but no corresponding entry in TXN_COMPONENTS table. Successful 
compaction will delete
+         * the retry metadata, so that abort cleanup is retried again (an 
optimistic retry approach).
+         */
+        removeCompactionAndAbortRetryEntries(dbConn, info);
+
+        if (!info.isAbortedTxnCleanup()) {
           // Remove entries from completed_txn_components as well, so we don't 
start looking there
           // again but only up to the highest write ID include in this 
compaction job.
           //highestWriteId will be NULL in upgrade scenarios
-          s = "DELETE FROM \"COMPLETED_TXN_COMPONENTS\" WHERE \"CTC_DATABASE\" 
= ? AND \"CTC_TABLE\" = ?";
+          String s = "DELETE FROM \"COMPLETED_TXN_COMPONENTS\" WHERE 
\"CTC_DATABASE\" = ? AND \"CTC_TABLE\" = ?";

Review Comment:
   Could you please choose different variable name then s? I think it is more 
expressive it is sqlCommand or command



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java:
##########
@@ -1295,25 +1288,33 @@ private static boolean timedOut(CompactionInfo ci, 
RetentionCounters rc, long pa
             && (rc.hasSucceededMajorCompaction || 
(rc.hasSucceededMinorCompaction && ci.type == CompactionType.MINOR));
   }
 
-  private void removeAbortRetryEntries(Connection dbConn, CompactionInfo info) 
throws MetaException, RetryException {
-    LOG.debug("Going to execute update <{}>", 
DELETE_ABORT_RETRY_ENTRIES_FROM_COMPACTION_QUEUE);
-    try (PreparedStatement pStmt = 
dbConn.prepareStatement(DELETE_ABORT_RETRY_ENTRIES_FROM_COMPACTION_QUEUE)) {
+  private void removeCompactionAndAbortRetryEntries(Connection dbConn, 
CompactionInfo info) throws MetaException, RetryException {
+    String query = 
String.format(DELETE_COMPACTION_AND_ABORT_RETRY_ENTRIES_FROM_COMPACTION_QUEUE,
+            info.partName != null ? " = ? " : " IS NULL ", info.id > 0 ? " OR 
\"CQ_ID\" = ?" : "");

Review Comment:
   This expression would kill the brain to unit test :)
   
![image](https://github.com/apache/hive/assets/7687174/dfa436b4-1860-4107-8e01-b4475159bc76)
   



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to