kuczoram commented on code in PR #6498:
URL: https://github.com/apache/hive/pull/6498#discussion_r3309632718


##########
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/handler/CompactionCleaner.java:
##########
@@ -99,6 +101,18 @@ private void clean(CompactionInfo ci, long minOpenTxn, 
boolean metricsEnabled) t
     LOG.info("Starting cleaning for {}, based on min open {}", ci,
         (ci.minOpenWriteId > 0) ? "writeId: " + ci.minOpenWriteId : "txnId: " 
+ minOpenTxn);
 
+    if (ci.nextTxnId == 0 && ci.txnId > 0 &&
+        (ci.type == CompactionType.MAJOR || ci.type == CompactionType.MINOR || 
ci.type == CompactionType.REBALANCE)) {
+      TxnStatus status = txnHandler.getTransactionStatus(ci.txnId);

Review Comment:
   > > With min.history.level, a compaction like this can block the cleaning 
for all consecutive compaction, even after the txn is aborted.
   > 
   > if i get it right, it's not the case with `min.history.writeid`, however, 
the check is generic
   
   No, with min.history.writeid it is not blocking the following cleaners. 
Because in that case what happens is that the cleaner's highwatermark will be 
0, so it won't delete anything, but there is no remainder checking for 
min.history.writeid, so it will consider the cleaning as successful. Marks the 
compaction as successful and just goes on. With min.history.level, it will 
clean nothing, but finds deltas which should be deleted, so it will stay in the 
queue with "ready for cleaning" state.



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