[jira] [Work logged] (HIVE-25977) Enhance Compaction Cleaner to skip when there is nothing to do #2

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HIVE-25977?focusedWorklogId=735347&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-735347 ]

ASF GitHub Bot logged work on HIVE-25977:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 02/Mar/22 15:12
            Start Date: 02/Mar/22 15:12
    Worklog Time Spent: 10m 
      Work Description: klcopp commented on a change in pull request #2971:
URL: https://github.com/apache/hive/pull/2971#discussion_r817789462



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java
##########
@@ -439,29 +440,40 @@ private boolean removeFiles(String location, ValidWriteIdList writeIdList, Compa
     return success;
   }
 
-  private boolean hasDataBelowWatermark(FileSystem fs, Path path, long highWatermark) throws IOException {
-    FileStatus[] children = fs.listStatus(path);
+  private boolean hasDataBelowWatermark(AcidDirectory acidDir, FileSystem fs, Path path, long highWatermark,
+      long minOpenTxn)
+      throws IOException {
+    Set<Path> acidPaths = new HashSet<>();
+    for (ParsedDelta delta : acidDir.getCurrentDirectories()) {
+      acidPaths.add(delta.getPath());
+    }
+    if (acidDir.getBaseDirectory() != null) {
+      acidPaths.add(acidDir.getBaseDirectory());
+    }
+    FileStatus[] children = fs.listStatus(path, p -> {
+      return !acidPaths.contains(p);
+    });
     for (FileStatus child : children) {
-      if (isFileBelowWatermark(child, highWatermark)) {
+      if (isFileBelowWatermark(child, highWatermark, minOpenTxn)) {
         return true;
       }
     }
     return false;
   }
 
-  private boolean isFileBelowWatermark(FileStatus child, long highWatermark) {
+  private boolean isFileBelowWatermark(FileStatus child, long highWatermark, long minOpenTxn) {
     Path p = child.getPath();
     String fn = p.getName();
     if (!child.isDirectory()) {
-      return false;
+      return true;
     }
     if (fn.startsWith(AcidUtils.BASE_PREFIX)) {
       ParsedBaseLight b = ParsedBaseLight.parseBase(p);
-      return b.getWriteId() < highWatermark;
+      return b.getWriteId() <= highWatermark && b.getVisibilityTxnId() <= minOpenTxn;

Review comment:
       I can't remember if we discussed this. What if the table has files:
   ```
   base_2_v5
   delta_3_3
   delta_4_4
   base_4_v16
   ``` (this compaction)
   
   hwm=4
   
   Say the worker is older and did not populate CQ_NEXT_TXN_ID, so the cleaner will pick up any compaction in "ready for cleaning"
   Say minTxnId=13
   The AcidDirectory will contain:
   base_2_v5
   delta_3_3
   delta_4_4
   
   So isFileBelowWatermark will investigate:
   base_4_v16
   And return false because 4=b.getWriteId() <= highWatermark=4 is true and 16=b.getVisibilityTxnId() <= minOpenTxn=13 is false.
   
   
   Also I probably have amnesia but can you remind me of the rationale behind `b.getVisibilityTxnId() <= minOpenTxn` here?
   Also keep in mind that the visibilityTxnId is always 0 unless the delta/base is a product of compaction.




-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 735347)
    Time Spent: 0.5h  (was: 20m)

> Enhance Compaction Cleaner to skip when there is nothing to do #2
> -----------------------------------------------------------------
>
>                 Key: HIVE-25977
>                 URL: https://issues.apache.org/jira/browse/HIVE-25977
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Zoltan Haindrich
>            Assignee: Zoltan Haindrich
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> initially this was just an addendum to the original patch ; but got delayed and altered - so it should have its own ticket



--
This message was sent by Atlassian Jira
(v8.20.1#820001)