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: [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]
