milleruntime commented on a change in pull request #1646:
URL: https://github.com/apache/accumulo/pull/1646#discussion_r448403131
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactor.java
##########
@@ -148,6 +149,13 @@ public CompactionStats call() {
reportedProblem = true;
} catch (CompactionCanceledException e) {
throw new IllegalStateException(e);
+ } catch (Throwable t) {
+ // This is an unexpected situation which is critical. If we cannot
minor compact
+ // then this tablet will eventually become unusable if we are still
ingesting data.
+ log.error("MinC failed ({}). Aborting.", t.getMessage(), t);
+ // TODO: Should we simply unload this tablet instead?
+ Halt.halt("Unexpected MinC failure: " + t.getMessage(), 2);
+ throw t;
Review comment:
Following the stack further up, we are already catching Throwable [here
in
MinorCompactionTask](https://github.com/apache/accumulo/blob/94844b3ef88bc9e02b5cb5c8eeff81411ac8221d/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java#L111).
So if we decide to do anything to the tablet, I think that would be the best
place since that method is already operating on the tablet object. The finally
in that try will set the ```minorCompactionState``` so one option would be to
add another state to the [enum in
Tablet](https://github.com/apache/accumulo/blob/15a95feebc74bd10e5ba73d4c2ecfc726435bc55/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java#L217)
to indicate failed state. I don't know what all this state is used for but it
may be less complicated than unloading the Tablet and less problematic than
halting the tserver.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
