ctubbsii commented on code in PR #5726:
URL: https://github.com/apache/accumulo/pull/5726#discussion_r2198841529
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -560,9 +582,15 @@ private void compactLocalityGroup(String lgName,
Set<ByteSequence> columnFamilie
SystemIteratorEnvironment iterEnv =
env.createIteratorEnv(context, acuTableConf, getExtent().tableId());
- SortedKeyValueIterator<Key,Value> itr =
iterEnv.getTopLevelIterator(IteratorConfigUtil
- .convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf,
iterators, iterEnv));
+ SortedKeyValueIterator<Key,Value> stack = null;
+ try {
+ stack = IteratorConfigUtil.convertItersAndLoad(env.getIteratorScope(),
cfsi, acuTableConf,
+ iterators, iterEnv);
+ } catch (RuntimeException e) {
+ throw new CompactionClassLoadingException("Error loading iterators",
e);
+ }
Review Comment:
I think that the underlying problem is that the original exception was
wrapped in a RTE. I don't think wrapping the RTE in yet another exception is
the right solution. I think the problem was the original wrapping, instead of
allowing it to pass through. I think it'd be better not to wrap the original
exception in a RTE in the first place.
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -97,6 +97,27 @@ public static class CompactionCanceledException extends
Exception {
private static final long serialVersionUID = 1L;
}
+ public static class CompactionClassLoadingException extends Exception {
Review Comment:
This is a very narrow exception that seems a bit more granular than what is
needed here. I think we need to handle a wider variety of problems initiating a
compaction.
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -560,9 +582,15 @@ private void compactLocalityGroup(String lgName,
Set<ByteSequence> columnFamilie
SystemIteratorEnvironment iterEnv =
env.createIteratorEnv(context, acuTableConf, getExtent().tableId());
- SortedKeyValueIterator<Key,Value> itr =
iterEnv.getTopLevelIterator(IteratorConfigUtil
- .convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf,
iterators, iterEnv));
+ SortedKeyValueIterator<Key,Value> stack = null;
+ try {
+ stack = IteratorConfigUtil.convertItersAndLoad(env.getIteratorScope(),
cfsi, acuTableConf,
Review Comment:
I don't think we should kill compactors when there is a bad compaction
configuration. That compaction should certainly be aborted, though. I think
that in general, the service should remain available for future compactions, if
at all possible.
--
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]
