dlmarion commented on code in PR #5395:
URL: https://github.com/apache/accumulo/pull/5395#discussion_r1991431321
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java:
##########
@@ -1340,8 +1391,11 @@ public void compact(CompactionServiceId service,
CompactionJob job, BooleanSuppl
SortedMap<StoredTabletFile,DataFileValue> allFiles =
tablet.getDatafiles();
HashMap<StoredTabletFile,DataFileValue> compactFiles = new HashMap<>();
cInfo.jobFiles.forEach(file -> compactFiles.put(file,
allFiles.get(file)));
-
- stats = CompactableUtils.compact(tablet, job, cInfo, compactEnv,
compactFiles, tmpFileName);
+ // Limit interrupting compactions to the part that reads and writes
files and avoid
+ // interrupting the metadata table updates.
+ try (var ignored = new CompactionInterrupter()) {
+ stats = CompactableUtils.compact(tablet, job, cInfo, compactEnv,
compactFiles, tmpFileName);
Review Comment:
I'm trying to understand how this PR is different than what is happening
inside CompactableUtils.compact at:
https://github.com/apache/accumulo/blob/29506999476e99ffc15019469c35705ced0d3278/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java#L573-L579
When `CompactableImpl.close` is called and sets `closed=true`, then
`cenv.isCompactionEnabled()` should return `false` and interrupt the
FileCompactor. I'm thinking the difference here is checking the
`Thread.interrupted` state and throwing an `InterruptedException` in
`CompactionInterrupter.close`. I'm wondering if the code below is functionally
equivalent:
```
try {
stats = CompactableUtils.compact(tablet, job, cInfo, compactEnv,
compactFiles, tmpFileName);
} finally {
if (Thread.interrupted()) {
throw new InterruptedException();
}
}
```
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
