keith-turner commented on a change in pull request #2235:
URL: https://github.com/apache/accumulo/pull/2235#discussion_r700595271
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java
##########
@@ -1388,11 +1388,12 @@ public synchronized void close() {
closed = true;
- // wait while internal jobs are running or external compactions are
committing, but do not
- // wait on external compactions that are running
+ // wait while internal jobs are running, external compactions are
committing or the status of
+ // chops is active, but do not wait on external compactions that are
running
while (runningJobs.stream()
.anyMatch(job -> !((CompactionExecutorIdImpl)
job.getExecutor()).isExternalId())
- || !externalCompactionsCommitting.isEmpty()) {
+ || !externalCompactionsCommitting.isEmpty()
+ || fileMgr.chopStatus != FileSelectionStatus.NOT_ACTIVE) {
Review comment:
I think in the current code when a chop is actually marking the state
will actually be NOT_ACTIVE. Thinking we should introduce a new explicit state
for MARKING and then we could do something like the following.
```suggestion
|| fileMgr.chopStatus == FileSelectionStatus.MARKING) {
```
To go with the above change could do something like the following in
checkIfChopComplete()
```java
private void checkifChopComplete(Set<StoredTabletFile> allFiles) {
boolean completed = false;
synchronized (this) {
if (closed) {
// if closed do not attempt transition to marking state
return;
}
// when this returns true it means we transitioned to the MARKING state
completed = fileMgr.finishChop(allFiles);
}
if (completed) {
try {
markChopped();
} finally {
synchronized (this) {
// this will transition from MARKING to NOT_ACTIVE
fileMgr.finishMarkingChop();
this.notifyAll();
}
}
TabletLogger.selected(getExtent(), CompactionKind.CHOP, Set.of());
}
}
```
In a
[branch](https://github.com/keith-turner/accumulo/commits/accumulo-2199) I
created for #2199 I have created a separate enum for chop status since chop
only uses a subset of the file status states. Adding an extra state that is
only used for chop would make this separate enum even more useful. For this PR
could add the new state to the existing enum and I can fix it up later.
--
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]
