From 99c807092c6932e276cc82c61875e92e656967d4 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 20 Aug 2025 20:49:02 +0000 Subject: [PATCH 1/9] Added ExitCodesIT and related changes --- .../core/clientImpl/ClientContext.java | 17 +- .../core/lock/ServiceLockSupport.java | 10 +- pom.xml | 5 + .../accumulo/server/AbstractServer.java | 21 +- .../apache/accumulo/server/ServerContext.java | 3 + .../apache/accumulo/compactor/Compactor.java | 404 +++++++++--------- .../accumulo/gc/SimpleGarbageCollector.java | 18 +- .../org/apache/accumulo/manager/Manager.java | 12 +- .../accumulo/manager/TabletGroupWatcher.java | 4 +- .../apache/accumulo/tserver/ScanServer.java | 15 +- .../apache/accumulo/tserver/TabletServer.java | 12 +- test/pom.xml | 4 + .../functional/ExitCodesIT_SimpleSuite.java | 329 ++++++++++++++ 13 files changed, 606 insertions(+), 248 deletions(-) create mode 100644 test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java index 79ac3440a46..8592b3bfdf1 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java @@ -918,21 +918,26 @@ public AuthenticationToken token() { @Override public synchronized void close() { if (closed.compareAndSet(false, true)) { - if (zooCacheCreated.get()) { - zooCache.get().close(); - } - if (zooKeeperOpened.get()) { - zooSession.get().close(); - } if (thriftTransportPool != null) { + log.info("Closing Thrift Transport Pool"); thriftTransportPool.shutdown(); } if (scannerReadaheadPool != null) { + log.info("Closing Scanner ReadAhead Pool"); scannerReadaheadPool.shutdownNow(); // abort all tasks, client is shutting down } if (cleanupThreadPool != null) { + log.info("Closing Cleanup ThreadPool"); cleanupThreadPool.shutdown(); // wait for shutdown tasks to execute } + if (zooCacheCreated.get()) { + log.info("Closing ZooCache"); + zooCache.get().close(); + } + if (zooKeeperOpened.get()) { + log.info("Closing ZooSession"); + zooSession.get().close(); + } } } diff --git a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java index 19be1b6f4fc..fad535fefa5 100644 --- a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java +++ b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java @@ -82,13 +82,13 @@ public void lostLock(LockLossReason reason) { Halt.halt(0, server + " lock in zookeeper lost (reason = " + reason + "), exiting cleanly because shutdown is complete."); } else { - Halt.halt(-1, server + " lock in zookeeper lost (reason = " + reason + "), exiting!"); + Halt.halt(1, server + " lock in zookeeper lost (reason = " + reason + "), exiting!"); } } @Override public void unableToMonitorLockNode(final Exception e) { - Halt.halt(-1, "FATAL: No longer able to monitor " + server + " lock node", e); + Halt.halt(1, "FATAL: No longer able to monitor " + server + " lock node", e); } @Override @@ -96,7 +96,7 @@ public synchronized void acquiredLock() { LOG.debug("Acquired {} lock", server); if (acquiredLock || failedToAcquireLock) { - Halt.halt(-1, "Zoolock in unexpected state AL " + acquiredLock + " " + failedToAcquireLock); + Halt.halt(1, "Zoolock in unexpected state AL " + acquiredLock + " " + failedToAcquireLock); } acquiredLock = true; @@ -111,11 +111,11 @@ public synchronized void failedToAcquireLock(Exception e) { String msg = "Failed to acquire " + server + " lock due to incorrect ZooKeeper authentication."; LOG.error("{} Ensure instance.secret is consistent across Accumulo configuration", msg, e); - Halt.halt(-1, msg); + Halt.halt(1, msg); } if (acquiredLock) { - Halt.halt(-1, + Halt.halt(1, "Zoolock in unexpected state acquiredLock true with FAL " + failedToAcquireLock); } diff --git a/pom.xml b/pom.xml index e28ee610455..eb31d294cc7 100644 --- a/pom.xml +++ b/pom.xml @@ -328,6 +328,11 @@ commons-validator 1.10.0 + + net.bytebuddy + byte-buddy + 1.17.6 + org.apache.accumulo accumulo-access diff --git a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java index 93cdf7a3af3..daea8f92e94 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java @@ -112,6 +112,7 @@ public static void startServer(AbstractServer server, Logger LOG) throws Excepti private volatile Thread verificationThread; private final AtomicBoolean shutdownRequested = new AtomicBoolean(false); private final AtomicBoolean shutdownComplete = new AtomicBoolean(false); + private final AtomicBoolean closed = new AtomicBoolean(false); protected AbstractServer(ServerId.Type serverType, ConfigOpts opts, Function serverContextFactory, String[] args) { @@ -203,7 +204,8 @@ protected AbstractServer(ServerId.Type serverType, ConfigOpts opts, * * @param isIdle whether the server is idle */ - protected void updateIdleStatus(boolean isIdle) { + // public for ExitCodesIT + public void updateIdleStatus(boolean isIdle) { boolean shouldResetIdlePeriod = !isIdle || idleReportingPeriodMillis == 0; boolean hasIdlePeriodStarted = idlePeriodTimer != null; boolean hasExceededIdlePeriod = @@ -331,7 +333,8 @@ public String getBindAddress() { return bindAddress; } - protected TServer getThriftServer() { + // public for ExitCodesIT + public TServer getThriftServer() { if (thriftServer == null) { return null; } @@ -476,8 +479,15 @@ public void startServiceLockVerificationThread() { @Override public void close() { - if (context != null) { - context.close(); + // close is called from the subclasses run method + // when shutting down. Close is also called after + // the runServer method completes in the startServer + // method in this class. Calling twice could raise + // an exception when in reality everything was fine + if (closed.compareAndSet(false, true)) { + if (context != null) { + context.close(); + } } } @@ -488,4 +498,7 @@ protected void waitForUpgrade() throws InterruptedException { } } + public void requestShutdownForTests() { + shutdownRequested.set(true); + } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java index 4363648ff54..021db43d14e 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java @@ -522,9 +522,11 @@ public void close() { getMetricsInfo().close(); } if (sharedSchedExecutorCreated.get()) { + log.info("Shutting down shared executor pool"); getScheduledExecutor().shutdownNow(); } if (sharedMetadataWriterCreated.get()) { + log.info("Shutting down shared metadata conditional writer"); try { ConditionalWriter writer = sharedMetadataWriter.get(); if (writer != null) { @@ -535,6 +537,7 @@ public void close() { } } if (sharedUserWriterCreated.get()) { + log.info("Shutting down shared user conditional writer"); try { ConditionalWriter writer = sharedUserWriter.get(); if (writer != null) { diff --git a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java index 64bf2b0200e..9a679081d03 100644 --- a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java +++ b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java @@ -829,237 +829,247 @@ public void run() { getConfiguration().getTimeInMillis(Property.COMPACTOR_CANCEL_CHECK_INTERVAL)); LOG.info("Compactor started, waiting for work"); - try { - - final AtomicReference err = new AtomicReference<>(); - final LogSorter logSorter = new LogSorter(this); - long nextSortLogsCheckTime = System.currentTimeMillis(); - while (!isShutdownRequested()) { - if (Thread.currentThread().isInterrupted()) { - LOG.info("Server process thread has been interrupted, shutting down"); - break; - } - try { - // mark compactor as idle while not in the compaction loop - updateIdleStatus(true); + final AtomicReference err = new AtomicReference<>(); + final LogSorter logSorter = new LogSorter(this); + long nextSortLogsCheckTime = System.currentTimeMillis(); - currentCompactionId.set(null); - err.set(null); - JOB_HOLDER.reset(); - - if (System.currentTimeMillis() > nextSortLogsCheckTime) { - // Attempt to process all existing log sorting work serially in this thread. - // When no work remains, this call will return so that we can look for compaction - // work. - LOG.debug("Checking to see if any recovery logs need sorting"); + while (!isShutdownRequested()) { + if (Thread.currentThread().isInterrupted()) { + LOG.info("Server process thread has been interrupted, shutting down"); + break; + } + try { + // mark compactor as idle while not in the compaction loop + updateIdleStatus(true); + + currentCompactionId.set(null); + err.set(null); + JOB_HOLDER.reset(); + + if (System.currentTimeMillis() > nextSortLogsCheckTime) { + // Attempt to process all existing log sorting work serially in this thread. + // When no work remains, this call will return so that we can look for compaction + // work. + LOG.debug("Checking to see if any recovery logs need sorting"); + try { nextSortLogsCheckTime = logSorter.sortLogsIfNeeded(); + } catch (KeeperException e) { + LOG.error("Error sorting logs", e); } + } - performFailureProcessing(errorHistory); + performFailureProcessing(errorHistory); - TExternalCompactionJob job; - try { - TNextCompactionJob next = getNextJob(getNextId()); - job = next.getJob(); - if (!job.isSetExternalCompactionId()) { - LOG.trace("No external compactions in queue {}", this.getResourceGroup()); - UtilWaitThread.sleep(getWaitTimeBetweenCompactionChecks(next.getCompactorCount())); - continue; - } - if (!job.getExternalCompactionId().equals(currentCompactionId.get().toString())) { - throw new IllegalStateException("Returned eci " + job.getExternalCompactionId() - + " does not match supplied eci " + currentCompactionId.get()); - } - } catch (RetriesExceededException e2) { - LOG.warn("Retries exceeded getting next job. Retrying..."); + TExternalCompactionJob job; + try { + TNextCompactionJob next = getNextJob(getNextId()); + job = next.getJob(); + if (!job.isSetExternalCompactionId()) { + LOG.trace("No external compactions in queue {}", this.getResourceGroup()); + UtilWaitThread.sleep(getWaitTimeBetweenCompactionChecks(next.getCompactorCount())); continue; } - LOG.debug("Received next compaction job: {}", job); + if (!job.getExternalCompactionId().equals(currentCompactionId.get().toString())) { + throw new IllegalStateException("Returned eci " + job.getExternalCompactionId() + + " does not match supplied eci " + currentCompactionId.get()); + } + } catch (RetriesExceededException e2) { + LOG.warn("Retries exceeded getting next job. Retrying..."); + continue; + } + LOG.debug("Received next compaction job: {}", job); - final LongAdder totalInputEntries = new LongAdder(); - final LongAdder totalInputBytes = new LongAdder(); - final CountDownLatch started = new CountDownLatch(1); - final CountDownLatch stopped = new CountDownLatch(1); + final LongAdder totalInputEntries = new LongAdder(); + final LongAdder totalInputBytes = new LongAdder(); + final CountDownLatch started = new CountDownLatch(1); + final CountDownLatch stopped = new CountDownLatch(1); - final FileCompactorRunnable fcr = - createCompactionJob(job, totalInputEntries, totalInputBytes, started, stopped, err); + final FileCompactorRunnable fcr = + createCompactionJob(job, totalInputEntries, totalInputBytes, started, stopped, err); - final Thread compactionThread = Threads.createNonCriticalThread( - "Compaction job for tablet " + job.getExtent().toString(), fcr); + final Thread compactionThread = Threads.createNonCriticalThread( + "Compaction job for tablet " + job.getExtent().toString(), fcr); - JOB_HOLDER.set(job, compactionThread, fcr.getFileCompactor()); + JOB_HOLDER.set(job, compactionThread, fcr.getFileCompactor()); - try { - // mark compactor as busy while compacting - updateIdleStatus(false); + try { + // mark compactor as busy while compacting + updateIdleStatus(false); + try { // Need to call FileCompactorRunnable.initialize after calling JOB_HOLDER.set fcr.initialize(); - - compactionThread.start(); // start the compactionThread - started.await(); // wait until the compactor is started - final long inputEntries = totalInputEntries.sum(); - final long waitTime = calculateProgressCheckTime(totalInputBytes.sum()); - LOG.debug("Progress checks will occur every {} seconds", waitTime); - String percentComplete = "unknown"; - - while (!stopped.await(waitTime, TimeUnit.SECONDS)) { - List running = - org.apache.accumulo.server.compaction.FileCompactor.getRunningCompactions(); - if (!running.isEmpty()) { - // Compaction has started. There should only be one in the list - CompactionInfo info = running.get(0); - if (info != null) { - final long entriesRead = info.getEntriesRead(); - final long entriesWritten = info.getEntriesWritten(); - if (inputEntries > 0) { - percentComplete = Float.toString((entriesRead / (float) inputEntries) * 100); - } - String message = String.format( - "Compaction in progress, read %d of %d input entries ( %s %s ), written %d entries", - entriesRead, inputEntries, percentComplete, "%", entriesWritten); - watcher.run(); - try { - LOG.debug("Updating coordinator with compaction progress: {}.", message); - TCompactionStatusUpdate update = new TCompactionStatusUpdate( - TCompactionState.IN_PROGRESS, message, inputEntries, entriesRead, - entriesWritten, fcr.getCompactionAge().toNanos()); - updateCompactionState(job, update); - } catch (RetriesExceededException e) { - LOG.warn("Error updating coordinator with compaction progress, error: {}", - e.getMessage()); - } - } - } else { - LOG.debug("Waiting on compaction thread to finish, but no RUNNING compaction"); - } - } - compactionThread.join(); - LOG.trace("Compaction thread finished."); - // Run the watcher again to clear out the finished compaction and set the - // stuck count to zero. - watcher.run(); - - if (err.get() != null) { - // maybe the error occured because the table was deleted or something like that, so - // force a cancel check to possibly reduce noise in the logs - checkIfCanceled(); + } catch (RetriesExceededException e) { + LOG.error( + "Error starting FileCompactableRunnable, cancelling compaction and moving to next job.", + e); + try { + cancel(job.getExternalCompactionId()); + } catch (TException e1) { + LOG.error("Error cancelling compaction.", e1); } + continue; + } finally { + currentCompactionId.set(null); + } - if (compactionThread.isInterrupted() || JOB_HOLDER.isCancelled() - || (err.get() != null && err.get().getClass().equals(InterruptedException.class))) { - LOG.warn("Compaction thread was interrupted, sending CANCELLED state"); - try { - TCompactionStatusUpdate update = - new TCompactionStatusUpdate(TCompactionState.CANCELLED, "Compaction cancelled", - -1, -1, -1, fcr.getCompactionAge().toNanos()); - updateCompactionState(job, update); - updateCompactionFailed(job, InterruptedException.class.getName()); - cancelled.incrementAndGet(); - } catch (RetriesExceededException e) { - LOG.error("Error updating coordinator with compaction cancellation.", e); - } finally { - currentCompactionId.set(null); - } - } else if (err.get() != null) { - final KeyExtent fromThriftExtent = KeyExtent.fromThrift(job.getExtent()); - try { - LOG.info("Updating coordinator with compaction failure: id: {}, extent: {}", - job.getExternalCompactionId(), fromThriftExtent); - TCompactionStatusUpdate update = new TCompactionStatusUpdate( - TCompactionState.FAILED, "Compaction failed due to: " + err.get().getMessage(), - -1, -1, -1, fcr.getCompactionAge().toNanos()); - updateCompactionState(job, update); - updateCompactionFailed(job, err.get().getClass().getName()); - failed.incrementAndGet(); - errorHistory.addError(fromThriftExtent.tableId(), err.get()); - } catch (RetriesExceededException e) { - LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", - job.getExternalCompactionId(), fromThriftExtent, e); - } finally { - currentCompactionId.set(null); - } - } else { - try { - LOG.trace("Updating coordinator with compaction completion."); - updateCompactionCompleted(job, JOB_HOLDER.getStats()); - completed.incrementAndGet(); - // job completed successfully, clear the error history - errorHistory.clear(); - } catch (RetriesExceededException e) { - LOG.error( - "Error updating coordinator with compaction completion, cancelling compaction.", - e); + compactionThread.start(); // start the compactionThread + started.await(); // wait until the compactor is started + final long inputEntries = totalInputEntries.sum(); + final long waitTime = calculateProgressCheckTime(totalInputBytes.sum()); + LOG.debug("Progress checks will occur every {} seconds", waitTime); + String percentComplete = "unknown"; + + while (!stopped.await(waitTime, TimeUnit.SECONDS)) { + List running = + org.apache.accumulo.server.compaction.FileCompactor.getRunningCompactions(); + if (!running.isEmpty()) { + // Compaction has started. There should only be one in the list + CompactionInfo info = running.get(0); + if (info != null) { + final long entriesRead = info.getEntriesRead(); + final long entriesWritten = info.getEntriesWritten(); + if (inputEntries > 0) { + percentComplete = Float.toString((entriesRead / (float) inputEntries) * 100); + } + String message = String.format( + "Compaction in progress, read %d of %d input entries ( %s %s ), written %d entries", + entriesRead, inputEntries, percentComplete, "%", entriesWritten); + watcher.run(); try { - cancel(job.getExternalCompactionId()); - } catch (TException e1) { - LOG.error("Error cancelling compaction.", e1); + LOG.debug("Updating coordinator with compaction progress: {}.", message); + TCompactionStatusUpdate update = new TCompactionStatusUpdate( + TCompactionState.IN_PROGRESS, message, inputEntries, entriesRead, + entriesWritten, fcr.getCompactionAge().toNanos()); + updateCompactionState(job, update); + } catch (RetriesExceededException e) { + LOG.warn("Error updating coordinator with compaction progress, error: {}", + e.getMessage()); } - } finally { - currentCompactionId.set(null); } + } else { + LOG.debug("Waiting on compaction thread to finish, but no RUNNING compaction"); } - } catch (RuntimeException e1) { - LOG.error( - "Compactor thread was interrupted waiting for compaction to start, cancelling job", - e1); + } + compactionThread.join(); + LOG.trace("Compaction thread finished."); + // Run the watcher again to clear out the finished compaction and set the + // stuck count to zero. + watcher.run(); + + if (err.get() != null) { + // maybe the error occured because the table was deleted or something like that, so + // force a cancel check to possibly reduce noise in the logs + checkIfCanceled(); + } + + if (compactionThread.isInterrupted() || JOB_HOLDER.isCancelled() + || (err.get() != null && err.get().getClass().equals(InterruptedException.class))) { + LOG.warn("Compaction thread was interrupted, sending CANCELLED state"); try { - cancel(job.getExternalCompactionId()); - } catch (TException e2) { - LOG.error("Error cancelling compaction.", e2); + TCompactionStatusUpdate update = + new TCompactionStatusUpdate(TCompactionState.CANCELLED, "Compaction cancelled", + -1, -1, -1, fcr.getCompactionAge().toNanos()); + updateCompactionState(job, update); + updateCompactionFailed(job, InterruptedException.class.getName()); + cancelled.incrementAndGet(); + } catch (RetriesExceededException e) { + LOG.error("Error updating coordinator with compaction cancellation.", e); + } finally { + currentCompactionId.set(null); } - } finally { - currentCompactionId.set(null); + } else if (err.get() != null) { + final KeyExtent fromThriftExtent = KeyExtent.fromThrift(job.getExtent()); + try { + LOG.info("Updating coordinator with compaction failure: id: {}, extent: {}", + job.getExternalCompactionId(), fromThriftExtent); + TCompactionStatusUpdate update = new TCompactionStatusUpdate(TCompactionState.FAILED, + "Compaction failed due to: " + err.get().getMessage(), -1, -1, -1, + fcr.getCompactionAge().toNanos()); + updateCompactionState(job, update); + updateCompactionFailed(job, err.get().getClass().getName()); + failed.incrementAndGet(); + errorHistory.addError(fromThriftExtent.tableId(), err.get()); + } catch (RetriesExceededException e) { + LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", + job.getExternalCompactionId(), fromThriftExtent, e); + } finally { + currentCompactionId.set(null); + } + } else { + try { + LOG.trace("Updating coordinator with compaction completion."); + updateCompactionCompleted(job, JOB_HOLDER.getStats()); + completed.incrementAndGet(); + // job completed successfully, clear the error history + errorHistory.clear(); + } catch (RetriesExceededException e) { + LOG.error( + "Error updating coordinator with compaction completion, cancelling compaction.", + e); + try { + cancel(job.getExternalCompactionId()); + } catch (TException e1) { + LOG.error("Error cancelling compaction.", e1); + } + } finally { + currentCompactionId.set(null); + } + } + } catch (RuntimeException e1) { + LOG.error( + "Compactor thread was interrupted waiting for compaction to start, cancelling job", + e1); + try { + cancel(job.getExternalCompactionId()); + } catch (TException e2) { + LOG.error("Error cancelling compaction.", e2); + } + } finally { + currentCompactionId.set(null); - // mark compactor as idle after compaction completes - updateIdleStatus(true); + // mark compactor as idle after compaction completes + updateIdleStatus(true); - // In the case where there is an error in the foreground code the background compaction - // may still be running. Must cancel it before starting another iteration of the loop to - // avoid multiple threads updating shared state. - while (compactionThread.isAlive()) { - compactionThread.interrupt(); - compactionThread.join(1000); - } + // In the case where there is an error in the foreground code the background compaction + // may still be running. Must cancel it before starting another iteration of the loop to + // avoid multiple threads updating shared state. + while (compactionThread.isAlive()) { + compactionThread.interrupt(); + compactionThread.join(1000); } - } catch (InterruptedException e) { - LOG.info("Interrupt Exception received, shutting down"); - gracefulShutdown(getContext().rpcCreds()); } - } // end while - } catch (Exception e) { - LOG.error("Unhandled error occurred in Compactor", e); - } finally { - // Shutdown local thrift server - LOG.debug("Stopping Thrift Servers"); - if (getThriftServer() != null) { - getThriftServer().stop(); + } catch (InterruptedException e) { + LOG.info("Interrupt Exception received, shutting down"); + gracefulShutdown(getContext().rpcCreds()); } + } // end while, shutdown requested - try { - LOG.debug("Closing filesystems"); - VolumeManager mgr = getContext().getVolumeManager(); - if (null != mgr) { - mgr.close(); - } - } catch (IOException e) { - LOG.warn("Failed to close filesystem : {}", e.getMessage(), e); - } + // Shutdown local thrift server + LOG.debug("Stopping Thrift Servers"); + if (getThriftServer() != null) { + getThriftServer().stop(); + } - getContext().getLowMemoryDetector().logGCInfo(getConfiguration()); - super.close(); - getShutdownComplete().set(true); - LOG.info("stop requested. exiting ... "); - try { - if (null != compactorLock) { - compactorLock.unlock(); - } - } catch (Exception e) { - LOG.warn("Failed to release compactor lock", e); + try { + LOG.debug("Closing filesystems"); + VolumeManager mgr = getContext().getVolumeManager(); + if (null != mgr) { + mgr.close(); } + } catch (IOException e) { + LOG.warn("Failed to close filesystem : {}", e.getMessage(), e); } + getContext().getLowMemoryDetector().logGCInfo(getConfiguration()); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. + getShutdownComplete().set(true); + super.close(); + } public static void main(String[] args) throws Exception { 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 25383876c5a..c04c16ce86d 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 @@ -93,7 +93,7 @@ public class SimpleGarbageCollector extends AbstractServer implements Iface { private final Timer lastCompactorCheck = Timer.startNew(); - SimpleGarbageCollector(ConfigOpts opts, String[] args) { + protected SimpleGarbageCollector(ConfigOpts opts, String[] args) { super(ServerId.Type.GARBAGE_COLLECTOR, opts, ServerContext::new, args); final AccumuloConfiguration conf = getConfiguration(); @@ -352,15 +352,12 @@ public void run() { gracefulShutdown(getContext().rpcCreds()); } } - super.close(); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. getShutdownComplete().set(true); - log.info("stop requested. exiting ... "); - try { - gcLock.unlock(); - } catch (Exception e) { - log.warn("Failed to release GarbageCollector lock", e); - } - + super.close(); } private void incrementStatsForRun(GCRun gcRun) { @@ -370,7 +367,8 @@ private void incrementStatsForRun(GCRun gcRun) { status.current.errors += gcRun.getErrorsStat(); } - private void logStats() { + // public for ExitCodesIT + public void logStats() { log.info("Number of data file candidates for deletion: {}", status.current.candidates); log.info("Number of data file candidates still in use: {}", status.current.inUse); log.info("Number of successfully deleted data files: {}", status.current.deleted); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java index 7805cc85ab6..14c979e613f 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java @@ -1257,14 +1257,12 @@ boolean canSuspendTablets() { throw new IllegalStateException("Exception waiting on watcher", e); } } - super.close(); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. getShutdownComplete().set(true); - log.info("stop requested. exiting ... "); - try { - managerLock.unlock(); - } catch (Exception e) { - log.warn("Failed to release Manager lock", e); - } + super.close(); } protected Fate initializeFateInstance(ServerContext context, FateStore store) { diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java index 0658262f560..975ac8f2454 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java @@ -478,7 +478,7 @@ private TableMgmtStats manageTablets(Iterator iter, Set filteredServersToShutdown = new HashSet<>(tableMgmtParams.getServersToShutdown()); - while (iter.hasNext()) { + while (iter.hasNext() && !manager.isShutdownRequested()) { final TabletManagement mti = iter.next(); if (mti == null) { throw new IllegalStateException("State store returned a null ManagerTabletInfo object"); @@ -728,7 +728,7 @@ public void run() { int[] oldCounts = new int[TabletState.values().length]; boolean lookForTabletsNeedingVolReplacement = true; - while (manager.stillManager()) { + while (manager.stillManager() && !manager.isShutdownRequested()) { if (!eventHandler.isNeedsFullScan()) { // If an event handled by the EventHandler.RangeProcessor indicated // that we need to do a full scan, then do it. Otherwise wait a bit diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java index a5562e44bf8..02bf94ef98b 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java @@ -450,17 +450,12 @@ public void run() { } context.getLowMemoryDetector().logGCInfo(getConfiguration()); - super.close(); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. getShutdownComplete().set(true); - LOG.info("stop requested. exiting ... "); - try { - if (null != lock) { - lock.unlock(); - } - } catch (Exception e) { - LOG.warn("Failed to release scan server lock", e); - } - + super.close(); } } 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 bee188443ef..b3ac5432458 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 @@ -708,14 +708,12 @@ public void run() { } context.getLowMemoryDetector().logGCInfo(getConfiguration()); - super.close(); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. getShutdownComplete().set(true); - log.info("TServerInfo: stop requested. exiting ... "); - try { - tabletServerLock.unlock(); - } catch (Exception e) { - log.warn("Failed to release tablet server lock", e); - } + super.close(); } private boolean sendManagerMessages(boolean managerDown, ManagerClientService.Client iface, diff --git a/test/pom.xml b/test/pom.xml index e6ba98372d7..eeb0c355f32 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -78,6 +78,10 @@ io.opentelemetry opentelemetry-context + + net.bytebuddy + byte-buddy + org.apache.accumulo accumulo-compactor diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java new file mode 100644 index 00000000000..4477849c11d --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java @@ -0,0 +1,329 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.test.functional; + +import static org.apache.accumulo.harness.AccumuloITBase.MINI_CLUSTER_ONLY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Constructor; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.Function; +import java.util.stream.Stream; + +import org.apache.accumulo.compactor.Compactor; +import org.apache.accumulo.core.cli.ConfigOpts; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.conf.SiteConfiguration; +import org.apache.accumulo.gc.SimpleGarbageCollector; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.manager.Manager; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl.ProcessInfo; +import org.apache.accumulo.server.AbstractServer; +import org.apache.accumulo.server.ServerContext; +import org.apache.accumulo.test.functional.ExitCodesIT_SimpleSuite.ProcessProxy.TerminalBehavior; +import org.apache.accumulo.test.util.Wait; +import org.apache.accumulo.tserver.ScanServer; +import org.apache.accumulo.tserver.TabletServer; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.slf4j.LoggerFactory; + +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; +import net.bytebuddy.implementation.ExceptionMethod; +import net.bytebuddy.implementation.Implementation; +import net.bytebuddy.implementation.MethodCall; +import net.bytebuddy.matcher.ElementMatchers; + +@Tag(MINI_CLUSTER_ONLY) +public class ExitCodesIT_SimpleSuite extends SharedMiniClusterBase { + + public static class ServerContextFunction implements Function { + + @Override + public ServerContext apply(SiteConfiguration site) { + return new ServerContext(site); + } + + } + + // Dynamic Proxy class for a server instance that will be invoked via + // Accumulo's Main class using MiniAccumuloClusterImpl._exec(). This + // ProcessProxy class will determine which class to instantiate + // and what the terminal behavior should be using two system + // properties. A subclass of the desired class is dynamically created + // and started. + public static class ProcessProxy { + + public static enum TerminalBehavior { + SHUTDOWN, EXCEPTION, ERROR + }; + + public static final String PROXY_CLASS = "proxy.server"; + public static final String PROXY_METHOD_BEHAVIOR = "proxy.method.behavior"; + + public static void main(String[] args) throws Exception { + + final String proxyServer = System.getProperty(PROXY_CLASS); + Objects.requireNonNull(proxyServer, PROXY_CLASS + " must exist as an env var"); + final String methodBehavior = System.getProperty(PROXY_METHOD_BEHAVIOR); + Objects.requireNonNull(methodBehavior, methodBehavior + " must exist as an env var"); + + final ServerType st = ServerType.valueOf(proxyServer); + final TerminalBehavior behavior = TerminalBehavior.valueOf(methodBehavior); + + // Determine the constructor arguments and parameters for each server class. + // Find a method with no-args called during the servers run method that + // we can intercept to signal shutdown, exception, or error. + final Class serverClass; + final String methodName; + final Class[] ctorParams; + final Object[] ctorArgs; + switch (st) { + case COMPACTOR: + List compactorArgs = new ArrayList<>(); + compactorArgs.add("-o"); + compactorArgs.add(Property.COMPACTOR_GROUP_NAME.getKey() + "=TEST"); + serverClass = Compactor.class; + methodName = "updateIdleStatus"; + ctorParams = new Class[] {ConfigOpts.class, String[].class}; + ctorArgs = new Object[] {new ConfigOpts(), compactorArgs.toArray(new String[] {})}; + break; + case GARBAGE_COLLECTOR: + serverClass = SimpleGarbageCollector.class; + methodName = "logStats"; + ctorParams = new Class[] {ConfigOpts.class, String[].class}; + ctorArgs = new Object[] {new ConfigOpts(), new String[] {}}; + break; + case MANAGER: + serverClass = Manager.class; + methodName = "getThriftServer"; + ctorParams = new Class[] {ConfigOpts.class, Function.class, String[].class}; + ctorArgs = new Object[] {new ConfigOpts(), new ServerContextFunction(), new String[] {}}; + break; + case SCAN_SERVER: + List scanServerArgs = new ArrayList<>(); + scanServerArgs.add("-o"); + scanServerArgs.add(Property.SSERV_GROUP_NAME.getKey() + "=TEST"); + serverClass = ScanServer.class; + methodName = "updateIdleStatus"; + ctorParams = new Class[] {ConfigOpts.class, String[].class}; + ctorArgs = new Object[] {new ConfigOpts(), scanServerArgs.toArray(new String[] {})}; + break; + case TABLET_SERVER: + List tabletServerArgs = new ArrayList<>(); + tabletServerArgs.add("-o"); + tabletServerArgs.add(Property.TSERV_GROUP_NAME.getKey() + "=TEST"); + serverClass = TabletServer.class; + methodName = "updateIdleStatus"; + ctorParams = new Class[] {ConfigOpts.class, Function.class, String[].class}; + ctorArgs = new Object[] {new ConfigOpts(), new ServerContextFunction(), + tabletServerArgs.toArray(new String[] {})}; + break; + case MONITOR: + case ZOOKEEPER: + default: + throw new UnsupportedOperationException(st + " is not currently supported"); + } + + final Implementation implementation; + switch (behavior) { + case ERROR: + implementation = + ExceptionMethod.throwing(StackOverflowError.class, "throwing unknown error"); + break; + case EXCEPTION: + implementation = + ExceptionMethod.throwing(RuntimeException.class, "throwing runtime exception"); + break; + case SHUTDOWN: + implementation = + MethodCall.invoke(AbstractServer.class.getMethod("requestShutdownForTests")) + .andThen(MethodCall.invoke(serverClass.getMethod(methodName))); + break; + default: + throw new UnsupportedOperationException(behavior + " is not currently supported"); + } + + // Dynamically create the subclass with the specified behavior, load it + // into the JVM, and return it's Class. + // creates a subclass of server class defining a public constructor that + // takes an Integer, but calls the server class constructor with the + // appropriate args. Calls to the intercepted method name are intercepted + // by the supplied implementation + final Class dynamicServerType = + new ByteBuddy().subclass(serverClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) + .name("org.apache.accumulo.test.functional.exit_codes." + serverClass.getSimpleName() + + "_" + behavior.name()) + .defineConstructor(Visibility.PUBLIC).withParameters(Integer.class) + .intercept(MethodCall.invoke(serverClass.getDeclaredConstructor(ctorParams)).onSuper() + .with(ctorArgs)) + .method(ElementMatchers.named(methodName)).intercept(implementation).make() + .load(serverClass.getClassLoader()).getLoaded(); + + // Find and invoke the constructor of the dynamically created class + final Constructor ctor = + dynamicServerType.getDeclaredConstructor(Integer.class); + final AbstractServer dynamicServerInstance = ctor.newInstance(1); + + // Start the server process + AbstractServer.startServer(dynamicServerInstance, LoggerFactory.getLogger(serverClass)); + } + + } + + @BeforeAll + public static void beforeTests() throws Exception { + // Start MiniCluster so that getCluster() does not + // return null, + SharedMiniClusterBase.startMiniCluster(); + getCluster().getClusterControl().stop(ServerType.GARBAGE_COLLECTOR); + } + + @AfterAll + public static void afterTests() { + SharedMiniClusterBase.stopMiniCluster(); + } + + // Junit doesn't support more than one parameterized + // argument to a test method. We need to generate + // the arguments here. + static Stream generateWorkerProcessArguments() { + List args = new ArrayList<>(); + for (ServerType st : ServerType.values()) { + if (st == ServerType.COMPACTOR || st == ServerType.SCAN_SERVER + || st == ServerType.TABLET_SERVER) { + for (TerminalBehavior tb : TerminalBehavior.values()) { + args.add(Arguments.of(st, tb)); + } + } + } + return args.stream(); + } + + @ParameterizedTest + @MethodSource("generateWorkerProcessArguments") + public void testWorkerProcesses(ServerType server, TerminalBehavior behavior) throws Exception { + Map properties = new HashMap<>(); + properties.put(ProcessProxy.PROXY_CLASS, server.name()); + properties.put(ProcessProxy.PROXY_METHOD_BEHAVIOR, behavior.name()); + getCluster().getConfig().setSystemProperties(properties); + ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); + Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); + int exitValue = pi.getProcess().exitValue(); + if (server != ServerType.SCAN_SERVER) { + if (behavior == TerminalBehavior.SHUTDOWN) { + assertEquals(0, exitValue); + } else { + assertEquals(1, exitValue); + } + } else { + // TODO: + // Run method closes normally in a finally block + // Finally runs regardless of Exception or Error + assertEquals(0, exitValue); + } + } + + static Stream generateGarbageCollectorArguments() { + List args = new ArrayList<>(); + for (ServerType st : ServerType.values()) { + if (st == ServerType.GARBAGE_COLLECTOR) { + for (TerminalBehavior tb : TerminalBehavior.values()) { + args.add(Arguments.of(st, tb)); + } + } + } + return args.stream(); + } + + @ParameterizedTest + @MethodSource("generateGarbageCollectorArguments") + public void testGarbageCollector(ServerType server, TerminalBehavior behavior) throws Exception { + Map properties = new HashMap<>(); + properties.put(ProcessProxy.PROXY_CLASS, server.name()); + properties.put(ProcessProxy.PROXY_METHOD_BEHAVIOR, behavior.name()); + getCluster().getConfig().setSystemProperties(properties); + ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); + if (behavior == TerminalBehavior.SHUTDOWN) { + Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); + int exitValue = pi.getProcess().exitValue(); + assertEquals(0, exitValue); + } else if (behavior == TerminalBehavior.EXCEPTION) { + // GarbageCollector logs exceptions and keeps going. + // We need to let this time out and then + // terminate the process. + IllegalStateException ise = assertThrows(IllegalStateException.class, + () -> Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000)); + assertTrue(ise.getMessage().contains("Timeout exceeded")); + pi.getProcess().destroyForcibly(); + } else { + Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); + int exitValue = pi.getProcess().exitValue(); + assertEquals(1, exitValue); + } + } + + static Stream generateManagerArguments() { + List args = new ArrayList<>(); + for (ServerType st : ServerType.values()) { + if (st == ServerType.MANAGER) { + for (TerminalBehavior tb : TerminalBehavior.values()) { + args.add(Arguments.of(st, tb)); + } + } + } + return args.stream(); + } + + @ParameterizedTest + @MethodSource("generateManagerArguments") + public void testManager(ServerType server, TerminalBehavior behavior) throws Exception { + try { + getCluster().getClusterControl().stop(ServerType.MANAGER); + Map properties = new HashMap<>(); + properties.put(ProcessProxy.PROXY_CLASS, server.name()); + properties.put(ProcessProxy.PROXY_METHOD_BEHAVIOR, behavior.name()); + getCluster().getConfig().setSystemProperties(properties); + ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); + Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); + int exitValue = pi.getProcess().exitValue(); + if (behavior == TerminalBehavior.SHUTDOWN) { + assertEquals(0, exitValue); + } else { + assertEquals(1, exitValue); + } + } finally { + getCluster().getClusterControl().stop(ServerType.MANAGER); + } + } + +} From 5043c2eacea406b795edf15287cf765460f4a574 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 20 Aug 2025 21:23:42 +0000 Subject: [PATCH 2/9] Fixed Manager test --- .../main/java/org/apache/accumulo/manager/Manager.java | 7 ++++++- .../test/functional/ExitCodesIT_SimpleSuite.java | 10 +++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java index 14c979e613f..67d15d7ad86 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java @@ -1206,7 +1206,7 @@ boolean canSuspendTablets() { break; } try { - Thread.sleep(500); + mainWait(); } catch (InterruptedException e) { log.info("Interrupt Exception received, shutting down"); gracefulShutdown(context.rpcCreds()); @@ -1265,6 +1265,11 @@ boolean canSuspendTablets() { super.close(); } + // method exists for ExitCodesIT + public void mainWait() throws InterruptedException { + Thread.sleep(500); + } + protected Fate initializeFateInstance(ServerContext context, FateStore store) { final Fate fateInstance = new Fate<>(this, store, true, TraceRepo::toLogString, diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java index 4477849c11d..01997b29c88 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java @@ -101,8 +101,9 @@ public static void main(String[] args) throws Exception { final TerminalBehavior behavior = TerminalBehavior.valueOf(methodBehavior); // Determine the constructor arguments and parameters for each server class. - // Find a method with no-args called during the servers run method that - // we can intercept to signal shutdown, exception, or error. + // Find a method with no-args that does not return anything that is + // called during the servers run method that we can intercept to signal + // shutdown, exception, or error. final Class serverClass; final String methodName; final Class[] ctorParams; @@ -125,7 +126,7 @@ public static void main(String[] args) throws Exception { break; case MANAGER: serverClass = Manager.class; - methodName = "getThriftServer"; + methodName = "mainWait"; ctorParams = new Class[] {ConfigOpts.class, Function.class, String[].class}; ctorArgs = new Object[] {new ConfigOpts(), new ServerContextFunction(), new String[] {}}; break; @@ -166,8 +167,7 @@ public static void main(String[] args) throws Exception { break; case SHUTDOWN: implementation = - MethodCall.invoke(AbstractServer.class.getMethod("requestShutdownForTests")) - .andThen(MethodCall.invoke(serverClass.getMethod(methodName))); + MethodCall.invoke(AbstractServer.class.getMethod("requestShutdownForTests")); break; default: throw new UnsupportedOperationException(behavior + " is not currently supported"); From d41dbcc1dd57ad7c42650df63a773c36e34e89c5 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 21 Aug 2025 12:20:56 +0000 Subject: [PATCH 3/9] Implemented PR suggestions --- .../accumulo/core/clientImpl/ClientContext.java | 10 +++++----- .../org/apache/accumulo/server/ServerContext.java | 6 +++--- ...itCodesIT_SimpleSuite.java => ExitCodesIT.java} | 14 +++++--------- 3 files changed, 13 insertions(+), 17 deletions(-) rename test/src/main/java/org/apache/accumulo/test/functional/{ExitCodesIT_SimpleSuite.java => ExitCodesIT.java} (97%) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java index 8592b3bfdf1..dc5a03288ed 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java @@ -919,23 +919,23 @@ public AuthenticationToken token() { public synchronized void close() { if (closed.compareAndSet(false, true)) { if (thriftTransportPool != null) { - log.info("Closing Thrift Transport Pool"); + log.debug("Closing Thrift Transport Pool"); thriftTransportPool.shutdown(); } if (scannerReadaheadPool != null) { - log.info("Closing Scanner ReadAhead Pool"); + log.debug("Closing Scanner ReadAhead Pool"); scannerReadaheadPool.shutdownNow(); // abort all tasks, client is shutting down } if (cleanupThreadPool != null) { - log.info("Closing Cleanup ThreadPool"); + log.debug("Closing Cleanup ThreadPool"); cleanupThreadPool.shutdown(); // wait for shutdown tasks to execute } if (zooCacheCreated.get()) { - log.info("Closing ZooCache"); + log.debug("Closing ZooCache"); zooCache.get().close(); } if (zooKeeperOpened.get()) { - log.info("Closing ZooSession"); + log.debug("Closing ZooSession"); zooSession.get().close(); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java index 021db43d14e..a6919bed3eb 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java @@ -522,11 +522,11 @@ public void close() { getMetricsInfo().close(); } if (sharedSchedExecutorCreated.get()) { - log.info("Shutting down shared executor pool"); + log.debug("Shutting down shared executor pool"); getScheduledExecutor().shutdownNow(); } if (sharedMetadataWriterCreated.get()) { - log.info("Shutting down shared metadata conditional writer"); + log.debug("Shutting down shared metadata conditional writer"); try { ConditionalWriter writer = sharedMetadataWriter.get(); if (writer != null) { @@ -537,7 +537,7 @@ public void close() { } } if (sharedUserWriterCreated.get()) { - log.info("Shutting down shared user conditional writer"); + log.debug("Shutting down shared user conditional writer"); try { ConditionalWriter writer = sharedUserWriter.get(); if (writer != null) { diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java similarity index 97% rename from test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java rename to test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java index 01997b29c88..1a212489cd4 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT_SimpleSuite.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java @@ -43,7 +43,7 @@ import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl.ProcessInfo; import org.apache.accumulo.server.AbstractServer; import org.apache.accumulo.server.ServerContext; -import org.apache.accumulo.test.functional.ExitCodesIT_SimpleSuite.ProcessProxy.TerminalBehavior; +import org.apache.accumulo.test.functional.ExitCodesIT.ProcessProxy.TerminalBehavior; import org.apache.accumulo.test.util.Wait; import org.apache.accumulo.tserver.ScanServer; import org.apache.accumulo.tserver.TabletServer; @@ -64,7 +64,7 @@ import net.bytebuddy.matcher.ElementMatchers; @Tag(MINI_CLUSTER_ONLY) -public class ExitCodesIT_SimpleSuite extends SharedMiniClusterBase { +public class ExitCodesIT extends SharedMiniClusterBase { public static class ServerContextFunction implements Function { @@ -239,17 +239,13 @@ public void testWorkerProcesses(ServerType server, TerminalBehavior behavior) th ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); int exitValue = pi.getProcess().exitValue(); - if (server != ServerType.SCAN_SERVER) { - if (behavior == TerminalBehavior.SHUTDOWN) { - assertEquals(0, exitValue); - } else { - assertEquals(1, exitValue); - } - } else { + if (server == ServerType.SCAN_SERVER) { // TODO: // Run method closes normally in a finally block // Finally runs regardless of Exception or Error assertEquals(0, exitValue); + } else { + assertEquals(behavior == TerminalBehavior.SHUTDOWN ? 0 : 1, exitValue); } } From 9ae6f1d4545cefc4d9321063ccc68d72e449daa8 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 21 Aug 2025 12:55:33 +0000 Subject: [PATCH 4/9] Resolve issue with ScanServer by removing finally block --- .../apache/accumulo/tserver/ScanServer.java | 93 +++++++++---------- .../accumulo/test/functional/ExitCodesIT.java | 15 +-- 2 files changed, 47 insertions(+), 61 deletions(-) diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java index 02bf94ef98b..3f5521a9002 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java @@ -400,63 +400,60 @@ public void run() { "Log sorting for tablet recovery is disabled, SSERV_WAL_SORT_MAX_CONCURRENT is less than 1."); } - try { - while (!isShutdownRequested()) { - if (Thread.currentThread().isInterrupted()) { - LOG.info("Server process thread has been interrupted, shutting down"); - break; - } - try { - Thread.sleep(1000); - updateIdleStatus(sessionManager.getActiveScans().isEmpty() - && tabletMetadataCache.estimatedSize() == 0); - } catch (InterruptedException e) { - LOG.info("Interrupt Exception received, shutting down"); - gracefulShutdown(getContext().rpcCreds()); - } + while (!isShutdownRequested()) { + if (Thread.currentThread().isInterrupted()) { + LOG.info("Server process thread has been interrupted, shutting down"); + break; } - } finally { - // Wait for scans to got to zero - while (!sessionManager.getActiveScans().isEmpty()) { - LOG.debug("Waiting on {} active scans to complete.", - sessionManager.getActiveScans().size()); - UtilWaitThread.sleep(1000); + try { + Thread.sleep(1000); + updateIdleStatus( + sessionManager.getActiveScans().isEmpty() && tabletMetadataCache.estimatedSize() == 0); + } catch (InterruptedException e) { + LOG.info("Interrupt Exception received, shutting down"); + gracefulShutdown(getContext().rpcCreds()); } + } - LOG.debug("Stopping Thrift Servers"); - getThriftServer().stop(); + // Wait for scans to get to zero + while (!sessionManager.getActiveScans().isEmpty()) { + LOG.debug("Waiting on {} active scans to complete.", sessionManager.getActiveScans().size()); + UtilWaitThread.sleep(1000); + } - try { - LOG.info("Removing server scan references"); - this.getContext().getAmple().scanServerRefs().delete(getAdvertiseAddress().toString(), - serverLockUUID); - } catch (Exception e) { - LOG.warn("Failed to remove scan server refs from metadata location", e); - } + LOG.debug("Stopping Thrift Servers"); + getThriftServer().stop(); - try { - LOG.debug("Closing filesystems"); - VolumeManager mgr = getContext().getVolumeManager(); - if (null != mgr) { - mgr.close(); - } - } catch (IOException e) { - LOG.warn("Failed to close filesystem : {}", e.getMessage(), e); - } + try { + LOG.info("Removing server scan references"); + this.getContext().getAmple().scanServerRefs().delete(getAdvertiseAddress().toString(), + serverLockUUID); + } catch (Exception e) { + LOG.warn("Failed to remove scan server refs from metadata location", e); + } - if (tmCacheExecutor != null) { - LOG.debug("Shutting down TabletMetadataCache executor"); - tmCacheExecutor.shutdownNow(); + try { + LOG.debug("Closing filesystems"); + VolumeManager mgr = getContext().getVolumeManager(); + if (null != mgr) { + mgr.close(); } + } catch (IOException e) { + LOG.warn("Failed to close filesystem : {}", e.getMessage(), e); + } - context.getLowMemoryDetector().logGCInfo(getConfiguration()); - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - getShutdownComplete().set(true); - super.close(); + if (tmCacheExecutor != null) { + LOG.debug("Shutting down TabletMetadataCache executor"); + tmCacheExecutor.shutdownNow(); } + + context.getLowMemoryDetector().logGCInfo(getConfiguration()); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. + getShutdownComplete().set(true); + super.close(); } @SuppressWarnings("unchecked") diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java index 1a212489cd4..4f8f948d268 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java @@ -239,14 +239,7 @@ public void testWorkerProcesses(ServerType server, TerminalBehavior behavior) th ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); int exitValue = pi.getProcess().exitValue(); - if (server == ServerType.SCAN_SERVER) { - // TODO: - // Run method closes normally in a finally block - // Finally runs regardless of Exception or Error - assertEquals(0, exitValue); - } else { - assertEquals(behavior == TerminalBehavior.SHUTDOWN ? 0 : 1, exitValue); - } + assertEquals(behavior == TerminalBehavior.SHUTDOWN ? 0 : 1, exitValue); } static Stream generateGarbageCollectorArguments() { @@ -312,11 +305,7 @@ public void testManager(ServerType server, TerminalBehavior behavior) throws Exc ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); int exitValue = pi.getProcess().exitValue(); - if (behavior == TerminalBehavior.SHUTDOWN) { - assertEquals(0, exitValue); - } else { - assertEquals(1, exitValue); - } + assertEquals(behavior == TerminalBehavior.SHUTDOWN ? 0 : 1, exitValue); } finally { getCluster().getClusterControl().stop(ServerType.MANAGER); } From bf5e3be8957cd3f51098e55899f7b049209eceee Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 25 Aug 2025 16:55:38 +0000 Subject: [PATCH 5/9] Removed finally block in AbstractServer.startServer that called close --- .../java/org/apache/accumulo/server/AbstractServer.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java index 940f34d8159..e065e5159dc 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java @@ -84,15 +84,6 @@ public static void startServer(AbstractServer server, Logger LOG) throws Excepti e.printStackTrace(); LOG.error("{} died, exception thrown from runServer.", server.getClass().getSimpleName(), e); throw e; - } finally { - try { - server.close(); - } catch (Throwable e) { - System.err.println("Exception thrown while closing " + server.getClass().getSimpleName()); - e.printStackTrace(); - LOG.error("Exception thrown while closing {}", server.getClass().getSimpleName(), e); - throw e; - } } } From 1337d84365fd5406de0410ceb8fbb8d19232bd78 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 25 Aug 2025 20:04:33 +0000 Subject: [PATCH 6/9] Moved common close calls into AbstractServer, updated Halt calls --- .../org/apache/accumulo/core/util/Halt.java | 2 +- .../accumulo/server/AbstractServer.java | 19 +++++++++++++------ .../server/manager/LiveTServerSet.java | 2 +- .../server/mem/LowMemoryDetector.java | 2 +- .../apache/accumulo/compactor/Compactor.java | 6 ------ .../accumulo/gc/SimpleGarbageCollector.java | 5 ----- .../org/apache/accumulo/manager/Manager.java | 5 ----- .../apache/accumulo/tserver/ScanServer.java | 6 ------ .../apache/accumulo/tserver/TabletServer.java | 10 ++-------- .../tserver/log/TabletServerLogger.java | 2 +- .../tserver/tablet/MinorCompactor.java | 2 +- .../accumulo/tserver/tablet/Tablet.java | 2 +- 12 files changed, 21 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/util/Halt.java b/core/src/main/java/org/apache/accumulo/core/util/Halt.java index 48e82deb9bd..bea10e9b12e 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/Halt.java +++ b/core/src/main/java/org/apache/accumulo/core/util/Halt.java @@ -40,7 +40,7 @@ public static void halt(final int status, final String msg, final Runnable runna halt(status, msg, null, runnable); } - public static void halt(final int status, final String msg, final Throwable exception, + private static void halt(final int status, final String msg, final Throwable exception, final Runnable runnable) { try { diff --git a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java index e065e5159dc..bf2b29face9 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java @@ -445,7 +445,7 @@ public void startServiceLockVerificationThread() { log.trace( "ServiceLockVerificationThread - checking ServiceLock existence in ZooKeeper"); if (lock != null && !lock.verifyLockAtSource()) { - Halt.halt(-1, "Lock verification thread could not find lock"); + Halt.halt(1, "Lock verification thread could not find lock"); } // Need to sleep, not yield when the thread priority is greater than NORM_PRIORITY // so that this thread does not get immediately rescheduled. @@ -470,11 +470,18 @@ public void startServiceLockVerificationThread() { @Override public void close() { - // close is called from the subclasses run method - // when shutting down. Close is also called after - // the runServer method completes in the startServer - // method in this class. Calling twice could raise - // an exception when in reality everything was fine + + context.getLowMemoryDetector().logGCInfo(getConfiguration()); + + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. + // The ServiceLockWatcher has a reference to shutdownComplete + // and will terminate the JVM with a 0 exit code if true. + // Otherwise it will exit with a non-zero exit code. + getShutdownComplete().set(true); + if (closed.compareAndSet(false, true)) { if (context != null) { context.close(); diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java b/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java index a178f0b44b6..d367f356ebf 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java +++ b/server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java @@ -519,7 +519,7 @@ public synchronized void remove(TServerInstance server) { try { context.getZooSession().asReaderWriter().recursiveDelete(slp.toString(), SKIP); } catch (Exception e) { - Halt.halt(-1, "error removing tablet server lock", e); + Halt.halt(1, "error removing tablet server lock", e); } context.getZooCache().clear(slp.toString()); } diff --git a/server/base/src/main/java/org/apache/accumulo/server/mem/LowMemoryDetector.java b/server/base/src/main/java/org/apache/accumulo/server/mem/LowMemoryDetector.java index 8d745e77743..ca320b3cd15 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/mem/LowMemoryDetector.java +++ b/server/base/src/main/java/org/apache/accumulo/server/mem/LowMemoryDetector.java @@ -193,7 +193,7 @@ public synchronized void logGCInfo(AccumuloConfiguration conf) { } if (maxIncreaseInCollectionTime > keepAliveTimeout) { - Halt.halt(-1, "Garbage collection may be interfering with lock keep-alive. Halting."); + Halt.halt(1, "Garbage collection may be interfering with lock keep-alive. Halting."); } localState.lastMemorySize = freeMemory; diff --git a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java index 9a679081d03..14e9ca8961d 100644 --- a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java +++ b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java @@ -1062,12 +1062,6 @@ public void run() { LOG.warn("Failed to close filesystem : {}", e.getMessage(), e); } - getContext().getLowMemoryDetector().logGCInfo(getConfiguration()); - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - getShutdownComplete().set(true); super.close(); } 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 c04c16ce86d..e1e6fe51444 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 @@ -352,11 +352,6 @@ public void run() { gracefulShutdown(getContext().rpcCreds()); } } - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - getShutdownComplete().set(true); super.close(); } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java index 67d15d7ad86..0eb1edfc4a9 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java @@ -1257,11 +1257,6 @@ boolean canSuspendTablets() { throw new IllegalStateException("Exception waiting on watcher", e); } } - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - getShutdownComplete().set(true); super.close(); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java index 3f5521a9002..4fcdc27910e 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java @@ -447,12 +447,6 @@ public void run() { tmCacheExecutor.shutdownNow(); } - context.getLowMemoryDetector().logGCInfo(getConfiguration()); - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - getShutdownComplete().set(true); super.close(); } 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 b3ac5432458..61472ced020 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 @@ -671,7 +671,7 @@ public void run() { try { // Ask the manager to unload our tablets and stop loading new tablets if (iface == null) { - Halt.halt(-1, "Error informing Manager that we are shutting down, exiting!"); + Halt.halt(1, "Error informing Manager that we are shutting down, exiting!"); } else { iface.tabletServerStopping(TraceUtil.traceInfo(), getContext().rpcCreds(), getTabletSession().getHostPortSession(), getResourceGroup().canonical()); @@ -690,7 +690,7 @@ public void run() { sendManagerMessages(managerDown, iface, advertiseAddressString); } catch (TException | RuntimeException e) { - Halt.halt(-1, "Error informing Manager that we are shutting down, exiting!", e); + Halt.halt(1, "Error informing Manager that we are shutting down, exiting!", e); } finally { returnManagerConnection(iface); } @@ -707,12 +707,6 @@ public void run() { log.warn("Failed to close filesystem : {}", e.getMessage(), e); } - context.getLowMemoryDetector().logGCInfo(getConfiguration()); - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - getShutdownComplete().set(true); super.close(); } 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 1ae26c5a05f..559cc8a487e 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 @@ -455,7 +455,7 @@ private void write(final Collection sessions, boolean mincFinish, if (sawWriteFailure != null) { log.info("WAL write failure, validating server lock in ZooKeeper", sawWriteFailure); if (tabletServerLock == null || !tabletServerLock.verifyLockAtSource()) { - Halt.halt(-1, "Writing to WAL has failed and TabletServer lock does not exist", + Halt.halt(1, "Writing to WAL has failed and TabletServer lock does not exist", sawWriteFailure); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactor.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactor.java index e091ecb4010..113aa1928d3 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactor.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactor.java @@ -99,7 +99,7 @@ public CompactionStats call() { if (tserverLock == null || !tserverLock.verifyLockAtSource()) { log.error("Minor compaction of {} has failed and TabletServer lock does not exist." + " Halting...", getExtent(), e); - Halt.halt(-1, "TabletServer lock does not exist", e); + Halt.halt(1, "TabletServer lock does not exist", e); } else { throw e; } 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 c2170a42d4b..1c96bddbe70 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 @@ -414,7 +414,7 @@ DataFileValue minorCompact(InMemoryMap memTable, ReferencedTabletFile tmpDatafil if (tserverLock == null || !tserverLock.verifyLockAtSource()) { log.error("Minor compaction of {} has failed and TabletServer lock does not exist." + " Halting...", getExtent(), e); - Halt.halt(-1, "TabletServer lock does not exist", e); + Halt.halt(1, "TabletServer lock does not exist", e); } else { TraceUtil.setException(span2, e, true); throw e; From 57e6330f1334b41de9c0a8291dc685d880df7146 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 25 Aug 2025 20:28:21 +0000 Subject: [PATCH 7/9] Fix SpotBugs and logic issue in AbstractServer.close --- .../accumulo/server/AbstractServer.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java index bf2b29face9..30562223585 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java @@ -471,19 +471,19 @@ public void startServiceLockVerificationThread() { @Override public void close() { - context.getLowMemoryDetector().logGCInfo(getConfiguration()); + if (closed.compareAndSet(false, true)) { - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - // The ServiceLockWatcher has a reference to shutdownComplete - // and will terminate the JVM with a 0 exit code if true. - // Otherwise it will exit with a non-zero exit code. - getShutdownComplete().set(true); + // Must set shutdown as completed before calling super.close(). + // super.close() calls ServerContext.close() -> + // ClientContext.close() -> ZooSession.close() which removes + // all of the ephemeral nodes and forces the watches to fire. + // The ServiceLockWatcher has a reference to shutdownComplete + // and will terminate the JVM with a 0 exit code if true. + // Otherwise it will exit with a non-zero exit code. + getShutdownComplete().set(true); - if (closed.compareAndSet(false, true)) { if (context != null) { + context.getLowMemoryDetector().logGCInfo(getConfiguration()); context.close(); } } From ec5cd1d06c1799c075635f0d168b8981fcafc024 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 27 Aug 2025 16:33:08 +0000 Subject: [PATCH 8/9] Addressed PR comments --- pom.xml | 5 - .../accumulo/server/AbstractServer.java | 28 +- .../apache/accumulo/compactor/Compactor.java | 5 - .../accumulo/gc/SimpleGarbageCollector.java | 1 - .../org/apache/accumulo/manager/Manager.java | 1 - .../apache/accumulo/tserver/ScanServer.java | 2 - .../apache/accumulo/tserver/TabletServer.java | 2 - test/pom.xml | 4 - .../accumulo/test/functional/ExitCodesIT.java | 332 ++++++++++-------- 9 files changed, 187 insertions(+), 193 deletions(-) diff --git a/pom.xml b/pom.xml index 7b1e5b66e34..50ed3d5b7dd 100644 --- a/pom.xml +++ b/pom.xml @@ -328,11 +328,6 @@ commons-validator 1.10.0 - - net.bytebuddy - byte-buddy - 1.17.6 - org.apache.accumulo accumulo-access diff --git a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java index 30562223585..0b94dc1203c 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java @@ -76,15 +76,7 @@ public static interface ThriftServerSupplier { } public static void startServer(AbstractServer server, Logger LOG) throws Exception { - try { - server.runServer(); - } catch (Throwable e) { - System.err - .println(server.getClass().getSimpleName() + " died, exception thrown from runServer."); - e.printStackTrace(); - LOG.error("{} died, exception thrown from runServer.", server.getClass().getSimpleName(), e); - throw e; - } + server.runServer(); } private final MetricSource metricSource; @@ -283,7 +275,10 @@ public AtomicBoolean getShutdownComplete() { */ public void runServer() throws Exception { final AtomicReference err = new AtomicReference<>(); - serverThread = new Thread(TraceUtil.wrap(this), applicationName); + serverThread = new Thread(TraceUtil.wrap(() -> { + this.run(); + close(); + }), applicationName); serverThread.setUncaughtExceptionHandler((thread, exception) -> err.set(exception)); serverThread.start(); serverThread.join(); @@ -473,13 +468,12 @@ public void close() { if (closed.compareAndSet(false, true)) { - // Must set shutdown as completed before calling super.close(). - // super.close() calls ServerContext.close() -> - // ClientContext.close() -> ZooSession.close() which removes - // all of the ephemeral nodes and forces the watches to fire. - // The ServiceLockWatcher has a reference to shutdownComplete - // and will terminate the JVM with a 0 exit code if true. - // Otherwise it will exit with a non-zero exit code. + // Must set shutdown as completed before calling ServerContext.close(). + // ServerContext.close() calls ClientContext.close() -> + // ZooSession.close() which removes all of the ephemeral nodes and + // forces the watches to fire. The ServiceLockWatcher has a reference + // to shutdownComplete and will terminate the JVM with a 0 exit code + // if true. Otherwise it will exit with a non-zero exit code. getShutdownComplete().set(true); if (context != null) { diff --git a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java index 14e9ca8961d..9772d4bd9f6 100644 --- a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java +++ b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java @@ -910,8 +910,6 @@ public void run() { LOG.error("Error cancelling compaction.", e1); } continue; - } finally { - currentCompactionId.set(null); } compactionThread.start(); // start the compactionThread @@ -1061,9 +1059,6 @@ public void run() { } catch (IOException e) { LOG.warn("Failed to close filesystem : {}", e.getMessage(), e); } - - super.close(); - } public static void main(String[] args) throws Exception { 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 e1e6fe51444..5b09324d4a3 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 @@ -352,7 +352,6 @@ public void run() { gracefulShutdown(getContext().rpcCreds()); } } - super.close(); } private void incrementStatsForRun(GCRun gcRun) { diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java index 0eb1edfc4a9..cb50ebae840 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java @@ -1257,7 +1257,6 @@ boolean canSuspendTablets() { throw new IllegalStateException("Exception waiting on watcher", e); } } - super.close(); } // method exists for ExitCodesIT diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java index 4fcdc27910e..f218f8f71f4 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java @@ -446,8 +446,6 @@ public void run() { LOG.debug("Shutting down TabletMetadataCache executor"); tmCacheExecutor.shutdownNow(); } - - super.close(); } @SuppressWarnings("unchecked") 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 61472ced020..1861389febc 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 @@ -706,8 +706,6 @@ public void run() { } catch (IOException e) { log.warn("Failed to close filesystem : {}", e.getMessage(), e); } - - super.close(); } private boolean sendManagerMessages(boolean managerDown, ManagerClientService.Client iface, diff --git a/test/pom.xml b/test/pom.xml index eeb0c355f32..e6ba98372d7 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -78,10 +78,6 @@ io.opentelemetry opentelemetry-context - - net.bytebuddy - byte-buddy - org.apache.accumulo accumulo-compactor diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java index 4f8f948d268..e5bbc48376e 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java @@ -23,19 +23,17 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.lang.reflect.Constructor; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.Function; import java.util.stream.Stream; import org.apache.accumulo.compactor.Compactor; import org.apache.accumulo.core.cli.ConfigOpts; import org.apache.accumulo.core.conf.Property; -import org.apache.accumulo.core.conf.SiteConfiguration; import org.apache.accumulo.gc.SimpleGarbageCollector; import org.apache.accumulo.harness.SharedMiniClusterBase; import org.apache.accumulo.manager.Manager; @@ -43,7 +41,6 @@ import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl.ProcessInfo; import org.apache.accumulo.server.AbstractServer; import org.apache.accumulo.server.ServerContext; -import org.apache.accumulo.test.functional.ExitCodesIT.ProcessProxy.TerminalBehavior; import org.apache.accumulo.test.util.Wait; import org.apache.accumulo.tserver.ScanServer; import org.apache.accumulo.tserver.TabletServer; @@ -52,152 +49,182 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; import org.slf4j.LoggerFactory; -import net.bytebuddy.ByteBuddy; -import net.bytebuddy.description.modifier.Visibility; -import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; -import net.bytebuddy.implementation.ExceptionMethod; -import net.bytebuddy.implementation.Implementation; -import net.bytebuddy.implementation.MethodCall; -import net.bytebuddy.matcher.ElementMatchers; - @Tag(MINI_CLUSTER_ONLY) public class ExitCodesIT extends SharedMiniClusterBase { - public static class ServerContextFunction implements Function { + public static enum TerminalBehavior { + SHUTDOWN, EXCEPTION, ERROR + }; - @Override - public ServerContext apply(SiteConfiguration site) { - return new ServerContext(site); - } + public static final String PROXY_METHOD_BEHAVIOR = "proxy.method.behavior"; + public static TerminalBehavior getTerminalBehavior() { + final String methodBehavior = System.getProperty(PROXY_METHOD_BEHAVIOR); + Objects.requireNonNull(methodBehavior, methodBehavior + " must exist as an env var"); + return TerminalBehavior.valueOf(methodBehavior); } - // Dynamic Proxy class for a server instance that will be invoked via - // Accumulo's Main class using MiniAccumuloClusterImpl._exec(). This - // ProcessProxy class will determine which class to instantiate - // and what the terminal behavior should be using two system - // properties. A subclass of the desired class is dynamically created - // and started. - public static class ProcessProxy { + public static class ExitCompactor extends Compactor { - public static enum TerminalBehavior { - SHUTDOWN, EXCEPTION, ERROR - }; + public static void main(String[] args) throws Exception { + List compactorArgs = new ArrayList<>(); + compactorArgs.add("-o"); + compactorArgs.add(Property.COMPACTOR_GROUP_NAME.getKey() + "=TEST"); + AbstractServer.startServer(new ExitCompactor(getTerminalBehavior(), compactorArgs), + LoggerFactory.getLogger(ExitCompactor.class)); + } - public static final String PROXY_CLASS = "proxy.server"; - public static final String PROXY_METHOD_BEHAVIOR = "proxy.method.behavior"; + private final TerminalBehavior behavior; - public static void main(String[] args) throws Exception { + public ExitCompactor(TerminalBehavior behavior, List compactorArgs) { + super(new ConfigOpts(), compactorArgs.toArray(new String[] {})); + this.behavior = behavior; + } - final String proxyServer = System.getProperty(PROXY_CLASS); - Objects.requireNonNull(proxyServer, PROXY_CLASS + " must exist as an env var"); - final String methodBehavior = System.getProperty(PROXY_METHOD_BEHAVIOR); - Objects.requireNonNull(methodBehavior, methodBehavior + " must exist as an env var"); - - final ServerType st = ServerType.valueOf(proxyServer); - final TerminalBehavior behavior = TerminalBehavior.valueOf(methodBehavior); - - // Determine the constructor arguments and parameters for each server class. - // Find a method with no-args that does not return anything that is - // called during the servers run method that we can intercept to signal - // shutdown, exception, or error. - final Class serverClass; - final String methodName; - final Class[] ctorParams; - final Object[] ctorArgs; - switch (st) { - case COMPACTOR: - List compactorArgs = new ArrayList<>(); - compactorArgs.add("-o"); - compactorArgs.add(Property.COMPACTOR_GROUP_NAME.getKey() + "=TEST"); - serverClass = Compactor.class; - methodName = "updateIdleStatus"; - ctorParams = new Class[] {ConfigOpts.class, String[].class}; - ctorArgs = new Object[] {new ConfigOpts(), compactorArgs.toArray(new String[] {})}; - break; - case GARBAGE_COLLECTOR: - serverClass = SimpleGarbageCollector.class; - methodName = "logStats"; - ctorParams = new Class[] {ConfigOpts.class, String[].class}; - ctorArgs = new Object[] {new ConfigOpts(), new String[] {}}; - break; - case MANAGER: - serverClass = Manager.class; - methodName = "mainWait"; - ctorParams = new Class[] {ConfigOpts.class, Function.class, String[].class}; - ctorArgs = new Object[] {new ConfigOpts(), new ServerContextFunction(), new String[] {}}; - break; - case SCAN_SERVER: - List scanServerArgs = new ArrayList<>(); - scanServerArgs.add("-o"); - scanServerArgs.add(Property.SSERV_GROUP_NAME.getKey() + "=TEST"); - serverClass = ScanServer.class; - methodName = "updateIdleStatus"; - ctorParams = new Class[] {ConfigOpts.class, String[].class}; - ctorArgs = new Object[] {new ConfigOpts(), scanServerArgs.toArray(new String[] {})}; - break; - case TABLET_SERVER: - List tabletServerArgs = new ArrayList<>(); - tabletServerArgs.add("-o"); - tabletServerArgs.add(Property.TSERV_GROUP_NAME.getKey() + "=TEST"); - serverClass = TabletServer.class; - methodName = "updateIdleStatus"; - ctorParams = new Class[] {ConfigOpts.class, Function.class, String[].class}; - ctorArgs = new Object[] {new ConfigOpts(), new ServerContextFunction(), - tabletServerArgs.toArray(new String[] {})}; + @Override + public void updateIdleStatus(boolean isIdle) { + switch (behavior) { + case ERROR: + throw new StackOverflowError("throwing stack overflow error"); + case EXCEPTION: + throw new RuntimeException("throwing runtime exception"); + case SHUTDOWN: + requestShutdownForTests(); break; - case MONITOR: - case ZOOKEEPER: default: - throw new UnsupportedOperationException(st + " is not currently supported"); + throw new UnsupportedOperationException(behavior + " is not currently supported"); } + } + } + + public static class ExitScanServer extends ScanServer { + + public static void main(String[] args) throws Exception { + List sserverArgs = new ArrayList<>(); + sserverArgs.add("-o"); + sserverArgs.add(Property.SSERV_GROUP_NAME.getKey() + "=TEST"); + AbstractServer.startServer(new ExitScanServer(getTerminalBehavior(), sserverArgs), + LoggerFactory.getLogger(ExitScanServer.class)); + } + + private final TerminalBehavior behavior; + + public ExitScanServer(TerminalBehavior behavior, List sserverArgs) { + super(new ConfigOpts(), sserverArgs.toArray(new String[] {})); + this.behavior = behavior; + } - final Implementation implementation; + @Override + public void updateIdleStatus(boolean isIdle) { switch (behavior) { case ERROR: - implementation = - ExceptionMethod.throwing(StackOverflowError.class, "throwing unknown error"); + throw new StackOverflowError("throwing stack overflow error"); + case EXCEPTION: + throw new RuntimeException("throwing runtime exception"); + case SHUTDOWN: + requestShutdownForTests(); break; + default: + throw new UnsupportedOperationException(behavior + " is not currently supported"); + } + } + } + + public static class ExitTabletServer extends TabletServer { + + public static void main(String[] args) throws Exception { + List tserverArgs = new ArrayList<>(); + tserverArgs.add("-o"); + tserverArgs.add(Property.TSERV_GROUP_NAME.getKey() + "=TEST"); + AbstractServer.startServer(new ExitTabletServer(getTerminalBehavior(), tserverArgs), + LoggerFactory.getLogger(ExitTabletServer.class)); + } + + private final TerminalBehavior behavior; + + public ExitTabletServer(TerminalBehavior behavior, List tserverArgs) { + super(new ConfigOpts(), ServerContext::new, tserverArgs.toArray(new String[] {})); + this.behavior = behavior; + } + + @Override + public void updateIdleStatus(boolean isIdle) { + switch (behavior) { + case ERROR: + throw new StackOverflowError("throwing stack overflow error"); case EXCEPTION: - implementation = - ExceptionMethod.throwing(RuntimeException.class, "throwing runtime exception"); + throw new RuntimeException("throwing runtime exception"); + case SHUTDOWN: + requestShutdownForTests(); break; + default: + throw new UnsupportedOperationException(behavior + " is not currently supported"); + } + } + } + + public static class ExitGC extends SimpleGarbageCollector { + + public static void main(String[] args) throws Exception { + AbstractServer.startServer(new ExitGC(getTerminalBehavior()), + LoggerFactory.getLogger(ExitGC.class)); + } + + private final TerminalBehavior behavior; + + public ExitGC(TerminalBehavior behavior) { + super(new ConfigOpts(), new String[] {}); + this.behavior = behavior; + } + + @Override + public void logStats() { + switch (behavior) { + case ERROR: + throw new StackOverflowError("throwing stack overflow error"); + case EXCEPTION: + throw new RuntimeException("throwing runtime exception"); case SHUTDOWN: - implementation = - MethodCall.invoke(AbstractServer.class.getMethod("requestShutdownForTests")); + requestShutdownForTests(); break; default: throw new UnsupportedOperationException(behavior + " is not currently supported"); } + } + } + + public static class ExitManager extends Manager { + + public static void main(String[] args) throws Exception { + AbstractServer.startServer(new ExitManager(getTerminalBehavior()), + LoggerFactory.getLogger(ExitManager.class)); + } + + private final TerminalBehavior behavior; - // Dynamically create the subclass with the specified behavior, load it - // into the JVM, and return it's Class. - // creates a subclass of server class defining a public constructor that - // takes an Integer, but calls the server class constructor with the - // appropriate args. Calls to the intercepted method name are intercepted - // by the supplied implementation - final Class dynamicServerType = - new ByteBuddy().subclass(serverClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) - .name("org.apache.accumulo.test.functional.exit_codes." + serverClass.getSimpleName() - + "_" + behavior.name()) - .defineConstructor(Visibility.PUBLIC).withParameters(Integer.class) - .intercept(MethodCall.invoke(serverClass.getDeclaredConstructor(ctorParams)).onSuper() - .with(ctorArgs)) - .method(ElementMatchers.named(methodName)).intercept(implementation).make() - .load(serverClass.getClassLoader()).getLoaded(); - - // Find and invoke the constructor of the dynamically created class - final Constructor ctor = - dynamicServerType.getDeclaredConstructor(Integer.class); - final AbstractServer dynamicServerInstance = ctor.newInstance(1); - - // Start the server process - AbstractServer.startServer(dynamicServerInstance, LoggerFactory.getLogger(serverClass)); + public ExitManager(TerminalBehavior behavior) throws IOException { + super(new ConfigOpts(), ServerContext::new, new String[] {}); + this.behavior = behavior; } + @Override + public void mainWait() throws InterruptedException { + switch (behavior) { + case ERROR: + throw new StackOverflowError("throwing stack overflow error"); + case EXCEPTION: + throw new RuntimeException("throwing runtime exception"); + case SHUTDOWN: + requestShutdownForTests(); + break; + default: + throw new UnsupportedOperationException(behavior + " is not currently supported"); + } + } } @BeforeAll @@ -233,35 +260,40 @@ static Stream generateWorkerProcessArguments() { @MethodSource("generateWorkerProcessArguments") public void testWorkerProcesses(ServerType server, TerminalBehavior behavior) throws Exception { Map properties = new HashMap<>(); - properties.put(ProcessProxy.PROXY_CLASS, server.name()); - properties.put(ProcessProxy.PROXY_METHOD_BEHAVIOR, behavior.name()); + properties.put(PROXY_METHOD_BEHAVIOR, behavior.name()); getCluster().getConfig().setSystemProperties(properties); - ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); + Class serverClass = null; + switch (server) { + case COMPACTOR: + serverClass = ExitCompactor.class; + break; + case SCAN_SERVER: + serverClass = ExitScanServer.class; + break; + case TABLET_SERVER: + serverClass = ExitTabletServer.class; + break; + case GARBAGE_COLLECTOR: + case MANAGER: + case MONITOR: + case ZOOKEEPER: + default: + throw new IllegalArgumentException("Unhandled type"); + } + ProcessInfo pi = getCluster()._exec(serverClass, server, Map.of(), new String[] {}); Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); int exitValue = pi.getProcess().exitValue(); assertEquals(behavior == TerminalBehavior.SHUTDOWN ? 0 : 1, exitValue); } - static Stream generateGarbageCollectorArguments() { - List args = new ArrayList<>(); - for (ServerType st : ServerType.values()) { - if (st == ServerType.GARBAGE_COLLECTOR) { - for (TerminalBehavior tb : TerminalBehavior.values()) { - args.add(Arguments.of(st, tb)); - } - } - } - return args.stream(); - } - @ParameterizedTest - @MethodSource("generateGarbageCollectorArguments") - public void testGarbageCollector(ServerType server, TerminalBehavior behavior) throws Exception { + @EnumSource + public void testGarbageCollector(TerminalBehavior behavior) throws Exception { Map properties = new HashMap<>(); - properties.put(ProcessProxy.PROXY_CLASS, server.name()); - properties.put(ProcessProxy.PROXY_METHOD_BEHAVIOR, behavior.name()); + properties.put(PROXY_METHOD_BEHAVIOR, behavior.name()); getCluster().getConfig().setSystemProperties(properties); - ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); + ProcessInfo pi = + getCluster()._exec(ExitGC.class, ServerType.GARBAGE_COLLECTOR, Map.of(), new String[] {}); if (behavior == TerminalBehavior.SHUTDOWN) { Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); int exitValue = pi.getProcess().exitValue(); @@ -281,28 +313,16 @@ public void testGarbageCollector(ServerType server, TerminalBehavior behavior) t } } - static Stream generateManagerArguments() { - List args = new ArrayList<>(); - for (ServerType st : ServerType.values()) { - if (st == ServerType.MANAGER) { - for (TerminalBehavior tb : TerminalBehavior.values()) { - args.add(Arguments.of(st, tb)); - } - } - } - return args.stream(); - } - @ParameterizedTest - @MethodSource("generateManagerArguments") - public void testManager(ServerType server, TerminalBehavior behavior) throws Exception { + @EnumSource + public void testManager(TerminalBehavior behavior) throws Exception { try { getCluster().getClusterControl().stop(ServerType.MANAGER); Map properties = new HashMap<>(); - properties.put(ProcessProxy.PROXY_CLASS, server.name()); - properties.put(ProcessProxy.PROXY_METHOD_BEHAVIOR, behavior.name()); + properties.put(PROXY_METHOD_BEHAVIOR, behavior.name()); getCluster().getConfig().setSystemProperties(properties); - ProcessInfo pi = getCluster()._exec(ProcessProxy.class, server, Map.of(), new String[] {}); + ProcessInfo pi = + getCluster()._exec(ExitManager.class, ServerType.MANAGER, Map.of(), new String[] {}); Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000); int exitValue = pi.getProcess().exitValue(); assertEquals(behavior == TerminalBehavior.SHUTDOWN ? 0 : 1, exitValue); From 62598852f7755d611b964f8740654d959f420003 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 27 Aug 2025 17:39:35 +0000 Subject: [PATCH 9/9] Changed GC wait time from 2m to 1m --- .../java/org/apache/accumulo/test/functional/ExitCodesIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java index e5bbc48376e..e37ef299752 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java @@ -303,7 +303,7 @@ public void testGarbageCollector(TerminalBehavior behavior) throws Exception { // We need to let this time out and then // terminate the process. IllegalStateException ise = assertThrows(IllegalStateException.class, - () -> Wait.waitFor(() -> !pi.getProcess().isAlive(), 120_000)); + () -> Wait.waitFor(() -> !pi.getProcess().isAlive(), 60_000)); assertTrue(ise.getMessage().contains("Timeout exceeded")); pi.getProcess().destroyForcibly(); } else {