This is an automated email from the ASF dual-hosted git repository. dlmarion pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new bbd87a6 Closes #1689: Don't catch Throwable (unless it's rethrown) (#1840) bbd87a6 is described below commit bbd87a6693fc5cdbbe947b0821a4f05a18cf905b Author: Dave Marion <dlmar...@apache.org> AuthorDate: Tue Jan 19 08:07:07 2021 -0500 Closes #1689: Don't catch Throwable (unless it's rethrown) (#1840) Changed catch clauses from catching Throwable to catching Exception, unless Throwable was being re-thrown. --- .../accumulo/core/clientImpl/TableOperationsImpl.java | 8 ++++---- .../core/clientImpl/TabletServerBatchReaderIterator.java | 7 +++++-- .../core/clientImpl/TabletServerBatchWriter.java | 14 +++++++------- .../java/org/apache/accumulo/fate/zookeeper/ZooLock.java | 6 +++--- .../main/java/org/apache/accumulo/server/ServerUtil.java | 2 +- .../org/apache/accumulo/server/client/BulkImporter.java | 2 +- .../accumulo/server/zookeeper/DistributedWorkQueue.java | 2 +- .../org/apache/accumulo/gc/SimpleGarbageCollector.java | 2 +- .../src/main/java/org/apache/accumulo/master/Master.java | 6 +++--- .../org/apache/accumulo/master/TabletGroupWatcher.java | 2 +- .../main/java/org/apache/accumulo/monitor/Monitor.java | 4 ++-- .../org/apache/accumulo/tserver/AssignmentHandler.java | 2 +- .../java/org/apache/accumulo/tserver/InMemoryMap.java | 2 +- .../java/org/apache/accumulo/tserver/TabletServer.java | 4 ++-- .../accumulo/tserver/TabletServerResourceManager.java | 4 ++-- .../org/apache/accumulo/tserver/ThriftClientHandler.java | 10 +++++----- .../org/apache/accumulo/tserver/UnloadTabletHandler.java | 2 +- .../accumulo/tserver/constraints/ConstraintChecker.java | 16 ++++++---------- .../java/org/apache/accumulo/tserver/log/LogSorter.java | 4 ++-- .../apache/accumulo/tserver/log/RecoveryLogReader.java | 2 +- .../apache/accumulo/tserver/log/TabletServerLogger.java | 8 ++++---- .../org/apache/accumulo/tserver/scan/LookupTask.java | 2 +- .../org/apache/accumulo/tserver/scan/NextBatchTask.java | 12 ++++-------- .../java/org/apache/accumulo/tserver/scan/ScanTask.java | 3 +++ .../org/apache/accumulo/tserver/tablet/Compactor.java | 6 +++--- .../accumulo/tserver/tablet/MinorCompactionTask.java | 6 +++--- .../java/org/apache/accumulo/tserver/tablet/Tablet.java | 6 +++--- start/src/main/java/org/apache/accumulo/start/Main.java | 2 +- .../start/classloader/vfs/AccumuloVFSClassLoader.java | 2 +- .../apache/accumulo/test/fate/zookeeper/ZooLockIT.java | 2 +- .../apache/accumulo/test/functional/SplitRecoveryIT.java | 2 +- .../apache/accumulo/test/functional/ZombieTServer.java | 2 +- 32 files changed, 76 insertions(+), 78 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index db3b0d1..6cb8eb4 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -417,10 +417,10 @@ public class TableOperationsImpl extends TableOperationsHelper { private TableId tableId; private ExecutorService executor; private CountDownLatch latch; - private AtomicReference<Throwable> exception; + private AtomicReference<Exception> exception; SplitEnv(String tableName, TableId tableId, ExecutorService executor, CountDownLatch latch, - AtomicReference<Throwable> exception) { + AtomicReference<Exception> exception) { this.tableName = tableName; this.tableId = tableId; this.executor = executor; @@ -462,7 +462,7 @@ public class TableOperationsImpl extends TableOperationsHelper { env.executor.execute(new SplitTask(env, splits.subList(0, mid))); env.executor.execute(new SplitTask(env, splits.subList(mid + 1, splits.size()))); - } catch (Throwable t) { + } catch (Exception t) { env.exception.compareAndSet(null, t); } } @@ -480,7 +480,7 @@ public class TableOperationsImpl extends TableOperationsHelper { Collections.sort(splits); CountDownLatch latch = new CountDownLatch(splits.size()); - AtomicReference<Throwable> exception = new AtomicReference<>(null); + AtomicReference<Exception> exception = new AtomicReference<>(null); ExecutorService executor = ThreadPools.createFixedThreadPool(16, "addSplits", false); try { diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java index ce900bb..e45e474 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java @@ -389,12 +389,15 @@ public class TabletServerBatchReaderIterator implements Iterator<Entry<Key,Value fatalException = new TableDeletedException(tableId.canonical()); } catch (SampleNotPresentException e) { fatalException = e; - } catch (Throwable t) { + } catch (Exception t) { if (queryThreadPool.isShutdown()) log.debug("Caught exception, but queryThreadPool is shutdown", t); else log.warn("Caught exception, but queryThreadPool is not shutdown", t); fatalException = t; + } catch (Throwable t) { + fatalException = t; + throw t; // let uncaught exception handler deal with the Error } finally { semaphore.release(); Thread.currentThread().setName(threadName); @@ -411,7 +414,7 @@ public class TabletServerBatchReaderIterator implements Iterator<Entry<Key,Value e.setTableInfo(getTableInfo()); log.debug("{}", e.getMessage(), e); fatalException = e; - } catch (Throwable t) { + } catch (Exception t) { log.debug("{}", t.getMessage(), t); fatalException = t; } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java index 83f5d98..f90cfca 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java @@ -92,7 +92,7 @@ import com.google.common.base.Joiner; * + Flush holds adding of new mutations so it does not wait indefinitely * * Considerations - * + All background threads must catch and note Throwable + * + All background threads must catch and note Exception * + mutations for a single tablet server are only processed by one thread * concurrently (if new mutations come in for a tablet server while one * thread is processing mutations for it, no other thread should @@ -155,7 +155,7 @@ public class TabletServerBatchWriter implements AutoCloseable { private final FailedMutations failedMutations; private int unknownErrors = 0; private boolean somethingFailed = false; - private Throwable lastUnknownError = null; + private Exception lastUnknownError = null; private static class TimeoutTracker { @@ -218,8 +218,8 @@ public class TabletServerBatchWriter implements AutoCloseable { > TabletServerBatchWriter.this.maxLatency) startProcessing(); } - } catch (Throwable t) { - updateUnknownErrors("Max latency task failed " + t.getMessage(), t); + } catch (Exception e) { + updateUnknownErrors("Max latency task failed " + e.getMessage(), e); } }), 0, this.maxLatency / 4, TimeUnit.MILLISECONDS); } @@ -524,7 +524,7 @@ public class TabletServerBatchWriter implements AutoCloseable { log.error("Server side error on {}", server, e); } - private synchronized void updateUnknownErrors(String msg, Throwable t) { + private synchronized void updateUnknownErrors(String msg, Exception t) { somethingFailed = true; unknownErrors++; this.lastUnknownError = t; @@ -620,7 +620,7 @@ public class TabletServerBatchWriter implements AutoCloseable { rf.size()); addFailedMutations(rf); } - } catch (Throwable t) { + } catch (Exception t) { updateUnknownErrors("tid=" + Thread.currentThread().getId() + " Failed to requeue failed mutations " + t.getMessage(), t); executor.remove(task); @@ -804,7 +804,7 @@ public class TabletServerBatchWriter implements AutoCloseable { } return; - } catch (Throwable t) { + } catch (Exception t) { updateUnknownErrors( "Failed to send tablet server " + location + " its batch : " + t.getMessage(), t); } diff --git a/core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java b/core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java index 872196c..54b6eb3 100644 --- a/core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java +++ b/core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java @@ -52,7 +52,7 @@ public class ZooLock implements Watcher { /** * lost the ability to monitor the lock node, and its status is unknown */ - void unableToMonitorLockNode(Throwable e); + void unableToMonitorLockNode(Exception e); } public interface AsyncLockWatcher extends LockWatcher { @@ -114,7 +114,7 @@ public class ZooLock implements Watcher { } @Override - public void unableToMonitorLockNode(Throwable e) { + public void unableToMonitorLockNode(Exception e) { lw.unableToMonitorLockNode(e); } @@ -288,7 +288,7 @@ public class ZooLock implements Watcher { else if (asyncLock != null) failedToAcquireLock(); } - } catch (Throwable e) { + } catch (Exception e) { lockWatcher.unableToMonitorLockNode(e); log.error("Failed to stat lock node " + asyncLockPath, e); } diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java b/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java index 9485744..00c53c4 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java @@ -186,7 +186,7 @@ public class ServerUtil { } } } - } catch (Throwable t) { + } catch (Exception t) { log.error("", t); } }, 1000, 10 * 60 * 1000, TimeUnit.MILLISECONDS); diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java b/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java index a23b186..df6d5a3 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java @@ -593,7 +593,7 @@ public class BulkImporter { } } catch (ThriftSecurityException e) { throw new AccumuloSecurityException(e.user, e.code, e); - } catch (Throwable t) { + } catch (Exception t) { log.error("Encountered unknown exception in assignMapFiles.", t); throw new AccumuloException(t); } diff --git a/server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java b/server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java index be1e7bb..38e4df6 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java +++ b/server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java @@ -152,7 +152,7 @@ public class DistributedWorkQueue { threadPool.execute(task); } - } catch (Throwable t) { + } catch (Exception t) { log.error("Unexpected error", t); } } diff --git a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java index 738b252..b7a8fc8 100644 --- a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java +++ b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java @@ -625,7 +625,7 @@ public class SimpleGarbageCollector extends AbstractServer implements Iface { } @Override - public void unableToMonitorLockNode(final Throwable e) { + public void unableToMonitorLockNode(final Exception e) { // ACCUMULO-3651 Level changed to error and FATAL added to message for slf4j compatibility Halt.halt(-1, () -> log.error("FATAL: No longer able to monitor lock node ", e)); diff --git a/server/manager/src/main/java/org/apache/accumulo/master/Master.java b/server/manager/src/main/java/org/apache/accumulo/master/Master.java index f04fbcf..3a9d6ac 100644 --- a/server/manager/src/main/java/org/apache/accumulo/master/Master.java +++ b/server/manager/src/main/java/org/apache/accumulo/master/Master.java @@ -781,7 +781,7 @@ public class Master extends AbstractServer break; } } - } catch (Throwable t) { + } catch (Exception t) { log.error("Error occurred reading / switching master goal state. Will" + " continue with attempt to update status", t); } @@ -789,7 +789,7 @@ public class Master extends AbstractServer try { wait = updateStatus(); eventListener.waitForEvents(wait); - } catch (Throwable t) { + } catch (Exception t) { log.error("Error balancing tablets, will wait for {} (seconds) and then retry ", WAIT_BETWEEN_ERRORS / ONE_SECOND, t); sleepUninterruptibly(WAIT_BETWEEN_ERRORS, TimeUnit.MILLISECONDS); @@ -1354,7 +1354,7 @@ public class Master extends AbstractServer } @Override - public void unableToMonitorLockNode(final Throwable e) { + public void unableToMonitorLockNode(final Exception e) { // ACCUMULO-3651 Changed level to error and added FATAL to message for slf4j compatibility Halt.halt(-1, () -> log.error("FATAL: No longer able to monitor master lock node", e)); diff --git a/server/manager/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java b/server/manager/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java index be8dbc0..450b623 100644 --- a/server/manager/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java +++ b/server/manager/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java @@ -493,7 +493,7 @@ abstract class TabletGroupWatcher extends Thread { Master.log.error( "Metadata table is inconsistent at {} and all assigned/future tservers are still online.", row); - } catch (Throwable e) { + } catch (Exception e) { Master.log.error("Error attempting repair of metadata " + row + ": " + e, e); } } diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java index 0e625da..df60ab1 100644 --- a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java +++ b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java @@ -423,7 +423,7 @@ public class Monitor extends AbstractServer implements HighlyAvailableService { server.addServlet(getViewServlet(), "/*"); server.start(); break; - } catch (Throwable ex) { + } catch (Exception ex) { log.error("Unable to start embedded web server", ex); } } @@ -666,7 +666,7 @@ public class Monitor extends AbstractServer implements HighlyAvailableService { } @Override - public void unableToMonitorLockNode(final Throwable e) { + public void unableToMonitorLockNode(final Exception e) { Halt.halt(-1, () -> log.error("No longer able to monitor Monitor lock node", e)); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java index 3bb08d4..66f09f1 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java @@ -188,7 +188,7 @@ class AssignmentHandler implements Runnable { } tablet = null; // release this reference successful = true; - } catch (Throwable e) { + } catch (Exception e) { log.warn("exception trying to assign tablet {} {}", extent, locationToOpen, e); if (e.getMessage() != null) { diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java index f1dddd6..075a96f 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java @@ -159,7 +159,7 @@ public class InMemoryMap { if (useNativeMap && NativeMap.isLoaded()) { try { return new NativeMapWrapper(); - } catch (Throwable t) { + } catch (Exception t) { log.error("Failed to create native map", t); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java index 7899165..bd0ec23 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java @@ -446,7 +446,7 @@ public class TabletServer extends AbstractServer { tablet.checkIfMinorCompactionNeededForLogs(closedCopy); } - } catch (Throwable t) { + } catch (Exception t) { log.error("Unexpected exception in {}", Thread.currentThread().getName(), t); sleepUninterruptibly(1, TimeUnit.SECONDS); } @@ -654,7 +654,7 @@ public class TabletServer extends AbstractServer { } @Override - public void unableToMonitorLockNode(final Throwable e) { + public void unableToMonitorLockNode(final Exception e) { Halt.halt(1, () -> log.error("Lost ability to monitor tablet server lock, exiting.", e)); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java index 2db0e39..f011ea5 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java @@ -541,7 +541,7 @@ public class TabletServerResourceManager { ArrayList<TabletMemoryReport> tabletStates = new ArrayList<>(tabletReportsCopy.values()); tabletsToMinorCompact = memoryManager.tabletsToMinorCompact(tabletStates); - } catch (Throwable t) { + } catch (Exception t) { log.error("Memory manager failed {}", t.getMessage(), t); } @@ -581,7 +581,7 @@ public class TabletServerResourceManager { // log.debug("mma.tabletsToMinorCompact = "+mma.tabletsToMinorCompact); } - } catch (Throwable t) { + } catch (Exception t) { log.error("Minor compactions for memory management failed", t); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java index 26a4cab..228ec83 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java @@ -401,7 +401,7 @@ class ThriftClientHandler extends ClientServiceHandler implements TabletClientSe long timeout = server.getConfiguration().getTimeInMillis(Property.TSERV_CLIENT_TIMEOUT); server.sessionManager.removeIfNotAccessed(scanID, timeout); return new ScanResult(param, true); - } catch (Throwable t) { + } catch (Exception t) { server.sessionManager.removeSession(scanID); log.warn("Failed to get next batch", t); throw new RuntimeException(t); @@ -569,7 +569,7 @@ class ThriftClientHandler extends ClientServiceHandler implements TabletClientSe Map<TKeyExtent,List<TRange>> failures = Collections.emptyMap(); List<TKeyExtent> fullScans = Collections.emptyList(); return new MultiScanResult(results, failures, fullScans, null, null, false, true); - } catch (Throwable t) { + } catch (Exception t) { server.sessionManager.removeSession(scanID); log.warn("Failed to get multiscan result", t); throw new RuntimeException(t); @@ -769,7 +769,7 @@ class ThriftClientHandler extends ClientServiceHandler implements TabletClientSe mutationCount += mutations.size(); } - } catch (Throwable t) { + } catch (Exception t) { error = t; log.error("Unexpected error preparing for commit", error); break; @@ -800,7 +800,7 @@ class ThriftClientHandler extends ClientServiceHandler implements TabletClientSe break; } catch (IOException | FSError ex) { log.warn("logging mutations failed, retrying"); - } catch (Throwable t) { + } catch (Exception t) { log.error("Unknown exception logging mutations, counts" + " for mutations in flight not decremented!", t); throw new RuntimeException(t); @@ -1100,7 +1100,7 @@ class ThriftClientHandler extends ClientServiceHandler implements TabletClientSe break; } catch (IOException | FSError ex) { log.warn("logging mutations failed, retrying"); - } catch (Throwable t) { + } catch (Exception t) { log.error("Unknown exception logging mutations, counts for" + " mutations in flight not decremented!", t); throw new RuntimeException(t); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/UnloadTabletHandler.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/UnloadTabletHandler.java index b3798a9..f77bbfe 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/UnloadTabletHandler.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/UnloadTabletHandler.java @@ -89,7 +89,7 @@ class UnloadTabletHandler implements Runnable { try { t.close(!goalState.equals(TUnloadTabletGoal.DELETED)); - } catch (Throwable e) { + } catch (Exception e) { if ((t.isClosing() || t.isClosed()) && e instanceof IllegalStateException) { log.debug("Failed to unload tablet {}... it was already closing or closed : {}", extent, diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/ConstraintChecker.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/ConstraintChecker.java index 51e4cc6..773f525 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/ConstraintChecker.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/ConstraintChecker.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.tserver.constraints; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map.Entry; @@ -62,7 +61,7 @@ public class ConstraintChecker { } } - } catch (Throwable e) { + } catch (Exception e) { constrains.clear(); constrains.add(new UnsatisfiableConstraint((short) -1, "Failed to load constraints, not accepting mutations.")); @@ -108,25 +107,22 @@ public class ConstraintChecker { constraint.getViolationDescription(vcode), 1)); } } - } catch (Throwable throwable) { - log.warn("CONSTRAINT FAILED : {}", throwable.getMessage(), throwable); + } catch (Exception e) { + log.warn("CONSTRAINT FAILED : {}", e.getMessage(), e); // constraint failed in some way, do not allow mutation to pass short vcode; String msg; - if (throwable instanceof NullPointerException) { + if (e instanceof NullPointerException) { vcode = -1; msg = "threw NullPointerException"; - } else if (throwable instanceof ArrayIndexOutOfBoundsException) { + } else if (e instanceof ArrayIndexOutOfBoundsException) { vcode = -2; msg = "threw ArrayIndexOutOfBoundsException"; - } else if (throwable instanceof NumberFormatException) { + } else if (e instanceof NumberFormatException) { vcode = -3; msg = "threw NumberFormatException"; - } else if (throwable instanceof IOException) { - vcode = -4; - msg = "threw IOException (or subclass of)"; } else { vcode = -100; msg = "threw some Exception"; diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java index 97de8ea..9155e89 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java @@ -152,7 +152,7 @@ public class LogSorter { fs.create(new Path(destPath, "finished")).close(); log.info("Finished log sort {} {} bytes {} parts in {}ms", name, getBytesCopied(), part, getSortTime()); - } catch (Throwable t) { + } catch (Exception t) { try { // parent dir may not exist fs.mkdirs(new Path(destPath)); @@ -160,7 +160,7 @@ public class LogSorter { } catch (IOException e) { log.error("Error creating failed flag file " + name, e); } - log.error("Caught throwable", t); + log.error("Caught exception", t); } finally { Thread.currentThread().setName(formerThreadName); try { diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java index 041f103..4afa85d 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java @@ -67,7 +67,7 @@ public class RecoveryLogReader implements CloseableIterator<Entry<LogFileKey,Log private static Object create(java.lang.Class<?> klass) { try { return klass.getConstructor().newInstance(); - } catch (Throwable t) { + } catch (Exception t) { throw new RuntimeException("Unable to construct objects to use for comparison"); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java index 739d5ba..0c4e1c2 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java @@ -363,15 +363,15 @@ public class TabletServerLogger { currentLog.close(); } catch (DfsLogger.LogClosedException ex) { // ignore - } catch (Throwable ex) { + } catch (Exception ex) { log.error("Unable to cleanly close log " + currentLog.getFileName() + ": " + ex, ex); } finally { this.tserver.walogClosed(currentLog); + currentLog = null; + logSizeEstimate.set(0); } - currentLog = null; - logSizeEstimate.set(0); } - } catch (Throwable t) { + } catch (Exception t) { throw new IOException(t); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/LookupTask.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/LookupTask.java index 9a2a129..a3c1a34 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/LookupTask.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/LookupTask.java @@ -177,7 +177,7 @@ public class LookupTask extends ScanTask<MultiScanResult> { } } catch (SampleNotPresentException e) { addResult(e); - } catch (Throwable e) { + } catch (Exception e) { log.warn("exception while doing multi-scan ", e); addResult(e); } finally { diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java index e3a6de4..a950290 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java @@ -18,11 +18,11 @@ */ package org.apache.accumulo.tserver.scan; +import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.accumulo.core.client.SampleNotPresentException; import org.apache.accumulo.core.iterators.IterationInterruptedException; -import org.apache.accumulo.core.util.Halt; import org.apache.accumulo.tserver.TabletServer; import org.apache.accumulo.tserver.TooManyFilesException; import org.apache.accumulo.tserver.session.SingleScanSession; @@ -89,13 +89,9 @@ public class NextBatchTask extends ScanTask<ScanBatch> { } } catch (TooManyFilesException | SampleNotPresentException e) { addResult(e); - } catch (OutOfMemoryError ome) { - Halt.halt("Ran out of memory scanning " + scanSession.extent + " for " + scanSession.client, - 1); - addResult(ome); - } catch (Throwable e) { - log.warn("exception while scanning tablet " - + (scanSession == null ? "(unknown)" : scanSession.extent), e); + } catch (IOException | RuntimeException e) { + log.warn("exception while scanning tablet {} for {}", scanSession.extent, scanSession.client, + e); addResult(e); } finally { runState.set(ScanRunState.FINISHED); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java index ea17bd2..dc28214 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/scan/ScanTask.java @@ -127,6 +127,9 @@ public abstract class ScanTask<T> implements RunnableFuture<T> { // returned resultQueue = null; + if (r instanceof Error) + throw (Error) r; // don't wrap an Error + if (r instanceof Throwable) throw new ExecutionException((Throwable) r); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java index 0da9213..58c2e98 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java @@ -309,7 +309,7 @@ public class Compactor implements Callable<CompactionStats> { iters.add(iter); - } catch (Throwable e) { + } catch (Exception e) { ProblemReports.getInstance(context).report( new ProblemReport(extent.tableId(), ProblemType.FILE_READ, mapFile.getPathStr(), e)); @@ -319,7 +319,7 @@ public class Compactor implements Callable<CompactionStats> { for (FileSKVIterator reader : readers) { try { reader.close(); - } catch (Throwable e2) { + } catch (Exception e2) { log.warn("Failed to close map file", e2); } } @@ -413,7 +413,7 @@ public class Compactor implements Callable<CompactionStats> { for (FileSKVIterator reader : readers) { try { reader.close(); - } catch (Throwable e) { + } catch (Exception e) { log.warn("Failed to close map file", e); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java index 83d060f..5a2243b 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java @@ -111,9 +111,9 @@ class MinorCompactionTask implements Runnable { if (tablet.needsSplit()) { tablet.getTabletServer().executeSplit(tablet); } - } catch (Throwable t) { - log.error("Unknown error during minor compaction for extent: " + tablet.getExtent(), t); - throw new RuntimeException(t); + } catch (Exception e) { + log.error("Unknown error during minor compaction for extent: {}", tablet.getExtent(), e); + throw e; } finally { tablet.minorCompactionComplete(); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java index d235171..cf08e26 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java @@ -387,7 +387,7 @@ public class Tablet { } } - } catch (Throwable t) { + } catch (Exception t) { String msg = "Error recovering tablet " + extent + " from log files"; if (tableConfiguration.getBoolean(Property.TABLE_FAILURES_IGNORE)) { log.warn(msg, t); @@ -795,7 +795,7 @@ public class Tablet { Thread.currentThread().setName(oldName); try { getTabletMemory().finalizeMinC(); - } catch (Throwable t) { + } catch (Exception t) { log.error("Failed to free tablet memory on {}", extent, t); } @@ -1298,7 +1298,7 @@ public class Tablet { try { getTabletMemory().getMemTable().delete(0); - } catch (Throwable t) { + } catch (Exception t) { log.error("Failed to delete mem table : " + t.getMessage() + " for tablet " + extent, t); } diff --git a/start/src/main/java/org/apache/accumulo/start/Main.java b/start/src/main/java/org/apache/accumulo/start/Main.java index 0dcdbec..bec8070 100644 --- a/start/src/main/java/org/apache/accumulo/start/Main.java +++ b/start/src/main/java/org/apache/accumulo/start/Main.java @@ -148,7 +148,7 @@ public class Main { Method main = null; try { main = classWithMain.getMethod("main", args.getClass()); - } catch (Throwable t) { + } catch (Exception t) { log.error("Could not run main method on '" + classWithMain.getName() + "'.", t); } if (main == null || !Modifier.isPublic(main.getModifiers()) diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java index 4a1390a..00c6ec1 100644 --- a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java +++ b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java @@ -391,7 +391,7 @@ public class AccumuloVFSClassLoader { } } out.print("\n"); - } catch (Throwable t) { + } catch (Exception t) { throw new RuntimeException(t); } } diff --git a/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/ZooLockIT.java b/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/ZooLockIT.java index 4060e10..d3cb5e3 100644 --- a/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/ZooLockIT.java +++ b/test/src/main/java/org/apache/accumulo/test/fate/zookeeper/ZooLockIT.java @@ -105,7 +105,7 @@ public class ZooLockIT extends SharedMiniClusterBase { } @Override - public synchronized void unableToMonitorLockNode(Throwable e) { + public synchronized void unableToMonitorLockNode(Exception e) { changes++; this.notifyAll(); } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java b/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java index de0120f..a43f2ba 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java @@ -108,7 +108,7 @@ public class SplitRecoveryIT extends ConfigurableMacBase { @SuppressFBWarnings(value = "DM_EXIT", justification = "System.exit() is a bad idea here, but okay for now, since it's a test") @Override - public void unableToMonitorLockNode(Throwable e) { + public void unableToMonitorLockNode(Exception e) { System.exit(-1); } }, "foo".getBytes(UTF_8)); diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java b/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java index f91937d..658b721 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java @@ -138,7 +138,7 @@ public class ZombieTServer { @SuppressFBWarnings(value = "DM_EXIT", justification = "System.exit() is a bad idea here, but okay for now, since it's a test") @Override - public void unableToMonitorLockNode(Throwable e) { + public void unableToMonitorLockNode(Exception e) { try { tch.halt(TraceUtil.traceInfo(), null, null); } catch (Exception ex) {