From a5ddec2f49d12515e275daccb6f11350e50dfd32 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 8 Jul 2025 18:46:28 +0000 Subject: [PATCH 01/19] Cause Compactor to exit when error loading classes AccumuloVFSClassLoader.getContextClassLoader throws an UncheckedIOException when there is an issue getting the ClassLoader for a context name. This runtime exception escapes all the way up to the calling code in Compactor and TabletServer, which fails the compaction and moves on to the next compaction. If there is an issue getting the ClassLoader for the context once, then it's likely to happen again. It's probably not safe to terminate the TabletServer in this case, but is likely safe for the Compactor. This change captures the RuntimeException in the FileCompactor.compactLocalityGroup where the iterator stack is created and raises a new checked exception which is handled in the calling code. --- .../server/compaction/FileCompactor.java | 38 ++++++++++++++++--- .../apache/accumulo/compactor/Compactor.java | 13 +++++++ .../tserver/tablet/CompactableUtils.java | 5 ++- .../tserver/tablet/MinorCompactor.java | 3 +- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java b/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java index 1f655dd3040..95a31091455 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java +++ b/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java @@ -97,6 +97,27 @@ public static class CompactionCanceledException extends Exception { private static final long serialVersionUID = 1L; } + public static class CompactionClassLoadingException extends Exception { + private static final long serialVersionUID = 1L; + + public CompactionClassLoadingException() { + super(); + } + + public CompactionClassLoadingException(String message, Throwable cause) { + super(message, cause); + } + + public CompactionClassLoadingException(String message) { + super(message); + } + + public CompactionClassLoadingException(Throwable cause) { + super(cause); + } + + } + public interface CompactionEnv { boolean isCompactionEnabled(); @@ -312,8 +333,8 @@ protected Map> getLocalityGroups(AccumuloConfiguration } @Override - public CompactionStats call() - throws IOException, CompactionCanceledException, InterruptedException { + public CompactionStats call() throws IOException, CompactionCanceledException, + InterruptedException, CompactionClassLoadingException { FileSKVWriter mfw = null; @@ -539,7 +560,8 @@ private List> openMapDataFiles( private void compactLocalityGroup(String lgName, Set columnFamilies, boolean inclusive, FileSKVWriter mfw, CompactionStats majCStats, - EnumSet dropCacheFilePrefixes) throws IOException, CompactionCanceledException { + EnumSet dropCacheFilePrefixes) + throws IOException, CompactionCanceledException, CompactionClassLoadingException { ArrayList readers = new ArrayList<>(filesToCompact.size()); Span compactSpan = TraceUtil.startSpan(this.getClass(), "compact"); try (Scope span = compactSpan.makeCurrent()) { @@ -560,9 +582,15 @@ private void compactLocalityGroup(String lgName, Set columnFamilie SystemIteratorEnvironment iterEnv = env.createIteratorEnv(context, acuTableConf, getExtent().tableId()); - SortedKeyValueIterator itr = iterEnv.getTopLevelIterator(IteratorConfigUtil - .convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf, iterators, iterEnv)); + SortedKeyValueIterator stack = null; + try { + stack = IteratorConfigUtil.convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf, + iterators, iterEnv); + } catch (RuntimeException e) { + throw new CompactionClassLoadingException("Error loading iterators", e); + } + SortedKeyValueIterator itr = iterEnv.getTopLevelIterator(stack); itr.seek(extent.toDataRange(), columnFamilies, inclusive); if (inclusive) { 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 ce18a0ef9ea..0f49f6f5326 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 @@ -105,6 +105,7 @@ import org.apache.accumulo.server.compaction.CompactionInfo; import org.apache.accumulo.server.compaction.CompactionWatcher; import org.apache.accumulo.server.compaction.FileCompactor; +import org.apache.accumulo.server.compaction.FileCompactor.CompactionClassLoadingException; import org.apache.accumulo.server.compaction.RetryableThriftCall; import org.apache.accumulo.server.compaction.RetryableThriftCall.RetriesExceededException; import org.apache.accumulo.server.conf.TableConfiguration; @@ -582,6 +583,9 @@ public void run() { TCompactionStatusUpdate update2 = new TCompactionStatusUpdate(TCompactionState.SUCCEEDED, "Compaction completed successfully", -1, -1, -1, this.getCompactionAge().toNanos()); updateCompactionState(job, update2); + } catch (FileCompactor.CompactionClassLoadingException ccle) { + LOG.error("Error loading classes for compaction", ccle); + err.set(ccle); } catch (FileCompactor.CompactionCanceledException cce) { LOG.debug("Compaction canceled {}", job.getExternalCompactionId()); err.set(cce); @@ -825,6 +829,15 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job); + if (err.get() instanceof CompactionClassLoadingException) { + // Compaction failed because it could not load classes. + // Raise an InterruptedException here which will be caught + // below to exit the Compactor. If we don't exit, then this + // Compactor will get the next job and continue to fail. + LOG.error( + "CompactionClassLoadingException occurred. Check iterator class configuration. Exiting Compactor."); + throw new InterruptedException(); + } } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", job.getExternalCompactionId(), fromThriftExtent, e); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java index 56a2006456c..f76a465db89 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java @@ -78,6 +78,7 @@ import org.apache.accumulo.server.compaction.CompactionStats; import org.apache.accumulo.server.compaction.FileCompactor; import org.apache.accumulo.server.compaction.FileCompactor.CompactionCanceledException; +import org.apache.accumulo.server.compaction.FileCompactor.CompactionClassLoadingException; import org.apache.accumulo.server.compaction.FileCompactor.CompactionEnv; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.fs.VolumeManager; @@ -559,8 +560,8 @@ private static AccumuloConfiguration getCompactionConfig(TableConfiguration tabl */ static CompactionStats compact(Tablet tablet, CompactionJob job, CompactableImpl.CompactionInfo cInfo, CompactionEnv cenv, - Map compactFiles, TabletFile tmpFileName) - throws IOException, CompactionCanceledException, InterruptedException { + Map compactFiles, TabletFile tmpFileName) throws IOException, + CompactionCanceledException, CompactionClassLoadingException, InterruptedException { TableConfiguration tableConf = tablet.getTableConfiguration(); AccumuloConfiguration compactionConfig = getCompactionConfig(tableConf, 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 f265056c6a5..bd69bfa2e6c 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 @@ -137,7 +137,8 @@ public CompactionStats call() { log.warn("MinC failed ({}) to create {} retrying ...", e.getMessage(), outputFileName, e); reportedProblem = true; retryCounter++; - } catch (CompactionCanceledException | InterruptedException e) { + } catch (CompactionCanceledException | CompactionClassLoadingException + | InterruptedException e) { throw new IllegalStateException(e); } From 825df0df3bdfed2ec0d075efa2c963d0d0976463 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 9 Jul 2025 15:11:47 +0000 Subject: [PATCH 02/19] Added IT --- .../ClassLoaderContextCompactionIT.java | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java diff --git a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java new file mode 100644 index 00000000000..7727aa6e71f --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java @@ -0,0 +1,133 @@ +/* + * 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.compaction; + +import static org.apache.accumulo.core.conf.Property.TABLE_FILE_MAX; +import static org.apache.accumulo.core.conf.Property.TABLE_MAJC_RATIO; +import static org.apache.accumulo.test.compaction.ExternalCompactionTestUtils.QUEUE1; +import static org.apache.accumulo.test.compaction.ExternalCompactionTestUtils.createTable; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.EnumSet; + +import org.apache.accumulo.compactor.Compactor; +import org.apache.accumulo.coordinator.CompactionCoordinator; +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.admin.CompactionConfig; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; +import org.apache.accumulo.core.metadata.schema.Ample; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; +import org.apache.accumulo.core.metadata.schema.TabletsMetadata; +import org.apache.accumulo.core.util.compaction.ExternalCompactionUtil; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.junit.jupiter.api.Test; + +public class ClassLoaderContextCompactionIT extends AccumuloClusterHarness { + + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + ExternalCompactionTestUtils.configureMiniCluster(cfg, coreSite); + cfg.setNumCompactors(2); + } + + @Test + public void testClassLoaderContextErrorKillsCompactor() throws Exception { + final String table1 = this.getUniqueNames(1)[0]; + try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) { + getCluster().getClusterControl().startCoordinator(CompactionCoordinator.class); + getCluster().getClusterControl().startCompactors(Compactor.class, 1, QUEUE1); + Wait.waitFor( + () -> ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client) == 1); + createTable(client, table1, "cs1"); + client.tableOperations().setProperty(table1, TABLE_FILE_MAX.getKey(), "1001"); + client.tableOperations().setProperty(table1, TABLE_MAJC_RATIO.getKey(), "1001"); + TableId tid = TableId.of(client.tableOperations().tableIdMap().get(table1)); + + ReadWriteIT.ingest(client, 1000, 1, 1, 0, "colf", table1, 20); + + Ample ample = ((ClientContext) client).getAmple(); + try ( + TabletsMetadata tms = ample.readTablets().forTable(tid).fetch(ColumnType.FILES).build()) { + TabletMetadata tm = tms.iterator().next(); + assertEquals(50, tm.getFiles().size()); + } + + final MiniAccumuloClusterImpl cluster = (MiniAccumuloClusterImpl) getCluster(); + final FileSystem fs = cluster.getFileSystem(); + + // Create the context directory in HDFS + final org.apache.hadoop.fs.Path contextDir = fs.makeQualified(new org.apache.hadoop.fs.Path( + cluster.getConfig().getAccumuloDir().toString(), "classpath")); + assertTrue(fs.mkdirs(contextDir)); + + // Copy the FooFilter.jar to the context dir + final org.apache.hadoop.fs.Path src = new org.apache.hadoop.fs.Path( + System.getProperty("java.io.tmpdir") + "/classes/org/apache/accumulo/test/FooFilter.jar"); + final org.apache.hadoop.fs.Path dst = new org.apache.hadoop.fs.Path(contextDir, "Test.jar"); + fs.copyFromLocalFile(src, dst); + assertTrue(fs.exists(dst)); + + // Define a classloader context that references Test.jar + @SuppressWarnings("removal") + final Property p = Property.VFS_CONTEXT_CLASSPATH_PROPERTY; + client.instanceOperations().setProperty(p.getKey() + "undefined", dst.toUri().toString()); + + // Force the classloader to look in the context jar first, don't delegate to the parent first + client.instanceOperations().setProperty("general.vfs.context.classpath.undefined.delegation", + "post"); + + // Set the context on the table + client.tableOperations().setProperty(table1, Property.TABLE_CLASSLOADER_CONTEXT.getKey(), + "undefined"); + + final IteratorSetting cfg = + new IteratorSetting(101, "FooFilter", "org.apache.accumulo.test.FooFilter"); + client.tableOperations().attachIterator(table1, cfg, EnumSet.of(IteratorScope.majc)); + + // delete Test.jar, so that the classloader will fail + assertTrue(fs.delete(dst, false)); + + // Start a compaction. The invalid cache dir property should cause a failure + client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); + + Wait.waitFor( + () -> ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client) == 0); + + } finally { + getCluster().getClusterControl().stopAllServers(ServerType.COMPACTOR); + getCluster().getClusterControl().stopAllServers(ServerType.COMPACTION_COORDINATOR); + } + + } + +} From 8695400fff504009a4998d7797dccbd7e9984dea Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 11 Jul 2025 18:04:02 +0000 Subject: [PATCH 03/19] Changes to propogate IOE and ROE up to classloader callers --- .../core/classloader/ClassLoaderUtil.java | 11 +++--- .../DefaultContextClassLoaderFactory.java | 3 +- .../client/ClientSideIteratorScanner.java | 2 +- .../core/client/rfile/RFileScanner.java | 2 +- .../core/clientImpl/OfflineIterator.java | 8 ++--- .../core/iterators/TypedValueCombiner.java | 2 +- .../iteratorsImpl/IteratorConfigUtil.java | 9 ++--- .../core/sample/impl/SamplerFactory.java | 2 +- .../spi/common/ContextClassLoaderFactory.java | 4 ++- .../core/spi/crypto/CryptoServiceFactory.java | 3 +- .../ContextClassLoaderFactoryTest.java | 3 +- .../accumulo/core/crypto/CryptoTest.java | 2 +- .../iteratorsImpl/IteratorConfigUtilTest.java | 13 +++---- .../server/client/ClientServiceHandler.java | 2 +- .../server/compaction/FileCompactor.java | 36 +++---------------- .../server/metrics/MetricsInfoImpl.java | 5 +-- .../apache/accumulo/compactor/Compactor.java | 13 ------- .../tserver/ConditionCheckerContext.java | 7 ++-- .../accumulo/tserver/TabletClientHandler.java | 6 ++-- .../tserver/tablet/CompactableUtils.java | 3 +- .../tserver/tablet/MinorCompactor.java | 5 ++- .../tserver/tablet/ScanDataSource.java | 9 +++-- .../accumulo/tserver/tablet/Tablet.java | 4 +-- .../java/org/apache/accumulo/shell/Shell.java | 3 +- .../shell/commands/SetIterCommand.java | 2 +- .../shell/commands/SetScanIterCommand.java | 2 +- .../shell/commands/SetShellIterCommand.java | 2 +- .../vfs/AccumuloVFSClassLoader.java | 30 ++++++++-------- .../performance/scan/CollectTabletStats.java | 2 +- 29 files changed, 86 insertions(+), 109 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java index 7af94627b0c..5ecc498f2db 100644 --- a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java @@ -18,6 +18,8 @@ */ package org.apache.accumulo.core.classloader; +import java.io.IOException; + import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory; @@ -74,7 +76,8 @@ public static synchronized void resetContextFactoryForTests() { } @SuppressWarnings("deprecation") - public static ClassLoader getClassLoader(String context) { + public static ClassLoader getClassLoader(String context) + throws IOException, ReflectiveOperationException { if (context != null && !context.isEmpty()) { return FACTORY.getClassLoader(context); } else { @@ -92,7 +95,7 @@ public static boolean isValidContext(String context) { return false; } return true; - } catch (RuntimeException e) { + } catch (IOException | ReflectiveOperationException e) { LOG.debug("Context {} is not valid.", context, e); return false; } @@ -102,12 +105,12 @@ public static boolean isValidContext(String context) { } public static Class loadClass(String context, String className, - Class extension) throws ClassNotFoundException { + Class extension) throws IOException, ReflectiveOperationException { return getClassLoader(context).loadClass(className).asSubclass(extension); } public static Class loadClass(String className, Class extension) - throws ClassNotFoundException { + throws IOException, ReflectiveOperationException { return loadClass(null, className, extension); } diff --git a/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java b/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java index 1bd5fcb6701..be22708c15e 100644 --- a/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java @@ -20,6 +20,7 @@ import static java.util.concurrent.TimeUnit.MINUTES; +import java.io.IOException; import java.util.Map; import java.util.Set; import java.util.concurrent.ScheduledFuture; @@ -97,7 +98,7 @@ private static void removeUnusedContexts(Set contextsInUse) { @SuppressWarnings("deprecation") @Override - public ClassLoader getClassLoader(String contextName) { + public ClassLoader getClassLoader(String contextName) throws IOException { return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader .getContextClassLoader(contextName); } diff --git a/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java b/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java index 8a646219511..0209e8deece 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java +++ b/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java @@ -242,7 +242,7 @@ public Iterator> iterator() { IteratorBuilder.builder(tm.values()).opts(serverSideIteratorOptions).env(iterEnv).build(); skvi = IteratorConfigUtil.loadIterators(smi, ib); - } catch (IOException e) { + } catch (IOException | ReflectiveOperationException e) { throw new RuntimeException(e); } diff --git a/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java b/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java index e34730ff360..9266a53e510 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java +++ b/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java @@ -320,7 +320,7 @@ public Iterator> iterator() { .opts(serverSideIteratorOptions).env(iterEnv).build(); iterator = IteratorConfigUtil.loadIterators(iterator, iteratorBuilder); } - } catch (IOException e) { + } catch (IOException | ReflectiveOperationException e) { throw new RuntimeException(e); } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java index d64e0181487..7ec99bd9c49 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java @@ -137,8 +137,8 @@ public Entry next() { } } - private void nextTablet() - throws TableNotFoundException, AccumuloException, IOException, AccumuloSecurityException { + private void nextTablet() throws TableNotFoundException, AccumuloException, IOException, + AccumuloSecurityException, ReflectiveOperationException { Range nextRange; @@ -204,8 +204,8 @@ private TabletMetadata getTabletFiles(Range nextRange) { } private SortedKeyValueIterator createIterator(KeyExtent extent, - Collection absFiles) - throws TableNotFoundException, AccumuloException, IOException, AccumuloSecurityException { + Collection absFiles) throws TableNotFoundException, AccumuloException, + IOException, AccumuloSecurityException, ReflectiveOperationException { // possible race condition here, if table is renamed String tableName = context.getTableName(tableId); diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java b/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java index 12656720a87..df0cc359636 100644 --- a/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java +++ b/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java @@ -132,7 +132,7 @@ protected void setEncoder(String encoderClass) { Class> clazz = (Class>) ClassLoaderUtil.loadClass(encoderClass, Encoder.class); encoder = clazz.getDeclaredConstructor().newInstance(); - } catch (ReflectiveOperationException e) { + } catch (IOException | ReflectiveOperationException e) { throw new IllegalArgumentException(e); } } diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java index 93c098d0efd..85369129966 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java @@ -174,7 +174,8 @@ public static IteratorBuilder.IteratorBuilderEnv loadIterConf(IteratorScope scop */ public static SortedKeyValueIterator convertItersAndLoad(IteratorScope scope, SortedKeyValueIterator source, AccumuloConfiguration conf, - List iterators, IteratorEnvironment env) throws IOException { + List iterators, IteratorEnvironment env) + throws IOException, ReflectiveOperationException { List ssiList = new ArrayList<>(); Map> ssio = new HashMap<>(); @@ -194,7 +195,7 @@ public static SortedKeyValueIterator convertItersAndLoad(IteratorScop */ public static SortedKeyValueIterator loadIterators(SortedKeyValueIterator source, IteratorBuilder iteratorBuilder) - throws IOException { + throws IOException, ReflectiveOperationException { SortedKeyValueIterator prev = source; final boolean useClassLoader = iteratorBuilder.useAccumuloClassLoader; Map>> classCache = new HashMap<>(); @@ -228,14 +229,14 @@ public static SortedKeyValueIterator convertItersAndLoad(IteratorScop } catch (ReflectiveOperationException e) { log.error("Failed to load iterator {}, for table {}, from context {}", iterInfo.className, iteratorBuilder.iteratorEnvironment.getTableId(), iteratorBuilder.context, e); - throw new RuntimeException(e); + throw e; } } return prev; } private static Class> loadClass(boolean useAccumuloClassLoader, - String context, IterInfo iterInfo) throws ClassNotFoundException { + String context, IterInfo iterInfo) throws IOException, ReflectiveOperationException { if (useAccumuloClassLoader) { @SuppressWarnings("unchecked") var clazz = (Class>) ClassLoaderUtil.loadClass(context, diff --git a/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java b/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java index 7d7e1ab0cb1..7f2d289879a 100644 --- a/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java @@ -48,7 +48,7 @@ public static Sampler newSampler(SamplerConfigurationImpl config, AccumuloConfig return sampler; - } catch (ReflectiveOperationException | RuntimeException e) { + } catch (ReflectiveOperationException | IOException | RuntimeException e) { log.error("Cannot initialize sampler {}: {}", config.getClassName(), e.getMessage(), e); return null; } diff --git a/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java b/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java index 3d9c18683f5..481f5ae4739 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java @@ -18,6 +18,8 @@ */ package org.apache.accumulo.core.spi.common; +import java.io.IOException; + /** * The ContextClassLoaderFactory provides a mechanism for various Accumulo components to use a * custom ClassLoader for specific contexts, such as loading table iterators. This factory is @@ -66,5 +68,5 @@ default void init(ContextClassLoaderEnvironment env) {} * consulting this plugin. * @return the class loader for the given contextName */ - ClassLoader getClassLoader(String contextName); + ClassLoader getClassLoader(String contextName) throws IOException, ReflectiveOperationException; } diff --git a/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java b/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java index d3683326b17..4fc6af3585a 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java @@ -18,6 +18,7 @@ */ package org.apache.accumulo.core.spi.crypto; +import java.io.IOException; import java.util.Map; import org.apache.accumulo.core.classloader.ClassLoaderUtil; @@ -46,7 +47,7 @@ default CryptoService newCryptoService(String cryptoServiceName) { try { return ClassLoaderUtil.loadClass(null, cryptoServiceName, CryptoService.class) .getDeclaredConstructor().newInstance(); - } catch (ReflectiveOperationException e) { + } catch (ReflectiveOperationException | IOException e) { throw new RuntimeException(e); } } diff --git a/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java b/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java index 83b3821640e..25f0c4390ab 100644 --- a/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java +++ b/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; +import java.io.IOException; import java.net.URLClassLoader; import java.util.Objects; @@ -64,7 +65,7 @@ public void setup() throws Exception { } @Test - public void differentContexts() { + public void differentContexts() throws IOException, ReflectiveOperationException { ConfigurationCopy cc = new ConfigurationCopy(); cc.set(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY.getKey(), diff --git a/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java b/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java index 08ef7f660fd..43a04aa53d5 100644 --- a/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java +++ b/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java @@ -388,7 +388,7 @@ public void testReadNoCryptoWithCryptoConfigured() throws Exception { } @Test - public void testMissingConfigProperties() throws ReflectiveOperationException { + public void testMissingConfigProperties() throws ReflectiveOperationException, IOException { var cryptoProps = getAllCryptoProperties(ConfigMode.CRYPTO_TABLE_ON); var droppedProperty = cryptoProps.remove(AESCryptoService.KEY_URI_PROPERTY); assertNotNull(droppedProperty); diff --git a/core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java b/core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java index 251a3441b36..3042a9a0a53 100644 --- a/core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java +++ b/core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java @@ -136,14 +136,15 @@ public Value getTopValue() { } private SortedKeyValueIterator createIter(IteratorScope scope, - SortedMapIterator source, AccumuloConfiguration conf) throws IOException { + SortedMapIterator source, AccumuloConfiguration conf) + throws IOException, ReflectiveOperationException { var ibEnv = IteratorConfigUtil.loadIterConf(scope, EMPTY_ITERS, new HashMap<>(), conf); var iteratorBuilder = ibEnv.env(ClientIteratorEnvironment.DEFAULT).useClassLoader(null).build(); return IteratorConfigUtil.loadIterators(source, iteratorBuilder); } @Test - public void test1() throws IOException { + public void test1() throws IOException, ReflectiveOperationException { ConfigurationCopy conf = new ConfigurationCopy(); // create an iterator that adds 1 and then squares @@ -177,7 +178,7 @@ public void test1() throws IOException { } @Test - public void test4() throws IOException { + public void test4() throws IOException, ReflectiveOperationException { // try loading for a different scope AccumuloConfiguration conf = new ConfigurationCopy(); @@ -209,7 +210,7 @@ public void test4() throws IOException { } @Test - public void test3() throws IOException { + public void test3() throws IOException, ReflectiveOperationException { // change the load order, so it squares and then adds ConfigurationCopy conf = new ConfigurationCopy(); @@ -245,7 +246,7 @@ public void test3() throws IOException { } @Test - public void test2() throws IOException { + public void test2() throws IOException, ReflectiveOperationException { ConfigurationCopy conf = new ConfigurationCopy(); @@ -284,7 +285,7 @@ public void test2() throws IOException { } @Test - public void test5() throws IOException { + public void test5() throws IOException, ReflectiveOperationException { ConfigurationCopy conf = new ConfigurationCopy(); // create an iterator that ages off diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java index 2edf8a55c29..5e8d8aa3ed5 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java @@ -445,7 +445,7 @@ public boolean checkClass(TInfo tinfo, TCredentials credentials, String classNam Class test = ClassLoaderUtil.loadClass(className, shouldMatch); test.getDeclaredConstructor().newInstance(); return true; - } catch (ClassCastException | ReflectiveOperationException e) { + } catch (ClassCastException | ReflectiveOperationException | IOException e) { log.warn("Error checking object types", e); return false; } diff --git a/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java b/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java index 95a31091455..c9352473db2 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java +++ b/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java @@ -97,27 +97,6 @@ public static class CompactionCanceledException extends Exception { private static final long serialVersionUID = 1L; } - public static class CompactionClassLoadingException extends Exception { - private static final long serialVersionUID = 1L; - - public CompactionClassLoadingException() { - super(); - } - - public CompactionClassLoadingException(String message, Throwable cause) { - super(message, cause); - } - - public CompactionClassLoadingException(String message) { - super(message); - } - - public CompactionClassLoadingException(Throwable cause) { - super(cause); - } - - } - public interface CompactionEnv { boolean isCompactionEnabled(); @@ -334,7 +313,7 @@ protected Map> getLocalityGroups(AccumuloConfiguration @Override public CompactionStats call() throws IOException, CompactionCanceledException, - InterruptedException, CompactionClassLoadingException { + InterruptedException, ReflectiveOperationException { FileSKVWriter mfw = null; @@ -561,7 +540,7 @@ private List> openMapDataFiles( private void compactLocalityGroup(String lgName, Set columnFamilies, boolean inclusive, FileSKVWriter mfw, CompactionStats majCStats, EnumSet dropCacheFilePrefixes) - throws IOException, CompactionCanceledException, CompactionClassLoadingException { + throws IOException, CompactionCanceledException, ReflectiveOperationException { ArrayList readers = new ArrayList<>(filesToCompact.size()); Span compactSpan = TraceUtil.startSpan(this.getClass(), "compact"); try (Scope span = compactSpan.makeCurrent()) { @@ -582,15 +561,8 @@ private void compactLocalityGroup(String lgName, Set columnFamilie SystemIteratorEnvironment iterEnv = env.createIteratorEnv(context, acuTableConf, getExtent().tableId()); - SortedKeyValueIterator stack = null; - try { - stack = IteratorConfigUtil.convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf, - iterators, iterEnv); - } catch (RuntimeException e) { - throw new CompactionClassLoadingException("Error loading iterators", e); - } - - SortedKeyValueIterator itr = iterEnv.getTopLevelIterator(stack); + SortedKeyValueIterator itr = iterEnv.getTopLevelIterator(IteratorConfigUtil + .convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf, iterators, iterEnv)); itr.seek(extent.toDataRange(), columnFamilies, inclusive); if (inclusive) { diff --git a/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java index b8434828a60..22d9e0002db 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.util.StringUtils.getTrimmedStrings; +import java.io.IOException; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -160,7 +161,7 @@ public DistributionStatisticConfig configure(Meter.Id id, registry.config().meterFilter(replicationFilter); registry.config().commonTags(commonTags); Metrics.globalRegistry.add(registry); - } catch (ReflectiveOperationException ex) { + } catch (ReflectiveOperationException | IOException ex) { LOG.warn("Could not load registry {}", factoryName, ex); } } @@ -206,7 +207,7 @@ public DistributionStatisticConfig configure(Meter.Id id, @VisibleForTesting @SuppressWarnings("deprecation") static MeterRegistry getRegistryFromFactory(final String factoryName, final ServerContext context) - throws ReflectiveOperationException { + throws IOException, ReflectiveOperationException { try { LOG.info("look for meter spi registry factory {}", factoryName); Class clazz = 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 0f49f6f5326..ce18a0ef9ea 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 @@ -105,7 +105,6 @@ import org.apache.accumulo.server.compaction.CompactionInfo; import org.apache.accumulo.server.compaction.CompactionWatcher; import org.apache.accumulo.server.compaction.FileCompactor; -import org.apache.accumulo.server.compaction.FileCompactor.CompactionClassLoadingException; import org.apache.accumulo.server.compaction.RetryableThriftCall; import org.apache.accumulo.server.compaction.RetryableThriftCall.RetriesExceededException; import org.apache.accumulo.server.conf.TableConfiguration; @@ -583,9 +582,6 @@ public void run() { TCompactionStatusUpdate update2 = new TCompactionStatusUpdate(TCompactionState.SUCCEEDED, "Compaction completed successfully", -1, -1, -1, this.getCompactionAge().toNanos()); updateCompactionState(job, update2); - } catch (FileCompactor.CompactionClassLoadingException ccle) { - LOG.error("Error loading classes for compaction", ccle); - err.set(ccle); } catch (FileCompactor.CompactionCanceledException cce) { LOG.debug("Compaction canceled {}", job.getExternalCompactionId()); err.set(cce); @@ -829,15 +825,6 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job); - if (err.get() instanceof CompactionClassLoadingException) { - // Compaction failed because it could not load classes. - // Raise an InterruptedException here which will be caught - // below to exit the Compactor. If we don't exit, then this - // Compactor will get the next job and continue to fail. - LOG.error( - "CompactionClassLoadingException occurred. Check iterator class configuration. Exiting Compactor."); - throw new InterruptedException(); - } } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", job.getExternalCompactionId(), fromThriftExtent, e); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java index f395c4134ad..cb98c24d35b 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java @@ -86,7 +86,7 @@ private static class MergedIterConfig { } SortedKeyValueIterator buildIterator(SortedKeyValueIterator systemIter, - TCondition tc) throws IOException { + TCondition tc) throws IOException, ReflectiveOperationException { ArrayByteSequence key = new ArrayByteSequence(tc.iterators); MergedIterConfig mic = mergedIterCache.get(key); @@ -111,7 +111,7 @@ SortedKeyValueIterator buildIterator(SortedKeyValueIterator systemIter, - ServerConditionalMutation scm) throws IOException { + ServerConditionalMutation scm) throws IOException, ReflectiveOperationException { boolean add = true; for (TCondition tc : scm.getConditions()) { @@ -157,7 +157,8 @@ public ConditionChecker(List conditionsToCheck, this.results = results; } - public void check(SortedKeyValueIterator systemIter) throws IOException { + public void check(SortedKeyValueIterator systemIter) + throws IOException, ReflectiveOperationException { checkArgument(!checked, "check() method should only be called once"); checked = true; diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java index 97dad09224b..571acb5a3fd 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java @@ -751,7 +751,7 @@ private NamespaceId getNamespaceId(TCredentials credentials, TableId tableId) private void checkConditions(Map> updates, ArrayList results, ConditionalSession cs, List symbols) - throws IOException { + throws IOException, ReflectiveOperationException { Iterator>> iter = updates.entrySet().iterator(); final CompressedIterators compressedIters = new CompressedIterators(symbols); @@ -905,7 +905,7 @@ private void addMutationsAsTCMResults(final List list, private Map> conditionalUpdate(ConditionalSession cs, Map> updates, ArrayList results, - List symbols) throws IOException { + List symbols) throws IOException, ReflectiveOperationException { // sort each list of mutations, this is done to avoid deadlock and doing seeks in order is // more efficient and detect duplicate rows. ConditionalMutationSet.sortConditionalMutations(updates); @@ -1023,7 +1023,7 @@ public List conditionalUpdate(TInfo tinfo, long sessID, } return results; - } catch (IOException ioe) { + } catch (IOException | ReflectiveOperationException ioe) { throw new TException(ioe); } catch (Exception e) { log.warn("Exception returned for conditionalUpdate {}", e); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java index f76a465db89..c6a2a1544bf 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java @@ -78,7 +78,6 @@ import org.apache.accumulo.server.compaction.CompactionStats; import org.apache.accumulo.server.compaction.FileCompactor; import org.apache.accumulo.server.compaction.FileCompactor.CompactionCanceledException; -import org.apache.accumulo.server.compaction.FileCompactor.CompactionClassLoadingException; import org.apache.accumulo.server.compaction.FileCompactor.CompactionEnv; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.fs.VolumeManager; @@ -561,7 +560,7 @@ private static AccumuloConfiguration getCompactionConfig(TableConfiguration tabl static CompactionStats compact(Tablet tablet, CompactionJob job, CompactableImpl.CompactionInfo cInfo, CompactionEnv cenv, Map compactFiles, TabletFile tmpFileName) throws IOException, - CompactionCanceledException, CompactionClassLoadingException, InterruptedException { + CompactionCanceledException, InterruptedException, ReflectiveOperationException { TableConfiguration tableConf = tablet.getTableConfiguration(); AccumuloConfiguration compactionConfig = getCompactionConfig(tableConf, 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 bd69bfa2e6c..93f73dbd2ed 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 @@ -115,7 +115,7 @@ public CompactionStats call() { } return ret; - } catch (IOException | UnsatisfiedLinkError e) { + } catch (IOException | ReflectiveOperationException | UnsatisfiedLinkError e) { log.warn("MinC failed ({}) to create {} retrying ...", e.getMessage(), outputFileName); ProblemReports.getInstance(tabletServer.getContext()).report( new ProblemReport(getExtent().tableId(), ProblemType.FILE_WRITE, outputFileName, e)); @@ -137,8 +137,7 @@ public CompactionStats call() { log.warn("MinC failed ({}) to create {} retrying ...", e.getMessage(), outputFileName, e); reportedProblem = true; retryCounter++; - } catch (CompactionCanceledException | CompactionClassLoadingException - | InterruptedException e) { + } catch (CompactionCanceledException | InterruptedException e) { throw new IllegalStateException(e); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java index 85f355f7c93..0f30f7673ee 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java @@ -125,12 +125,17 @@ public boolean isCurrent() { @Override public SortedKeyValueIterator iterator() throws IOException { if (iter == null) { - iter = createIterator(); + try { + iter = createIterator(); + } catch (ReflectiveOperationException e) { + throw new IOException("Error creating iterator", e); + } } return iter; } - private SortedKeyValueIterator createIterator() throws IOException { + private SortedKeyValueIterator createIterator() + throws IOException, ReflectiveOperationException { Map files; 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 8e296d400dc..71f4efca1c5 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 @@ -457,7 +457,7 @@ private void removeOldTemporaryFiles( } public void checkConditions(ConditionChecker checker, Authorizations authorizations, - AtomicBoolean iFlag) throws IOException { + AtomicBoolean iFlag) throws IOException, ReflectiveOperationException { ScanParameters scanParams = new ScanParameters(-1, authorizations, Collections.emptySet(), null, null, false, null, -1, null); @@ -469,7 +469,7 @@ public void checkConditions(ConditionChecker checker, Authorizations authorizati try { SortedKeyValueIterator iter = new SourceSwitchingIterator(dataSource); checker.check(iter); - } catch (IOException | RuntimeException e) { + } catch (IOException | RuntimeException | ReflectiveOperationException e) { sawException = true; throw e; } finally { diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java index 5e7a84a02ff..3d5cad9bdd2 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java +++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java @@ -469,7 +469,8 @@ public AccumuloClient getAccumuloClient() { } public ClassLoader getClassLoader(final CommandLine cl, final Shell shellState) - throws AccumuloException, TableNotFoundException, AccumuloSecurityException { + throws AccumuloException, TableNotFoundException, AccumuloSecurityException, IOException, + ReflectiveOperationException { boolean tables = cl.hasOption(OptUtil.tableOpt().getOpt()) || !shellState.getTableName().isEmpty(); diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java index 6e4eed888f7..937b97d8755 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java @@ -60,7 +60,7 @@ public class SetIterCommand extends Command { @Override public int execute(final String fullCommand, final CommandLine cl, final Shell shellState) throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException, - ShellCommandException { + ShellCommandException, ReflectiveOperationException { boolean tables = cl.hasOption(OptUtil.tableOpt().getOpt()) || !shellState.getTableName().isEmpty(); diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java index 2cb2295a8d2..2009cab38e8 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java @@ -43,7 +43,7 @@ public class SetScanIterCommand extends SetIterCommand { @Override public int execute(final String fullCommand, final CommandLine cl, final Shell shellState) throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException, - ShellCommandException { + ShellCommandException, ReflectiveOperationException { Shell.log.warn("Deprecated, use {}", new SetShellIterCommand().getName()); return super.execute(fullCommand, cl, shellState); } diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java index f10c13c8698..9ac3e1eea48 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java @@ -38,7 +38,7 @@ public class SetShellIterCommand extends SetIterCommand { @Override public int execute(final String fullCommand, final CommandLine cl, final Shell shellState) throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException, - ShellCommandException { + ShellCommandException, ReflectiveOperationException { return super.execute(fullCommand, cl, shellState); } diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java index c75b931b87c..f1f17795f92 100644 --- a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java +++ b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java @@ -194,12 +194,8 @@ private static ReloadingClassLoader createDynamicClassloader(final ClassLoader p return new AccumuloReloadingVFSClassLoader(dynamicCPath, generateVfs(), wrapper, 1000, true); } - public static ClassLoader getClassLoader() { - try { - return getClassLoader_Internal(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + public static ClassLoader getClassLoader() throws IOException { + return getClassLoader_Internal(); } private static ClassLoader getClassLoader_Internal() throws IOException { @@ -417,19 +413,25 @@ public static void removeUnusedContexts(Set contextsInUse) { } } - public static ClassLoader getContextClassLoader(String contextName) { - try { - return getContextManager().getClassLoader(contextName); - } catch (IOException e) { - throw new UncheckedIOException( - "Error getting context class loader for context: " + contextName, e); - } + public static ClassLoader getContextClassLoader(String contextName) throws IOException { + return getContextManager().getClassLoader(contextName); } public static synchronized ContextManager getContextManager() throws IOException { if (contextManager == null) { getClassLoader(); - contextManager = new ContextManager(generateVfs(), AccumuloVFSClassLoader::getClassLoader); + try { + contextManager = new ContextManager(generateVfs(), () -> { + try { + return getClassLoader(); + } catch (IOException e) { + // throw runtime, then unwrap it. + throw new UncheckedIOException(e); + } + }); + } catch (UncheckedIOException uioe) { + throw uioe.getCause(); + } } return contextManager; diff --git a/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java b/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java index bba4b355bc0..62e034bbef5 100644 --- a/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java +++ b/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java @@ -429,7 +429,7 @@ private static SortedKeyValueIterator createScanIterator(KeyExtent ke Collection> mapfiles, Authorizations authorizations, byte[] defaultLabels, HashSet columnSet, List ssiList, Map> ssio, boolean useTableIterators, TableConfiguration conf, - ServerContext context) throws IOException { + ServerContext context) throws IOException, ReflectiveOperationException { SortedMapIterator smi = new SortedMapIterator(new TreeMap<>()); From 7c7ecef2069f8d34e78b50d20728f8599bb963d2 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 11 Jul 2025 21:29:06 +0000 Subject: [PATCH 04/19] Return exception class name, capture failures and successes --- .../thrift/CompactionCoordinatorService.java | 137 ++++++++++++++++-- .../main/thrift/compaction-coordinator.thrift | 1 + .../coordinator/CompactionCoordinator.java | 91 +++++++++++- .../CompactionCoordinatorTest.java | 2 +- .../apache/accumulo/compactor/Compactor.java | 9 +- .../accumulo/compactor/CompactorTest.java | 2 +- 6 files changed, 220 insertions(+), 22 deletions(-) diff --git a/core/src/main/thrift-gen-java/org/apache/accumulo/core/compaction/thrift/CompactionCoordinatorService.java b/core/src/main/thrift-gen-java/org/apache/accumulo/core/compaction/thrift/CompactionCoordinatorService.java index ac8e15f6e5d..a59cec5ae91 100644 --- a/core/src/main/thrift-gen-java/org/apache/accumulo/core/compaction/thrift/CompactionCoordinatorService.java +++ b/core/src/main/thrift-gen-java/org/apache/accumulo/core/compaction/thrift/CompactionCoordinatorService.java @@ -35,7 +35,7 @@ public interface Iface { public void updateCompactionStatus(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, TCompactionStatusUpdate status, long timestamp) throws org.apache.thrift.TException; - public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent) throws org.apache.thrift.TException; + public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, java.lang.String exceptionClassName) throws org.apache.thrift.TException; public TExternalCompactionList getRunningCompactions(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials) throws org.apache.thrift.TException; @@ -53,7 +53,7 @@ public interface AsyncIface { public void updateCompactionStatus(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, TCompactionStatusUpdate status, long timestamp, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException; - public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException; + public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, java.lang.String exceptionClassName, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException; public void getRunningCompactions(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException; @@ -164,19 +164,20 @@ public void recv_updateCompactionStatus() throws org.apache.thrift.TException } @Override - public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent) throws org.apache.thrift.TException + public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, java.lang.String exceptionClassName) throws org.apache.thrift.TException { - send_compactionFailed(tinfo, credentials, externalCompactionId, extent); + send_compactionFailed(tinfo, credentials, externalCompactionId, extent, exceptionClassName); recv_compactionFailed(); } - public void send_compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent) throws org.apache.thrift.TException + public void send_compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, java.lang.String exceptionClassName) throws org.apache.thrift.TException { compactionFailed_args args = new compactionFailed_args(); args.setTinfo(tinfo); args.setCredentials(credentials); args.setExternalCompactionId(externalCompactionId); args.setExtent(extent); + args.setExceptionClassName(exceptionClassName); sendBase("compactionFailed", args); } @@ -423,9 +424,9 @@ public Void getResult() throws org.apache.thrift.TException { } @Override - public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException { + public void compactionFailed(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, java.lang.String exceptionClassName, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException { checkReady(); - compactionFailed_call method_call = new compactionFailed_call(tinfo, credentials, externalCompactionId, extent, resultHandler, this, ___protocolFactory, ___transport); + compactionFailed_call method_call = new compactionFailed_call(tinfo, credentials, externalCompactionId, extent, exceptionClassName, resultHandler, this, ___protocolFactory, ___transport); this.___currentMethod = method_call; ___manager.call(method_call); } @@ -435,12 +436,14 @@ public static class compactionFailed_call extends org.apache.thrift.async.TAsync private org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials; private java.lang.String externalCompactionId; private org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent; - public compactionFailed_call(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, org.apache.thrift.async.AsyncMethodCallback resultHandler, org.apache.thrift.async.TAsyncClient client, org.apache.thrift.protocol.TProtocolFactory protocolFactory, org.apache.thrift.transport.TNonblockingTransport transport) throws org.apache.thrift.TException { + private java.lang.String exceptionClassName; + public compactionFailed_call(org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, java.lang.String exceptionClassName, org.apache.thrift.async.AsyncMethodCallback resultHandler, org.apache.thrift.async.TAsyncClient client, org.apache.thrift.protocol.TProtocolFactory protocolFactory, org.apache.thrift.transport.TNonblockingTransport transport) throws org.apache.thrift.TException { super(client, protocolFactory, transport, resultHandler, false); this.tinfo = tinfo; this.credentials = credentials; this.externalCompactionId = externalCompactionId; this.extent = extent; + this.exceptionClassName = exceptionClassName; } @Override @@ -451,6 +454,7 @@ public void write_args(org.apache.thrift.protocol.TProtocol prot) throws org.apa args.setCredentials(credentials); args.setExternalCompactionId(externalCompactionId); args.setExtent(extent); + args.setExceptionClassName(exceptionClassName); args.write(prot); prot.writeMessageEnd(); } @@ -715,7 +719,7 @@ protected boolean rethrowUnhandledExceptions() { @Override public compactionFailed_result getResult(I iface, compactionFailed_args args) throws org.apache.thrift.TException { compactionFailed_result result = new compactionFailed_result(); - iface.compactionFailed(args.tinfo, args.credentials, args.externalCompactionId, args.extent); + iface.compactionFailed(args.tinfo, args.credentials, args.externalCompactionId, args.extent, args.exceptionClassName); return result; } } @@ -1088,7 +1092,7 @@ protected boolean isOneway() { @Override public void start(I iface, compactionFailed_args args, org.apache.thrift.async.AsyncMethodCallback resultHandler) throws org.apache.thrift.TException { - iface.compactionFailed(args.tinfo, args.credentials, args.externalCompactionId, args.extent,resultHandler); + iface.compactionFailed(args.tinfo, args.credentials, args.externalCompactionId, args.extent, args.exceptionClassName,resultHandler); } } @@ -4652,6 +4656,7 @@ public static class compactionFailed_args implements org.apache.thrift.TBase byName = new java.util.HashMap(); @@ -4690,6 +4697,8 @@ public static _Fields findByThriftId(int fieldId) { return EXTERNAL_COMPACTION_ID; case 4: // EXTENT return EXTENT; + case 5: // EXCEPTION_CLASS_NAME + return EXCEPTION_CLASS_NAME; default: return null; } @@ -4744,6 +4753,8 @@ public java.lang.String getFieldName() { new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING))); tmpMap.put(_Fields.EXTENT, new org.apache.thrift.meta_data.FieldMetaData("extent", org.apache.thrift.TFieldRequirementType.DEFAULT, new org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType.STRUCT, org.apache.accumulo.core.dataImpl.thrift.TKeyExtent.class))); + tmpMap.put(_Fields.EXCEPTION_CLASS_NAME, new org.apache.thrift.meta_data.FieldMetaData("exceptionClassName", org.apache.thrift.TFieldRequirementType.DEFAULT, + new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING))); metaDataMap = java.util.Collections.unmodifiableMap(tmpMap); org.apache.thrift.meta_data.FieldMetaData.addStructMetaDataMap(compactionFailed_args.class, metaDataMap); } @@ -4755,13 +4766,15 @@ public compactionFailed_args( org.apache.accumulo.core.trace.thrift.TInfo tinfo, org.apache.accumulo.core.securityImpl.thrift.TCredentials credentials, java.lang.String externalCompactionId, - org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent) + org.apache.accumulo.core.dataImpl.thrift.TKeyExtent extent, + java.lang.String exceptionClassName) { this(); this.tinfo = tinfo; this.credentials = credentials; this.externalCompactionId = externalCompactionId; this.extent = extent; + this.exceptionClassName = exceptionClassName; } /** @@ -4780,6 +4793,9 @@ public compactionFailed_args(compactionFailed_args other) { if (other.isSetExtent()) { this.extent = new org.apache.accumulo.core.dataImpl.thrift.TKeyExtent(other.extent); } + if (other.isSetExceptionClassName()) { + this.exceptionClassName = other.exceptionClassName; + } } @Override @@ -4793,6 +4809,7 @@ public void clear() { this.credentials = null; this.externalCompactionId = null; this.extent = null; + this.exceptionClassName = null; } @org.apache.thrift.annotation.Nullable @@ -4895,6 +4912,31 @@ public void setExtentIsSet(boolean value) { } } + @org.apache.thrift.annotation.Nullable + public java.lang.String getExceptionClassName() { + return this.exceptionClassName; + } + + public compactionFailed_args setExceptionClassName(@org.apache.thrift.annotation.Nullable java.lang.String exceptionClassName) { + this.exceptionClassName = exceptionClassName; + return this; + } + + public void unsetExceptionClassName() { + this.exceptionClassName = null; + } + + /** Returns true if field exceptionClassName is set (has been assigned a value) and false otherwise */ + public boolean isSetExceptionClassName() { + return this.exceptionClassName != null; + } + + public void setExceptionClassNameIsSet(boolean value) { + if (!value) { + this.exceptionClassName = null; + } + } + @Override public void setFieldValue(_Fields field, @org.apache.thrift.annotation.Nullable java.lang.Object value) { switch (field) { @@ -4930,6 +4972,14 @@ public void setFieldValue(_Fields field, @org.apache.thrift.annotation.Nullable } break; + case EXCEPTION_CLASS_NAME: + if (value == null) { + unsetExceptionClassName(); + } else { + setExceptionClassName((java.lang.String)value); + } + break; + } } @@ -4949,6 +4999,9 @@ public java.lang.Object getFieldValue(_Fields field) { case EXTENT: return getExtent(); + case EXCEPTION_CLASS_NAME: + return getExceptionClassName(); + } throw new java.lang.IllegalStateException(); } @@ -4969,6 +5022,8 @@ public boolean isSet(_Fields field) { return isSetExternalCompactionId(); case EXTENT: return isSetExtent(); + case EXCEPTION_CLASS_NAME: + return isSetExceptionClassName(); } throw new java.lang.IllegalStateException(); } @@ -5022,6 +5077,15 @@ public boolean equals(compactionFailed_args that) { return false; } + boolean this_present_exceptionClassName = true && this.isSetExceptionClassName(); + boolean that_present_exceptionClassName = true && that.isSetExceptionClassName(); + if (this_present_exceptionClassName || that_present_exceptionClassName) { + if (!(this_present_exceptionClassName && that_present_exceptionClassName)) + return false; + if (!this.exceptionClassName.equals(that.exceptionClassName)) + return false; + } + return true; } @@ -5045,6 +5109,10 @@ public int hashCode() { if (isSetExtent()) hashCode = hashCode * 8191 + extent.hashCode(); + hashCode = hashCode * 8191 + ((isSetExceptionClassName()) ? 131071 : 524287); + if (isSetExceptionClassName()) + hashCode = hashCode * 8191 + exceptionClassName.hashCode(); + return hashCode; } @@ -5096,6 +5164,16 @@ public int compareTo(compactionFailed_args other) { return lastComparison; } } + lastComparison = java.lang.Boolean.compare(isSetExceptionClassName(), other.isSetExceptionClassName()); + if (lastComparison != 0) { + return lastComparison; + } + if (isSetExceptionClassName()) { + lastComparison = org.apache.thrift.TBaseHelper.compareTo(this.exceptionClassName, other.exceptionClassName); + if (lastComparison != 0) { + return lastComparison; + } + } return 0; } @@ -5151,6 +5229,14 @@ public java.lang.String toString() { sb.append(this.extent); } first = false; + if (!first) sb.append(", "); + sb.append("exceptionClassName:"); + if (this.exceptionClassName == null) { + sb.append("null"); + } else { + sb.append(this.exceptionClassName); + } + first = false; sb.append(")"); return sb.toString(); } @@ -5240,6 +5326,14 @@ public void read(org.apache.thrift.protocol.TProtocol iprot, compactionFailed_ar org.apache.thrift.protocol.TProtocolUtil.skip(iprot, schemeField.type); } break; + case 5: // EXCEPTION_CLASS_NAME + if (schemeField.type == org.apache.thrift.protocol.TType.STRING) { + struct.exceptionClassName = iprot.readString(); + struct.setExceptionClassNameIsSet(true); + } else { + org.apache.thrift.protocol.TProtocolUtil.skip(iprot, schemeField.type); + } + break; default: org.apache.thrift.protocol.TProtocolUtil.skip(iprot, schemeField.type); } @@ -5276,6 +5370,11 @@ public void write(org.apache.thrift.protocol.TProtocol oprot, compactionFailed_a struct.extent.write(oprot); oprot.writeFieldEnd(); } + if (struct.exceptionClassName != null) { + oprot.writeFieldBegin(EXCEPTION_CLASS_NAME_FIELD_DESC); + oprot.writeString(struct.exceptionClassName); + oprot.writeFieldEnd(); + } oprot.writeFieldStop(); oprot.writeStructEnd(); } @@ -5307,7 +5406,10 @@ public void write(org.apache.thrift.protocol.TProtocol prot, compactionFailed_ar if (struct.isSetExtent()) { optionals.set(3); } - oprot.writeBitSet(optionals, 4); + if (struct.isSetExceptionClassName()) { + optionals.set(4); + } + oprot.writeBitSet(optionals, 5); if (struct.isSetTinfo()) { struct.tinfo.write(oprot); } @@ -5320,12 +5422,15 @@ public void write(org.apache.thrift.protocol.TProtocol prot, compactionFailed_ar if (struct.isSetExtent()) { struct.extent.write(oprot); } + if (struct.isSetExceptionClassName()) { + oprot.writeString(struct.exceptionClassName); + } } @Override public void read(org.apache.thrift.protocol.TProtocol prot, compactionFailed_args struct) throws org.apache.thrift.TException { org.apache.thrift.protocol.TTupleProtocol iprot = (org.apache.thrift.protocol.TTupleProtocol) prot; - java.util.BitSet incoming = iprot.readBitSet(4); + java.util.BitSet incoming = iprot.readBitSet(5); if (incoming.get(0)) { struct.tinfo = new org.apache.accumulo.core.trace.thrift.TInfo(); struct.tinfo.read(iprot); @@ -5345,6 +5450,10 @@ public void read(org.apache.thrift.protocol.TProtocol prot, compactionFailed_arg struct.extent.read(iprot); struct.setExtentIsSet(true); } + if (incoming.get(4)) { + struct.exceptionClassName = iprot.readString(); + struct.setExceptionClassNameIsSet(true); + } } } diff --git a/core/src/main/thrift/compaction-coordinator.thrift b/core/src/main/thrift/compaction-coordinator.thrift index cbf3ac0d1fc..ae6a7cce6b9 100644 --- a/core/src/main/thrift/compaction-coordinator.thrift +++ b/core/src/main/thrift/compaction-coordinator.thrift @@ -112,6 +112,7 @@ service CompactionCoordinatorService { 2:security.TCredentials credentials 3:string externalCompactionId 4:data.TKeyExtent extent + 5:string exceptionClassName ) /* diff --git a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java index add5d607565..ad7c2b2b83b 100644 --- a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java +++ b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java @@ -35,6 +35,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import org.apache.accumulo.coordinator.QueueSummaries.PrioTserver; @@ -55,6 +56,7 @@ import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.NamespaceId; +import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.dataImpl.thrift.TKeyExtent; import org.apache.accumulo.core.fate.zookeeper.ServiceLock; @@ -130,6 +132,10 @@ public class CompactionCoordinator extends AbstractServer implements /* Map of queue name to last time compactor called to get a compaction job */ private static final Map TIME_COMPACTOR_LAST_CHECKED = new ConcurrentHashMap<>(); + private final ConcurrentHashMap failingQueues = new ConcurrentHashMap<>(); + private final ConcurrentHashMap failingCompactors = new ConcurrentHashMap<>(); + private final ConcurrentHashMap failingTables = new ConcurrentHashMap<>(); + private final GarbageCollectionLogger gcLogger = new GarbageCollectionLogger(); protected AuditedSecurityOperation security; protected final AccumuloConfiguration aconf; @@ -501,6 +507,20 @@ public TNextCompactionJob getCompactionJob(TInfo tinfo, TCredentials credentials LOG.trace("getCompactionJob called for queue {} by compactor {}", queue, compactorAddress); TIME_COMPACTOR_LAST_CHECKED.put(queue, System.currentTimeMillis()); + AtomicLong queueFailures = failingQueues.get(queue); + if (queueFailures != null && queueFailures.get() > 10) { + LOG.warn("Compactions for queue {} may be in a failing state. Not executing compactions" + + " for this queue", queue); + return new TNextCompactionJob(new TExternalCompactionJob(), compactorCounts.get(queue)); + } + + AtomicLong compactorFailures = failingCompactors.get(compactorAddress); + if (compactorFailures != null && compactorFailures.get() > 10) { + LOG.warn("Compactor {} may be in a failing state. Not executing compactions" + + " for this compactor", compactorAddress); + return new TNextCompactionJob(new TExternalCompactionJob(), compactorCounts.get(queue)); + } + TExternalCompactionJob result = null; PrioTserver prioTserver = QUEUE_SUMMARIES.getNextTserver(queue); @@ -531,6 +551,22 @@ public TNextCompactionJob getCompactionJob(TInfo tinfo, TCredentials credentials prioTserver = QUEUE_SUMMARIES.getNextTserver(queue); continue; } + + if (job.getExtent() != null) { + KeyExtent ke = KeyExtent.fromThrift(job.getExtent()); + if (ke.tableId() != null) { + AtomicLong tableFailures = failingTables.get(ke.tableId()); + if (tableFailures != null && tableFailures.get() > 10) { + LOG.warn( + "Compactions for table {} may be in a failing state. Not executing compactions" + + " for this table", + ke.tableId()); + // TODO: Need to unreserve compaction? + continue; + } + } + } + // It is possible that by the time this added that the tablet has already canceled the // compaction or the compactor that made this request is dead. In these cases the compaction // is not actually running. @@ -602,6 +638,7 @@ public void compactionCompleted(TInfo tinfo, TCredentials credentials, LOG.debug("Compaction completed, id: {}, stats: {}, extent: {}", externalCompactionId, stats, extent); final var ecid = ExternalCompactionId.of(externalCompactionId); + captureSuccess(ecid, extent); compactionFinalizer.commitCompaction(ecid, extent, stats.fileSize, stats.entriesWritten); // It's possible that RUNNING might not have an entry for this ecid in the case // of a coordinator restart when the Coordinator can't find the TServer for the @@ -611,18 +648,68 @@ public void compactionCompleted(TInfo tinfo, TCredentials credentials, @Override public void compactionFailed(TInfo tinfo, TCredentials credentials, String externalCompactionId, - TKeyExtent extent) throws ThriftSecurityException { + TKeyExtent extent, String exceptionClassName) throws ThriftSecurityException { // do not expect users to call this directly, expect other tservers to call this method if (!security.canPerformSystemActions(credentials)) { throw new AccumuloSecurityException(credentials.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED).asThriftException(); } KeyExtent fromThriftExtent = KeyExtent.fromThrift(extent); - LOG.info("Compaction failed: id: {}, extent: {}", externalCompactionId, fromThriftExtent); + LOG.info("Compaction failed: id: {}, extent: {}, compactor exception:{}", externalCompactionId, + fromThriftExtent, exceptionClassName); final var ecid = ExternalCompactionId.of(externalCompactionId); + if (exceptionClassName != null) { + captureFailure(ecid, fromThriftExtent); + } compactionFailed(Map.of(ecid, KeyExtent.fromThrift(extent))); } + private void captureFailure(ExternalCompactionId ecid, KeyExtent extent) { + var rc = RUNNING_CACHE.get(ecid); + if (rc != null) { + final String queue = rc.getQueueName(); + failingQueues.computeIfAbsent(queue, q -> new AtomicLong(0)).incrementAndGet(); + final String compactor = rc.getCompactorAddress(); + failingCompactors.computeIfAbsent(compactor, c -> new AtomicLong(0)).incrementAndGet(); + } + failingTables.computeIfAbsent(extent.tableId(), t -> new AtomicLong(0)).incrementAndGet(); + } + + private void captureSuccess(ExternalCompactionId ecid, KeyExtent extent) { + var rc = RUNNING_CACHE.get(ecid); + if (rc != null) { + final String queue = rc.getQueueName(); + failingQueues.computeIfPresent(queue, (q, curr) -> { + long currentValue = Math.max(0L, curr.get() - 1); + if (currentValue == 0) { + return null; // remove from the map + } else { + curr.set(currentValue); + return curr; + } + }); + final String compactor = rc.getCompactorAddress(); + failingCompactors.computeIfPresent(compactor, (c, curr) -> { + long currentValue = Math.max(0L, curr.get() - 1); + if (currentValue == 0) { + return null; // remove from the map + } else { + curr.set(currentValue); + return curr; + } + }); + } + failingTables.computeIfPresent(extent.tableId(), (t, curr) -> { + long currentValue = Math.max(0L, curr.get() - 1); + if (currentValue == 0) { + return null; // remove from the map + } else { + curr.set(currentValue); + return curr; + } + }); + } + void compactionFailed(Map compactions) { compactionFinalizer.failCompactions(compactions); compactions.forEach((k, v) -> recordCompletion(k)); diff --git a/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java b/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java index 8b320aac341..7b758acb940 100644 --- a/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java +++ b/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java @@ -188,7 +188,7 @@ public void compactionCompleted(TInfo tinfo, TCredentials credentials, @Override public void compactionFailed(TInfo tinfo, TCredentials credentials, String externalCompactionId, - TKeyExtent extent) throws ThriftSecurityException {} + TKeyExtent extent, String exceptionClassName) throws ThriftSecurityException {} void setMetadataCompactionIds(Set mci) { metadataCompactionIds = mci; 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 ce18a0ef9ea..ad0143c1084 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 @@ -395,16 +395,17 @@ protected void updateCompactionState(TExternalCompactionJob job, TCompactionStat * Notify the CompactionCoordinator the job failed * * @param job current compaction job + * @param exception cause of failure * @throws RetriesExceededException thrown when retries have been exceeded */ - protected void updateCompactionFailed(TExternalCompactionJob job) + protected void updateCompactionFailed(TExternalCompactionJob job, Throwable exception) throws RetriesExceededException { RetryableThriftCall thriftCall = new RetryableThriftCall<>(1000, RetryableThriftCall.MAX_WAIT_TIME, 25, () -> { Client coordinatorClient = getCoordinatorClient(); try { coordinatorClient.compactionFailed(TraceUtil.traceInfo(), getContext().rpcCreds(), - job.getExternalCompactionId(), job.extent); + job.getExternalCompactionId(), job.extent, exception.getClass().getName()); return ""; } finally { ThriftUtil.returnClient(coordinatorClient, getContext()); @@ -809,7 +810,7 @@ public void run() { new TCompactionStatusUpdate(TCompactionState.CANCELLED, "Compaction cancelled", -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); - updateCompactionFailed(job); + updateCompactionFailed(job, null); } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction cancellation.", e); } finally { @@ -824,7 +825,7 @@ public void run() { TCompactionState.FAILED, "Compaction failed due to: " + err.get().getMessage(), -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); - updateCompactionFailed(job); + updateCompactionFailed(job, err.get()); } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", job.getExternalCompactionId(), fromThriftExtent, e); diff --git a/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java b/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java index 3489e8fef85..228b6911a55 100644 --- a/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java +++ b/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java @@ -272,7 +272,7 @@ protected void updateCompactionState(TExternalCompactionJob job, TCompactionStat } @Override - protected void updateCompactionFailed(TExternalCompactionJob job) + protected void updateCompactionFailed(TExternalCompactionJob job, Throwable exception) throws RetriesExceededException { failedCalled = true; } From 6563aeec3b5f3c154a9159adbbff3c86853c6995 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 14 Jul 2025 15:35:52 +0000 Subject: [PATCH 05/19] Moved failure account to Compactor, log summary in Coordinator --- .../apache/accumulo/core/conf/Property.java | 22 +++++++ .../coordinator/CompactionCoordinator.java | 43 +++++-------- .../CompactionCoordinatorTest.java | 3 + .../apache/accumulo/compactor/Compactor.java | 62 ++++++++++++++++++- 4 files changed, 100 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index d9d94cb3677..1e73b1d4e57 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -1556,6 +1556,28 @@ public enum Property { COMPACTOR_CLIENTPORT("compactor.port.client", "9133", PropertyType.PORT, "The port used for handling client connections on the compactor servers.", "2.1.0"), @Experimental + COMPACTOR_FAILURE_BACKOFF_THRESHOLD("compactor.failure.backoff.threshold", "3", + PropertyType.COUNT, + "The number of consecutive failures that must occur before the Compactor starts to back off" + + " processing compactions.", + "2.1.4"), + @Experimental + COMPACTOR_FAILURE_BACKOFF_INTERVAL("compactor.failure.backoff.interval", "0", + PropertyType.TIMEDURATION, + "The time basis for computing the wait time for compaction failure backoff. A value of zero disables" + + " the backoff feature. When a non-zero value is supplied, then after compactor.failure.backoff.threshold" + + " failures have occurred, the compactor will wait compactor.failure.backoff.interval * the number of" + + " failures seconds before executing the next compaction. For example, if this value is 10s, then after" + + " three failures the Compactor will wait 30s before starting the next compaction. If the compaction fails" + + " again, then it will wait 40s before starting the next compaction.", + "2.1.4"), + @Experimental + COMPACTOR_FAILURE_BACKOFF_RESET("compactor.failure.backoff.reset", "10m", + PropertyType.TIMEDURATION, + "The maximum amount of time that the compactor will wait before executing the next compaction. When this" + + " time limit has been reached, the failures are cleared.", + "2.1.4"), + @Experimental COMPACTOR_MINTHREADS("compactor.threads.minimum", "4", PropertyType.COUNT, "The minimum number of threads to use to handle incoming requests.", "2.1.0"), @Experimental diff --git a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java index ad7c2b2b83b..473e3825536 100644 --- a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java +++ b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java @@ -318,6 +318,7 @@ public void run() { tserverSet.startListeningForTabletServerChanges(); startDeadCompactionDetector(); + startFailureSummaryLogging(); LOG.info("Starting loop to check tservers for compaction summaries"); while (!isShutdownRequested()) { @@ -507,20 +508,6 @@ public TNextCompactionJob getCompactionJob(TInfo tinfo, TCredentials credentials LOG.trace("getCompactionJob called for queue {} by compactor {}", queue, compactorAddress); TIME_COMPACTOR_LAST_CHECKED.put(queue, System.currentTimeMillis()); - AtomicLong queueFailures = failingQueues.get(queue); - if (queueFailures != null && queueFailures.get() > 10) { - LOG.warn("Compactions for queue {} may be in a failing state. Not executing compactions" - + " for this queue", queue); - return new TNextCompactionJob(new TExternalCompactionJob(), compactorCounts.get(queue)); - } - - AtomicLong compactorFailures = failingCompactors.get(compactorAddress); - if (compactorFailures != null && compactorFailures.get() > 10) { - LOG.warn("Compactor {} may be in a failing state. Not executing compactions" - + " for this compactor", compactorAddress); - return new TNextCompactionJob(new TExternalCompactionJob(), compactorCounts.get(queue)); - } - TExternalCompactionJob result = null; PrioTserver prioTserver = QUEUE_SUMMARIES.getNextTserver(queue); @@ -552,21 +539,6 @@ public TNextCompactionJob getCompactionJob(TInfo tinfo, TCredentials credentials continue; } - if (job.getExtent() != null) { - KeyExtent ke = KeyExtent.fromThrift(job.getExtent()); - if (ke.tableId() != null) { - AtomicLong tableFailures = failingTables.get(ke.tableId()); - if (tableFailures != null && tableFailures.get() > 10) { - LOG.warn( - "Compactions for table {} may be in a failing state. Not executing compactions" - + " for this table", - ke.tableId()); - // TODO: Need to unreserve compaction? - continue; - } - } - } - // It is possible that by the time this added that the tablet has already canceled the // compaction or the compactor that made this request is dead. In these cases the compaction // is not actually running. @@ -675,6 +647,19 @@ private void captureFailure(ExternalCompactionId ecid, KeyExtent extent) { failingTables.computeIfAbsent(extent.tableId(), t -> new AtomicLong(0)).incrementAndGet(); } + protected void startFailureSummaryLogging() { + ScheduledFuture future = getContext().getScheduledExecutor() + .scheduleWithFixedDelay(this::printFailures, 0, 5, TimeUnit.MINUTES); + ThreadPools.watchNonCriticalScheduledTask(future); + } + + private void printFailures() { + LOG.warn("Compaction failure summary:"); + LOG.warn("Queue failures: {}", failingQueues); + LOG.warn("Table failures: {}", failingTables); + LOG.warn("Compactor failures: {}", failingCompactors); + } + private void captureSuccess(ExternalCompactionId ecid, KeyExtent extent) { var rc = RUNNING_CACHE.get(ecid); if (rc != null) { diff --git a/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java b/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java index 7b758acb940..a38bcb61a0b 100644 --- a/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java +++ b/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java @@ -117,6 +117,9 @@ protected TestCoordinator(CompactionFinalizer finalizer, LiveTServerSet tservers @Override protected void startDeadCompactionDetector() {} + @Override + protected void startFailureSummaryLogging() {} + @Override protected long getTServerCheckInterval() { gracefulShutdown(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 ad0143c1084..5d045fc82ce 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 @@ -28,6 +28,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -38,6 +39,7 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.LongAdder; import java.util.function.Supplier; @@ -73,6 +75,8 @@ import org.apache.accumulo.core.file.FileOperations; import org.apache.accumulo.core.file.FileSKVIterator; import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil; +import org.apache.accumulo.core.metadata.MetadataTable; +import org.apache.accumulo.core.metadata.RootTable; import org.apache.accumulo.core.metadata.StoredTabletFile; import org.apache.accumulo.core.metadata.TabletFile; import org.apache.accumulo.core.metadata.schema.DataFileValue; @@ -93,6 +97,7 @@ import org.apache.accumulo.core.trace.TraceUtil; import org.apache.accumulo.core.trace.thrift.TInfo; import org.apache.accumulo.core.util.HostAndPort; +import org.apache.accumulo.core.util.Pair; import org.apache.accumulo.core.util.ServerServices; import org.apache.accumulo.core.util.ServerServices.Service; import org.apache.accumulo.core.util.UtilWaitThread; @@ -699,6 +704,7 @@ public void run() { try { final AtomicReference err = new AtomicReference<>(); + final Map> errorHistory = new HashMap<>(); while (!isShutdownRequested()) { if (Thread.currentThread().isInterrupted()) { @@ -732,6 +738,52 @@ public void run() { } LOG.debug("Received next compaction job: {}", job); + // The errorHistory is cleared on a successful compaction. If the errorHistory map + // is not empty, then we have a history of recent failures. If the current job's + // table is in the map with 2 consecutive failures or if there is more than one + // entry in the map, then start backing off and complain loudly. + TableId tableId = TableId.of(new String(job.getExtent().getTable(), UTF_8)); + if (!tableId.equals(RootTable.ID) && !tableId.equals(MetadataTable.ID)) { + long totalFailures = 0; + if (errorHistory.size() >= 2) { + totalFailures = + errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); + LOG.warn( + "This Compactor has had {} consecutive failures across different tables. Failures: {}", + totalFailures, errorHistory); + } else if (errorHistory.containsKey(tableId)) { + totalFailures = errorHistory.get(tableId).getSecond().get(); + if (totalFailures > 2) { + LOG.warn( + "This Compactor has had {} consecutive failures for table {}. Failures: {}", + totalFailures, errorHistory); + } + } + if (totalFailures + > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { + long interval = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); + if (interval > 0) { + long max = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); + // Introduce a 30s wait per failure, up to 10 mins + long backoffMS = Math.min(max, interval * totalFailures); + LOG.warn( + "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", + backoffMS); + if (backoffMS == max) { + errorHistory.clear(); + } + Thread.sleep(backoffMS); + } else if (interval == 0) { + LOG.info( + "This Compactor has had {} consecutive failures and failure backoff is disabled.", + totalFailures); + errorHistory.clear(); + } + } + } + final LongAdder totalInputEntries = new LongAdder(); final LongAdder totalInputBytes = new LongAdder(); final CountDownLatch started = new CountDownLatch(1); @@ -817,7 +869,7 @@ public void run() { currentCompactionId.set(null); } } else if (err.get() != null) { - KeyExtent fromThriftExtent = KeyExtent.fromThrift(job.getExtent()); + final KeyExtent fromThriftExtent = KeyExtent.fromThrift(job.getExtent()); try { LOG.info("Updating coordinator with compaction failure: id: {}, extent: {}", job.getExternalCompactionId(), fromThriftExtent); @@ -826,6 +878,12 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job, err.get()); + long failures = errorHistory + .computeIfAbsent(fromThriftExtent.tableId(), + t -> new Pair<>(err.get(), new AtomicLong(0))) + .getSecond().incrementAndGet(); + LOG.warn("Compaction for table {} has failed {} times", fromThriftExtent.tableId(), + failures); } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", job.getExternalCompactionId(), fromThriftExtent, e); @@ -836,6 +894,8 @@ public void run() { try { LOG.trace("Updating coordinator with compaction completion."); updateCompactionCompleted(job, JOB_HOLDER.getStats()); + // job completed successfully, clear the error history + errorHistory.clear(); } catch (RetriesExceededException e) { LOG.error( "Error updating coordinator with compaction completion, cancelling compaction.", From 65a7a358f4f7176286c0a0c32fac349d917f8e5c Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 14 Jul 2025 16:42:23 +0000 Subject: [PATCH 06/19] Minor update --- pom.xml | 2 +- .../apache/accumulo/compactor/Compactor.java | 63 ++++++++----------- 2 files changed, 26 insertions(+), 39 deletions(-) diff --git a/pom.xml b/pom.xml index c8aa687bbb3..6c2548370fd 100644 --- a/pom.xml +++ b/pom.xml @@ -116,7 +116,7 @@ ${project.version} false - --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED --add-opens java.management/java.lang.management=ALL-UNNAMED --add-opens java.management/sun.management=ALL-UNNAMED --add-opens java.base/java.security=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED + --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED --add-opens java.management/java.lang.management=ALL-UNNAMED --add-opens java.management/sun.management=ALL-UNNAMED --add-opens java.base/java.security=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.base/java.util.stream=ALL-UNNAMED --add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED false 1 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 5d045fc82ce..318ff69ac5e 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 @@ -738,48 +738,35 @@ public void run() { } LOG.debug("Received next compaction job: {}", job); - // The errorHistory is cleared on a successful compaction. If the errorHistory map - // is not empty, then we have a history of recent failures. If the current job's - // table is in the map with 2 consecutive failures or if there is more than one - // entry in the map, then start backing off and complain loudly. - TableId tableId = TableId.of(new String(job.getExtent().getTable(), UTF_8)); + final TableId tableId = TableId.of(new String(job.getExtent().getTable(), UTF_8)); if (!tableId.equals(RootTable.ID) && !tableId.equals(MetadataTable.ID)) { - long totalFailures = 0; - if (errorHistory.size() >= 2) { - totalFailures = - errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); - LOG.warn( - "This Compactor has had {} consecutive failures across different tables. Failures: {}", + + final long totalFailures = + errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); + if (totalFailures > 0) { + LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", totalFailures, errorHistory); - } else if (errorHistory.containsKey(tableId)) { - totalFailures = errorHistory.get(tableId).getSecond().get(); - if (totalFailures > 2) { - LOG.warn( - "This Compactor has had {} consecutive failures for table {}. Failures: {}", - totalFailures, errorHistory); - } - } - if (totalFailures - > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { - long interval = - getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); - if (interval > 0) { - long max = - getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); - // Introduce a 30s wait per failure, up to 10 mins - long backoffMS = Math.min(max, interval * totalFailures); - LOG.warn( - "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", - backoffMS); - if (backoffMS == max) { + if (totalFailures + > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { + final long interval = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); + if (interval > 0) { + final long max = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); + final long backoffMS = Math.min(max, interval * totalFailures); + LOG.warn( + "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", + backoffMS); + if (backoffMS == max) { + errorHistory.clear(); + } + Thread.sleep(backoffMS); + } else if (interval == 0) { + LOG.info( + "This Compactor has had {} consecutive failures and failure backoff is disabled.", + totalFailures); errorHistory.clear(); } - Thread.sleep(backoffMS); - } else if (interval == 0) { - LOG.info( - "This Compactor has had {} consecutive failures and failure backoff is disabled.", - totalFailures); - errorHistory.clear(); } } } From 95ebecf2ecc5b76e486ac042ff4d60444aebe35e Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 14 Jul 2025 17:27:11 +0000 Subject: [PATCH 07/19] Add property to terminate Compactor based on consecutive failures --- .../apache/accumulo/core/conf/Property.java | 6 ++ .../coordinator/CompactionCoordinator.java | 8 ++ .../apache/accumulo/compactor/Compactor.java | 73 ++++++++++--------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index 1e73b1d4e57..56beece0455 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -1578,6 +1578,12 @@ public enum Property { + " time limit has been reached, the failures are cleared.", "2.1.4"), @Experimental + COMPACTOR_FAILURE_TERMINATION_THRESHOLD("compactor.failure.termination.threshold", "0", + PropertyType.TIMEDURATION, + "The number of consecutive failures at which the Compactor exits and the process terminates. A zero" + + " value disables this feature.", + "2.1.4"), + @Experimental COMPACTOR_MINTHREADS("compactor.threads.minimum", "4", PropertyType.COUNT, "The minimum number of threads to use to handle incoming requests.", "2.1.0"), @Experimental diff --git a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java index 473e3825536..feb4b3d6e46 100644 --- a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java +++ b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java @@ -654,6 +654,14 @@ protected void startFailureSummaryLogging() { } private void printFailures() { + + // Remove down compactors from failing list + Map> allCompactors = + ExternalCompactionUtil.getCompactorAddrs(getContext()); + Set allCompactorAddrs = new HashSet<>(); + allCompactors.values().forEach(l -> l.forEach(c -> allCompactorAddrs.add(c.toString()))); + failingCompactors.keySet().retainAll(allCompactorAddrs); + LOG.warn("Compaction failure summary:"); LOG.warn("Queue failures: {}", failingQueues); LOG.warn("Table failures: {}", failingTables); 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 318ff69ac5e..58b79005200 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 @@ -75,8 +75,6 @@ import org.apache.accumulo.core.file.FileOperations; import org.apache.accumulo.core.file.FileSKVIterator; import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil; -import org.apache.accumulo.core.metadata.MetadataTable; -import org.apache.accumulo.core.metadata.RootTable; import org.apache.accumulo.core.metadata.StoredTabletFile; import org.apache.accumulo.core.metadata.TabletFile; import org.apache.accumulo.core.metadata.schema.DataFileValue; @@ -719,6 +717,44 @@ public void run() { err.set(null); JOB_HOLDER.reset(); + // consecutive failure processing + final long totalFailures = + errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); + if (totalFailures > 0) { + LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", totalFailures, + errorHistory); + final long failureThreshold = + getConfiguration().getCount(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD); + if (failureThreshold > 0 && totalFailures > failureThreshold) { + LOG.error("Consecutive failures ({}) has exceeded failure threshold ({}), exiting...", + totalFailures, failureThreshold); + throw new InterruptedException( + "Consecutive failures has exceeded failure threshold, exiting..."); + } + if (totalFailures + > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { + final long interval = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); + if (interval > 0) { + final long max = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); + final long backoffMS = Math.min(max, interval * totalFailures); + LOG.warn( + "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", + backoffMS); + if (backoffMS == max) { + errorHistory.clear(); + } + Thread.sleep(backoffMS); + } else if (interval == 0) { + LOG.info( + "This Compactor has had {} consecutive failures and failure backoff is disabled.", + totalFailures); + errorHistory.clear(); + } + } + } + TExternalCompactionJob job; try { TNextCompactionJob next = getNextJob(getNextId()); @@ -738,39 +774,6 @@ public void run() { } LOG.debug("Received next compaction job: {}", job); - final TableId tableId = TableId.of(new String(job.getExtent().getTable(), UTF_8)); - if (!tableId.equals(RootTable.ID) && !tableId.equals(MetadataTable.ID)) { - - final long totalFailures = - errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); - if (totalFailures > 0) { - LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", - totalFailures, errorHistory); - if (totalFailures - > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { - final long interval = - getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); - if (interval > 0) { - final long max = - getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); - final long backoffMS = Math.min(max, interval * totalFailures); - LOG.warn( - "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", - backoffMS); - if (backoffMS == max) { - errorHistory.clear(); - } - Thread.sleep(backoffMS); - } else if (interval == 0) { - LOG.info( - "This Compactor has had {} consecutive failures and failure backoff is disabled.", - totalFailures); - errorHistory.clear(); - } - } - } - } - final LongAdder totalInputEntries = new LongAdder(); final LongAdder totalInputBytes = new LongAdder(); final CountDownLatch started = new CountDownLatch(1); From 2865ad2531d2bd39ba5003af154b50c469ca2afd Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 15 Jul 2025 18:42:29 +0000 Subject: [PATCH 08/19] Fix property type, move error history logic to method --- .../apache/accumulo/core/conf/Property.java | 2 +- .../apache/accumulo/compactor/Compactor.java | 79 ++++++++++--------- 2 files changed, 43 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index 56beece0455..7c0375aa666 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -1579,7 +1579,7 @@ public enum Property { "2.1.4"), @Experimental COMPACTOR_FAILURE_TERMINATION_THRESHOLD("compactor.failure.termination.threshold", "0", - PropertyType.TIMEDURATION, + PropertyType.COUNT, "The number of consecutive failures at which the Compactor exits and the process terminates. A zero" + " value disables this feature.", "2.1.4"), 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 58b79005200..507122c077b 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 @@ -670,6 +670,47 @@ protected Collection getServiceTags(HostAndPort clientAddress) { clientAddress, queueName); } + private void performFailureProcessing(Map> errorHistory) + throws InterruptedException { + // consecutive failure processing + final long totalFailures = + errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); + if (totalFailures > 0) { + LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", totalFailures, + errorHistory); + final long failureThreshold = + getConfiguration().getCount(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD); + if (failureThreshold > 0 && totalFailures > failureThreshold) { + LOG.error("Consecutive failures ({}) has exceeded failure threshold ({}), exiting...", + totalFailures, failureThreshold); + throw new InterruptedException( + "Consecutive failures has exceeded failure threshold, exiting..."); + } + if (totalFailures + > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { + final long interval = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); + if (interval > 0) { + final long max = + getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); + final long backoffMS = Math.min(max, interval * totalFailures); + LOG.warn( + "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", + backoffMS); + if (backoffMS == max) { + errorHistory.clear(); + } + Thread.sleep(backoffMS); + } else if (interval == 0) { + LOG.info( + "This Compactor has had {} consecutive failures and failure backoff is disabled.", + totalFailures); + errorHistory.clear(); + } + } + } + } + @Override public void run() { @@ -717,43 +758,7 @@ public void run() { err.set(null); JOB_HOLDER.reset(); - // consecutive failure processing - final long totalFailures = - errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); - if (totalFailures > 0) { - LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", totalFailures, - errorHistory); - final long failureThreshold = - getConfiguration().getCount(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD); - if (failureThreshold > 0 && totalFailures > failureThreshold) { - LOG.error("Consecutive failures ({}) has exceeded failure threshold ({}), exiting...", - totalFailures, failureThreshold); - throw new InterruptedException( - "Consecutive failures has exceeded failure threshold, exiting..."); - } - if (totalFailures - > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { - final long interval = - getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); - if (interval > 0) { - final long max = - getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_RESET); - final long backoffMS = Math.min(max, interval * totalFailures); - LOG.warn( - "Not starting next compaction for {}ms due to consecutive failures. Check the log and address any issues.", - backoffMS); - if (backoffMS == max) { - errorHistory.clear(); - } - Thread.sleep(backoffMS); - } else if (interval == 0) { - LOG.info( - "This Compactor has had {} consecutive failures and failure backoff is disabled.", - totalFailures); - errorHistory.clear(); - } - } - } + performFailureProcessing(errorHistory); TExternalCompactionJob job; try { From 3d71032a9a259f443ba336f31d65b7ba67004302 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 15 Jul 2025 19:23:44 +0000 Subject: [PATCH 09/19] Modified error history to capture all exceptions, print nice summary --- .../apache/accumulo/compactor/Compactor.java | 62 ++++++++++++++++--- 1 file changed, 52 insertions(+), 10 deletions(-) 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 507122c077b..f7940520d33 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 @@ -95,7 +95,6 @@ import org.apache.accumulo.core.trace.TraceUtil; import org.apache.accumulo.core.trace.thrift.TInfo; import org.apache.accumulo.core.util.HostAndPort; -import org.apache.accumulo.core.util.Pair; import org.apache.accumulo.core.util.ServerServices; import org.apache.accumulo.core.util.ServerServices.Service; import org.apache.accumulo.core.util.UtilWaitThread; @@ -148,6 +147,54 @@ public interface FileCompactorRunnable extends Runnable { private static final SecureRandom random = new SecureRandom(); + private static class ErrorHistory extends HashMap> { + + private static final long serialVersionUID = 1L; + + public long getTotalFailures() { + long total = 0; + for (TableId tid : keySet()) { + total += getTotalTableFailures(tid); + } + return total; + } + + public long getTotalTableFailures(TableId tid) { + long total = 0; + for (AtomicLong failures : get(tid).values()) { + total += failures.get(); + } + return total; + } + + /** + * Add error for table, and return total number of failures for the table + * + * @param tid table id + * @param error exception + * @return total number of failures for table + */ + public long addError(TableId tid, Throwable error) { + computeIfAbsent(tid, t -> new HashMap()) + .computeIfAbsent(error, e -> new AtomicLong(0)).incrementAndGet(); + return getTotalTableFailures(tid); + } + + @Override + public String toString() { + StringBuilder buf = new StringBuilder(); + buf.append("\n"); + for (TableId tid : keySet()) { + buf.append("\tTable: ").append(tid).append("\n"); + for (Entry error : get(tid).entrySet()) { + buf.append("\t\tException: ").append(error.getKey()).append(", count: ") + .append(error.getValue().get()); + } + } + return buf.toString(); + } + } + public static class CompactorServerOpts extends ServerOpts { @Parameter(required = true, names = {"-q", "--queue"}, description = "compaction queue name") private String queueName = null; @@ -670,11 +717,9 @@ protected Collection getServiceTags(HostAndPort clientAddress) { clientAddress, queueName); } - private void performFailureProcessing(Map> errorHistory) - throws InterruptedException { + private void performFailureProcessing(ErrorHistory errorHistory) throws InterruptedException { // consecutive failure processing - final long totalFailures = - errorHistory.values().stream().mapToLong(p -> p.getSecond().get()).sum(); + final long totalFailures = errorHistory.getTotalFailures(); if (totalFailures > 0) { LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", totalFailures, errorHistory); @@ -743,7 +788,7 @@ public void run() { try { final AtomicReference err = new AtomicReference<>(); - final Map> errorHistory = new HashMap<>(); + final ErrorHistory errorHistory = new ErrorHistory(); while (!isShutdownRequested()) { if (Thread.currentThread().isInterrupted()) { @@ -873,10 +918,7 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job, err.get()); - long failures = errorHistory - .computeIfAbsent(fromThriftExtent.tableId(), - t -> new Pair<>(err.get(), new AtomicLong(0))) - .getSecond().incrementAndGet(); + long failures = errorHistory.addError(fromThriftExtent.tableId(), err.get()); LOG.warn("Compaction for table {} has failed {} times", fromThriftExtent.tableId(), failures); } catch (RetriesExceededException e) { From d45b2d6827d89580be7e6b13dbc00ad4672261c8 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 15 Jul 2025 20:33:01 +0000 Subject: [PATCH 10/19] Fixed ErrorHistory Log4J was not invoking ErrorHistory.toString when logging, so explicitly called toString when logging. Also, Throwable doesn't implement hashCode, so the HashMap wasn't working as expected. Changed the map key from Throwable to String to fix --- .../org/apache/accumulo/compactor/Compactor.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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 f7940520d33..c317abd9c2c 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 @@ -147,7 +147,7 @@ public interface FileCompactorRunnable extends Runnable { private static final SecureRandom random = new SecureRandom(); - private static class ErrorHistory extends HashMap> { + private static class ErrorHistory extends HashMap> { private static final long serialVersionUID = 1L; @@ -175,8 +175,8 @@ public long getTotalTableFailures(TableId tid) { * @return total number of failures for table */ public long addError(TableId tid, Throwable error) { - computeIfAbsent(tid, t -> new HashMap()) - .computeIfAbsent(error, e -> new AtomicLong(0)).incrementAndGet(); + computeIfAbsent(tid, t -> new HashMap()) + .computeIfAbsent(error.toString(), e -> new AtomicLong(0)).incrementAndGet(); return getTotalTableFailures(tid); } @@ -185,9 +185,9 @@ public String toString() { StringBuilder buf = new StringBuilder(); buf.append("\n"); for (TableId tid : keySet()) { - buf.append("\tTable: ").append(tid).append("\n"); - for (Entry error : get(tid).entrySet()) { - buf.append("\t\tException: ").append(error.getKey()).append(", count: ") + buf.append("Table: ").append(tid).append("\n"); + for (Entry error : get(tid).entrySet()) { + buf.append("\tException: ").append(error.getKey()).append(", count: ") .append(error.getValue().get()); } } @@ -722,7 +722,7 @@ private void performFailureProcessing(ErrorHistory errorHistory) throws Interrup final long totalFailures = errorHistory.getTotalFailures(); if (totalFailures > 0) { LOG.warn("This Compactor has had {} consecutive failures. Failures: {}", totalFailures, - errorHistory); + errorHistory.toString()); // ErrorHistory.toString not invoked without .toString final long failureThreshold = getConfiguration().getCount(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD); if (failureThreshold > 0 && totalFailures > failureThreshold) { From 9368643fdf2ed98db69c89d8ba6e340025d07ab6 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 15 Jul 2025 20:53:55 +0000 Subject: [PATCH 11/19] Updated IT --- .../apache/accumulo/compactor/Compactor.java | 5 ++-- .../ClassLoaderContextCompactionIT.java | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) 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 c317abd9c2c..25b1e442016 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 @@ -725,8 +725,9 @@ private void performFailureProcessing(ErrorHistory errorHistory) throws Interrup errorHistory.toString()); // ErrorHistory.toString not invoked without .toString final long failureThreshold = getConfiguration().getCount(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD); - if (failureThreshold > 0 && totalFailures > failureThreshold) { - LOG.error("Consecutive failures ({}) has exceeded failure threshold ({}), exiting...", + if (failureThreshold > 0 && totalFailures >= failureThreshold) { + LOG.error( + "Consecutive failures ({}) has met or exceeded failure threshold ({}), exiting...", totalFailures, failureThreshold); throw new InterruptedException( "Consecutive failures has exceeded failure threshold, exiting..."); diff --git a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java index 7727aa6e71f..830ac053224 100644 --- a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java +++ b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java @@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.EnumSet; +import java.util.List; import org.apache.accumulo.compactor.Compactor; import org.apache.accumulo.coordinator.CompactionCoordinator; @@ -41,6 +42,7 @@ import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; import org.apache.accumulo.core.metadata.schema.TabletsMetadata; +import org.apache.accumulo.core.util.HostAndPort; import org.apache.accumulo.core.util.compaction.ExternalCompactionUtil; import org.apache.accumulo.harness.AccumuloClusterHarness; import org.apache.accumulo.minicluster.ServerType; @@ -57,6 +59,10 @@ public class ClassLoaderContextCompactionIT extends AccumuloClusterHarness { @Override public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { ExternalCompactionTestUtils.configureMiniCluster(cfg, coreSite); + cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL, "15s"); + cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD, "2"); + cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_RESET, "10m"); + cfg.setProperty(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD, "3"); cfg.setNumCompactors(2); } @@ -68,6 +74,10 @@ public void testClassLoaderContextErrorKillsCompactor() throws Exception { getCluster().getClusterControl().startCompactors(Compactor.class, 1, QUEUE1); Wait.waitFor( () -> ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client) == 1); + List compactors = + ExternalCompactionUtil.getCompactorAddrs((ClientContext) client).get(QUEUE1); + assertEquals(1, compactors.size()); + final HostAndPort compactorAddr = compactors.get(0); createTable(client, table1, "cs1"); client.tableOperations().setProperty(table1, TABLE_FILE_MAX.getKey(), "1001"); client.tableOperations().setProperty(table1, TABLE_MAJC_RATIO.getKey(), "1001"); @@ -119,7 +129,27 @@ public void testClassLoaderContextErrorKillsCompactor() throws Exception { // Start a compaction. The invalid cache dir property should cause a failure client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); + Wait.waitFor( + () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) + == null); + assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); + client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); + Wait.waitFor( + () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) + == null); + assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); + + client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); + Wait.waitFor( + () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) + == null); + assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); + + client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); + Wait.waitFor( + () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) + == null); Wait.waitFor( () -> ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client) == 0); From f302e83eeefce33be644eae6586384ba665188c3 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 16 Jul 2025 11:54:39 +0000 Subject: [PATCH 12/19] Updated logging, addressed PR suggestions --- .../org/apache/accumulo/compactor/Compactor.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) 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 25b1e442016..cb0c45d2adf 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 @@ -168,26 +168,23 @@ public long getTotalTableFailures(TableId tid) { } /** - * Add error for table, and return total number of failures for the table + * Add error for table * * @param tid table id * @param error exception - * @return total number of failures for table */ - public long addError(TableId tid, Throwable error) { + public void addError(TableId tid, Throwable error) { computeIfAbsent(tid, t -> new HashMap()) .computeIfAbsent(error.toString(), e -> new AtomicLong(0)).incrementAndGet(); - return getTotalTableFailures(tid); } @Override public String toString() { StringBuilder buf = new StringBuilder(); - buf.append("\n"); for (TableId tid : keySet()) { - buf.append("Table: ").append(tid).append("\n"); + buf.append("\nTable: ").append(tid); for (Entry error : get(tid).entrySet()) { - buf.append("\tException: ").append(error.getKey()).append(", count: ") + buf.append("\n\tException: ").append(error.getKey()).append(", count: ") .append(error.getValue().get()); } } @@ -919,9 +916,7 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job, err.get()); - long failures = errorHistory.addError(fromThriftExtent.tableId(), err.get()); - LOG.warn("Compaction for table {} has failed {} times", fromThriftExtent.tableId(), - failures); + errorHistory.addError(fromThriftExtent.tableId(), err.get()); } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", job.getExternalCompactionId(), fromThriftExtent, e); From 04faeabc3af7a976834f72a29965b8f9ec5bec92 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 16 Jul 2025 12:17:05 +0000 Subject: [PATCH 13/19] Fix backoff condition, was off by one --- .../org/apache/accumulo/compactor/Compactor.java | 2 +- .../compaction/ClassLoaderContextCompactionIT.java | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) 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 cb0c45d2adf..da8fa2ab8d3 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 @@ -730,7 +730,7 @@ private void performFailureProcessing(ErrorHistory errorHistory) throws Interrup "Consecutive failures has exceeded failure threshold, exiting..."); } if (totalFailures - > getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { + >= getConfiguration().getCount(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD)) { final long interval = getConfiguration().getTimeInMillis(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL); if (interval > 0) { diff --git a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java index 830ac053224..b4440344ce4 100644 --- a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java +++ b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java @@ -59,8 +59,10 @@ public class ClassLoaderContextCompactionIT extends AccumuloClusterHarness { @Override public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { ExternalCompactionTestUtils.configureMiniCluster(cfg, coreSite); - cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL, "15s"); - cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD, "2"); + // After 1 failure start backing off by 5s. + // After 3 failures, terminate the Compactor + cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_THRESHOLD, "1"); + cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_INTERVAL, "5s"); cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_RESET, "10m"); cfg.setProperty(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD, "3"); cfg.setNumCompactors(2); @@ -127,7 +129,7 @@ public void testClassLoaderContextErrorKillsCompactor() throws Exception { // delete Test.jar, so that the classloader will fail assertTrue(fs.delete(dst, false)); - // Start a compaction. The invalid cache dir property should cause a failure + // Start a compaction. The missing jar should cause a failure client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); Wait.waitFor( () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) @@ -146,10 +148,7 @@ public void testClassLoaderContextErrorKillsCompactor() throws Exception { == null); assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); - client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); - Wait.waitFor( - () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) - == null); + // Three failures have occurred, Compactor should shut down. Wait.waitFor( () -> ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client) == 0); From 052ffffe558d77d9bde09cebfe0008aecc97b67f Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 16 Jul 2025 16:22:20 +0000 Subject: [PATCH 14/19] Added new metrics to Compactor to track success, cancellation, failure --- .../core/clientImpl/ClientContext.java | 4 + .../core/metrics/MetricsProducer.java | 6 ++ .../accumulo/server/AbstractServer.java | 6 +- .../apache/accumulo/server/ServerContext.java | 3 + .../coordinator/CompactionCoordinator.java | 1 + .../apache/accumulo/compactor/Compactor.java | 53 +++++++++- .../accumulo/gc/SimpleGarbageCollector.java | 1 + .../org/apache/accumulo/manager/Manager.java | 1 + .../apache/accumulo/tserver/ScanServer.java | 1 + .../apache/accumulo/tserver/TabletServer.java | 2 +- .../ClassLoaderContextCompactionIT.java | 98 +++++++++++++++++++ 11 files changed, 171 insertions(+), 5 deletions(-) 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 c2bcabe01e6..e5b059a80f7 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 @@ -163,6 +163,10 @@ private void ensureOpen() { } } + protected boolean isClosed() { + return closed; + } + private ScanServerSelector createScanServerSelector() { String clazz = ClientProperty.SCAN_SERVER_SELECTOR.getValue(info.getProperties()); try { diff --git a/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java b/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java index c98a2f5c61f..f4fcfd8a0ca 100644 --- a/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java +++ b/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java @@ -625,6 +625,12 @@ public interface MetricsProducer { String METRICS_COMPACTOR_MAJC_STUCK = METRICS_COMPACTOR_PREFIX + "majc.stuck"; String METRICS_COMPACTOR_ENTRIES_READ = METRICS_COMPACTOR_PREFIX + "entries.read"; String METRICS_COMPACTOR_ENTRIES_WRITTEN = METRICS_COMPACTOR_PREFIX + "entries.written"; + String METRICS_COMPACTOR_COMPACTIONS_CANCELLED = METRICS_COMPACTOR_PREFIX + "majc.cancelled"; + String METRICS_COMPACTOR_COMPACTIONS_COMPLETED = METRICS_COMPACTOR_PREFIX + "majc.completed"; + String METRICS_COMPACTOR_COMPACTIONS_FAILED = METRICS_COMPACTOR_PREFIX + "majc.failed"; + String METRICS_COMPACTOR_FAILURES_CONSECUTIVE = + METRICS_COMPACTOR_PREFIX + "majc.failures.consecutive"; + String METRICS_COMPACTOR_FAILURES_TERMINATION = METRICS_COMPACTOR_PREFIX + "terminated"; String METRICS_FATE_PREFIX = "accumulo.fate."; String METRICS_FATE_TYPE_IN_PROGRESS = METRICS_FATE_PREFIX + "ops.in.progress.by.type"; 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 cd5de7c442c..b2122caafd9 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 @@ -322,7 +322,11 @@ public void startServiceLockVerificationThread() { } @Override - public void close() {} + public void close() { + if (context != null) { + context.close(); + } + } protected void waitForUpgrade() throws InterruptedException { while (AccumuloDataVersion.getCurrentVersion(getContext()) < AccumuloDataVersion.get()) { 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 a2a1f93fd47..30543aac24a 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 @@ -84,6 +84,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Preconditions; + /** * Provides a server context for Accumulo server components that operate with the system credentials * and have access to the system files and configuration. @@ -461,6 +463,7 @@ public MetricsInfo getMetricsInfo() { @Override public void close() { + Preconditions.checkState(!isClosed(), "ServerContext.close was already called."); getMetricsInfo().close(); super.close(); } diff --git a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java index feb4b3d6e46..bfb144c1e95 100644 --- a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java +++ b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java @@ -359,6 +359,7 @@ public void run() { if (coordinatorAddress.server != null) { coordinatorAddress.server.stop(); } + super.close(); getShutdownComplete().set(true); LOG.info("stop requested. exiting ... "); try { 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 da8fa2ab8d3..a317525b50f 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 @@ -126,6 +126,7 @@ import com.google.common.base.Preconditions; import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.LongTaskTimer; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; @@ -147,7 +148,7 @@ public interface FileCompactorRunnable extends Runnable { private static final SecureRandom random = new SecureRandom(); - private static class ErrorHistory extends HashMap> { + private static class ConsecutiveErrorHistory extends HashMap> { private static final long serialVersionUID = 1L; @@ -218,6 +219,11 @@ public String getQueueName() { private ServerAddress compactorAddress = null; private final AtomicBoolean compactionRunning = new AtomicBoolean(false); + private final ConsecutiveErrorHistory errorHistory = new ConsecutiveErrorHistory(); + private final AtomicLong completed = new AtomicLong(0); + private final AtomicLong cancelled = new AtomicLong(0); + private final AtomicLong failed = new AtomicLong(0); + private final AtomicLong terminated = new AtomicLong(0); protected Compactor(CompactorServerOpts opts, String[] args) { super("compactor", opts, args); @@ -233,6 +239,26 @@ private long getTotalEntriesWritten() { return FileCompactor.getTotalEntriesWritten(); } + private double getConsecutiveFailures() { + return errorHistory.getTotalFailures(); + } + + private double getCancellations() { + return cancelled.get(); + } + + private double getCompletions() { + return completed.get(); + } + + private double getFailures() { + return failed.get(); + } + + private double getTerminated() { + return terminated.get(); + } + @Override public void registerMetrics(MeterRegistry registry) { super.registerMetrics(registry); @@ -243,6 +269,22 @@ public void registerMetrics(MeterRegistry registry) { .builder(METRICS_COMPACTOR_ENTRIES_WRITTEN, this, Compactor::getTotalEntriesWritten) .description("Number of entries written by all compactions that have run on this compactor") .register(registry); + FunctionCounter + .builder(METRICS_COMPACTOR_COMPACTIONS_CANCELLED, this, Compactor::getCancellations) + .description("Number compactions that have been cancelled on this compactor") + .register(registry); + FunctionCounter + .builder(METRICS_COMPACTOR_COMPACTIONS_COMPLETED, this, Compactor::getCompletions) + .description("Number compactions that have succeeded on this compactor").register(registry); + FunctionCounter.builder(METRICS_COMPACTOR_COMPACTIONS_FAILED, this, Compactor::getFailures) + .description("Number compactions that have failed on this compactor").register(registry); + FunctionCounter.builder(METRICS_COMPACTOR_FAILURES_TERMINATION, this, Compactor::getTerminated) + .description("Will report 1 if the Compactor terminates due to consecutive failure, else 0") + .register(registry); + Gauge.builder(METRICS_COMPACTOR_FAILURES_CONSECUTIVE, this, Compactor::getConsecutiveFailures) + .description( + "Number of consecutive compaction failures. Resets to zero on a successful compaction") + .register(registry); LongTaskTimer timer = LongTaskTimer.builder(METRICS_COMPACTOR_MAJC_STUCK) .description("Number and duration of stuck major compactions").register(registry); CompactionWatcher.setTimer(timer); @@ -714,7 +756,8 @@ protected Collection getServiceTags(HostAndPort clientAddress) { clientAddress, queueName); } - private void performFailureProcessing(ErrorHistory errorHistory) throws InterruptedException { + private void performFailureProcessing(ConsecutiveErrorHistory errorHistory) + throws InterruptedException { // consecutive failure processing final long totalFailures = errorHistory.getTotalFailures(); if (totalFailures > 0) { @@ -726,6 +769,7 @@ private void performFailureProcessing(ErrorHistory errorHistory) throws Interrup LOG.error( "Consecutive failures ({}) has met or exceeded failure threshold ({}), exiting...", totalFailures, failureThreshold); + terminated.incrementAndGet(); throw new InterruptedException( "Consecutive failures has exceeded failure threshold, exiting..."); } @@ -786,7 +830,6 @@ public void run() { try { final AtomicReference err = new AtomicReference<>(); - final ErrorHistory errorHistory = new ErrorHistory(); while (!isShutdownRequested()) { if (Thread.currentThread().isInterrupted()) { @@ -901,6 +944,7 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job, null); + cancelled.incrementAndGet(); } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction cancellation.", e); } finally { @@ -916,6 +960,7 @@ public void run() { -1, -1, -1, fcr.getCompactionAge().toNanos()); updateCompactionState(job, update); updateCompactionFailed(job, err.get()); + failed.incrementAndGet(); errorHistory.addError(fromThriftExtent.tableId(), err.get()); } catch (RetriesExceededException e) { LOG.error("Error updating coordinator with compaction failure: id: {}, extent: {}", @@ -927,6 +972,7 @@ public void run() { 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) { @@ -990,6 +1036,7 @@ public void run() { } gcLogger.logGCInfo(getConfiguration()); + super.close(); getShutdownComplete().set(true); LOG.info("stop requested. exiting ... "); try { 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 a7cfca69b8d..902642a6c03 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 @@ -344,6 +344,7 @@ public void run() { gracefulShutdown(getContext().rpcCreds()); } } + super.close(); getShutdownComplete().set(true); log.info("stop requested. exiting ... "); try { 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 c4914fc0ff6..a2abec19929 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 @@ -1498,6 +1498,7 @@ boolean canSuspendTablets() { throw new IllegalStateException("Exception waiting on watcher", e); } } + super.close(); getShutdownComplete().set(true); log.info("stop requested. exiting ... "); try { 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 78ddee2b855..ec89e668af9 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 @@ -463,6 +463,7 @@ public void run() { } gcLogger.logGCInfo(getConfiguration()); + super.close(); getShutdownComplete().set(true); LOG.info("stop requested. exiting ... "); try { 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 0cf5c2ec473..e66f6ae2d4a 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 @@ -1044,7 +1044,7 @@ public void run() { } gcLogger.logGCInfo(getConfiguration()); - + super.close(); getShutdownComplete().set(true); log.info("TServerInfo: stop requested. exiting ... "); try { diff --git a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java index b4440344ce4..15a539bf5ce 100644 --- a/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java +++ b/test/src/main/java/org/apache/accumulo/test/compaction/ClassLoaderContextCompactionIT.java @@ -27,6 +27,9 @@ import java.util.EnumSet; import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import org.apache.accumulo.compactor.Compactor; import org.apache.accumulo.coordinator.CompactionCoordinator; @@ -42,20 +45,43 @@ import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; import org.apache.accumulo.core.metadata.schema.TabletsMetadata; +import org.apache.accumulo.core.metrics.MetricsProducer; +import org.apache.accumulo.core.spi.metrics.LoggingMeterRegistryFactory; import org.apache.accumulo.core.util.HostAndPort; import org.apache.accumulo.core.util.compaction.ExternalCompactionUtil; +import org.apache.accumulo.core.util.threads.Threads; import org.apache.accumulo.harness.AccumuloClusterHarness; import org.apache.accumulo.minicluster.ServerType; import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl; import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.metrics.TestStatsDRegistryFactory; +import org.apache.accumulo.test.metrics.TestStatsDSink; +import org.apache.accumulo.test.metrics.TestStatsDSink.Metric; import org.apache.accumulo.test.util.Wait; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class ClassLoaderContextCompactionIT extends AccumuloClusterHarness { + private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderContextCompactionIT.class); + private static TestStatsDSink sink; + + @BeforeAll + public static void before() throws Exception { + sink = new TestStatsDSink(); + } + + @AfterAll + public static void after() throws Exception { + sink.close(); + } + @Override public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { ExternalCompactionTestUtils.configureMiniCluster(cfg, coreSite); @@ -66,10 +92,63 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreS cfg.setProperty(Property.COMPACTOR_FAILURE_BACKOFF_RESET, "10m"); cfg.setProperty(Property.COMPACTOR_FAILURE_TERMINATION_THRESHOLD, "3"); cfg.setNumCompactors(2); + // Tell the server processes to use a StatsDMeterRegistry and the simple logging registry + // that will be configured to push all metrics to the sink we started. + cfg.setProperty(Property.GENERAL_MICROMETER_ENABLED, "true"); + cfg.setProperty(Property.GENERAL_MICROMETER_JVM_METRICS_ENABLED, "true"); + cfg.setProperty("general.custom.metrics.opts.logging.step", "1s"); + String clazzList = LoggingMeterRegistryFactory.class.getName() + "," + + TestStatsDRegistryFactory.class.getName(); + cfg.setProperty(Property.GENERAL_MICROMETER_FACTORY, clazzList); + Map sysProps = Map.of(TestStatsDRegistryFactory.SERVER_HOST, "127.0.0.1", + TestStatsDRegistryFactory.SERVER_PORT, Integer.toString(sink.getPort())); + cfg.setSystemProperties(sysProps); } @Test public void testClassLoaderContextErrorKillsCompactor() throws Exception { + + final AtomicBoolean shutdownTailer = new AtomicBoolean(false); + final AtomicLong cancellations = new AtomicLong(0); + final AtomicLong completions = new AtomicLong(0); + final AtomicLong failures = new AtomicLong(0); + final AtomicLong consecutive = new AtomicLong(0); + final AtomicLong terminations = new AtomicLong(0); + + final Thread thread = Threads.createNonCriticalThread("metric-tailer", () -> { + while (!shutdownTailer.get()) { + List statsDMetrics = sink.getLines(); + for (String s : statsDMetrics) { + if (shutdownTailer.get()) { + break; + } + if (s.startsWith(MetricsProducer.METRICS_COMPACTOR_COMPACTIONS_CANCELLED)) { + Metric m = TestStatsDSink.parseStatsDMetric(s); + LOG.info("{}", m); + cancellations.set(Long.parseLong(m.getValue())); + } else if (s.startsWith(MetricsProducer.METRICS_COMPACTOR_COMPACTIONS_COMPLETED)) { + Metric m = TestStatsDSink.parseStatsDMetric(s); + LOG.info("{}", m); + completions.set(Long.parseLong(m.getValue())); + } else if (s.startsWith(MetricsProducer.METRICS_COMPACTOR_COMPACTIONS_FAILED)) { + Metric m = TestStatsDSink.parseStatsDMetric(s); + LOG.info("{}", m); + failures.set(Long.parseLong(m.getValue())); + } else if (s.startsWith(MetricsProducer.METRICS_COMPACTOR_FAILURES_TERMINATION)) { + Metric m = TestStatsDSink.parseStatsDMetric(s); + LOG.info("{}", m); + terminations.set(Long.parseLong(m.getValue())); + } else if (s.startsWith(MetricsProducer.METRICS_COMPACTOR_FAILURES_CONSECUTIVE)) { + Metric m = TestStatsDSink.parseStatsDMetric(s); + LOG.info("{}", m); + consecutive.set(Long.parseLong(m.getValue())); + } + + } + } + }); + thread.start(); + final String table1 = this.getUniqueNames(1)[0]; try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) { getCluster().getClusterControl().startCoordinator(CompactionCoordinator.class); @@ -129,30 +208,49 @@ public void testClassLoaderContextErrorKillsCompactor() throws Exception { // delete Test.jar, so that the classloader will fail assertTrue(fs.delete(dst, false)); + assertEquals(0, cancellations.get()); + assertEquals(0, completions.get()); + assertEquals(0, failures.get()); + assertEquals(0, terminations.get()); + assertEquals(0, consecutive.get()); + // Start a compaction. The missing jar should cause a failure client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); Wait.waitFor( () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) == null); assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); + Wait.waitFor(() -> failures.get() == 1); + Wait.waitFor(() -> consecutive.get() == 1); + Wait.waitFor(() -> failures.get() == 0); client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); Wait.waitFor( () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) == null); assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); + Wait.waitFor(() -> failures.get() == 1); + Wait.waitFor(() -> consecutive.get() == 2); + Wait.waitFor(() -> failures.get() == 0); client.tableOperations().compact(table1, new CompactionConfig().setWait(false)); Wait.waitFor( () -> ExternalCompactionUtil.getRunningCompaction(compactorAddr, (ClientContext) client) == null); assertEquals(1, ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client)); + Wait.waitFor(() -> failures.get() == 1); + Wait.waitFor(() -> consecutive.get() == 3); // Three failures have occurred, Compactor should shut down. Wait.waitFor( () -> ExternalCompactionUtil.countCompactors(QUEUE1, (ClientContext) client) == 0); + Wait.waitFor(() -> terminations.get() == 1); + assertEquals(0, cancellations.get()); + assertEquals(0, completions.get()); } finally { + shutdownTailer.set(true); + thread.join(); getCluster().getClusterControl().stopAllServers(ServerType.COMPACTOR); getCluster().getClusterControl().stopAllServers(ServerType.COMPACTION_COORDINATOR); } From daf32fc1ed3b541cca288ff850b735cedacfeac6 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 16 Jul 2025 16:32:13 +0000 Subject: [PATCH 15/19] Updated MetricsProducer javadoc for new metrics --- .../core/metrics/MetricsProducer.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java b/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java index f4fcfd8a0ca..ae6bec42899 100644 --- a/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java +++ b/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java @@ -73,6 +73,41 @@ * FunctionCounter * Number of entries written by all threads performing compactions * + * + * N/A + * N/A + * {@value #METRICS_COMPACTOR_COMPACTIONS_CANCELLED} + * FunctionCounter + * Number of compactions cancelled on a compactor + * + * + * N/A + * N/A + * {@value #METRICS_COMPACTOR_COMPACTIONS_COMPLETED} + * FunctionCounter + * Number of compactions completed on a compactor + * + * + * N/A + * N/A + * {@value #METRICS_COMPACTOR_COMPACTIONS_FAILED} + * FunctionCounter + * Number of compactions failed on a compactor + * + * + * N/A + * N/A + * {@value #METRICS_COMPACTOR_FAILURES_CONSECUTIVE} + * Gauge + * Number of consecutive compaction failures on a compactor + * + * + * N/A + * N/A + * {@value #METRICS_COMPACTOR_FAILURES_TERMINATION} + * Gauge + * Number of Compactors terminated due to consecutive failures + * * * * currentFateOps From d6c8aaca3a911ce303a6a878221d1fe0e6f9ba36 Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Wed, 16 Jul 2025 15:18:28 -0400 Subject: [PATCH 16/19] log success and failure counts (#56) --- .../coordinator/CompactionCoordinator.java | 107 +++++++++++------- 1 file changed, 67 insertions(+), 40 deletions(-) diff --git a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java index bfb144c1e95..873ce2abd9b 100644 --- a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java +++ b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java @@ -35,7 +35,6 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import org.apache.accumulo.coordinator.QueueSummaries.PrioTserver; @@ -97,6 +96,7 @@ import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.event.Level; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; @@ -132,9 +132,36 @@ public class CompactionCoordinator extends AbstractServer implements /* Map of queue name to last time compactor called to get a compaction job */ private static final Map TIME_COMPACTOR_LAST_CHECKED = new ConcurrentHashMap<>(); - private final ConcurrentHashMap failingQueues = new ConcurrentHashMap<>(); - private final ConcurrentHashMap failingCompactors = new ConcurrentHashMap<>(); - private final ConcurrentHashMap failingTables = new ConcurrentHashMap<>(); + static class FailureCounts { + long failures; + long successes; + + FailureCounts(long failures, long successes) { + this.failures = failures; + this.successes = successes; + } + + static FailureCounts incrementFailure(Object key, FailureCounts counts) { + if (counts == null) { + return new FailureCounts(1, 0); + } + counts.failures++; + return counts; + } + + static FailureCounts incrementSuccess(Object key, FailureCounts counts) { + if (counts == null) { + return new FailureCounts(0, 1); + } + counts.successes++; + return counts; + } + } + + private final ConcurrentHashMap failingQueues = new ConcurrentHashMap<>(); + private final ConcurrentHashMap failingCompactors = + new ConcurrentHashMap<>(); + private final ConcurrentHashMap failingTables = new ConcurrentHashMap<>(); private final GarbageCollectionLogger gcLogger = new GarbageCollectionLogger(); protected AuditedSecurityOperation security; @@ -641,20 +668,45 @@ private void captureFailure(ExternalCompactionId ecid, KeyExtent extent) { var rc = RUNNING_CACHE.get(ecid); if (rc != null) { final String queue = rc.getQueueName(); - failingQueues.computeIfAbsent(queue, q -> new AtomicLong(0)).incrementAndGet(); + failingQueues.compute(queue, FailureCounts::incrementFailure); final String compactor = rc.getCompactorAddress(); - failingCompactors.computeIfAbsent(compactor, c -> new AtomicLong(0)).incrementAndGet(); + failingCompactors.compute(compactor, FailureCounts::incrementFailure); } - failingTables.computeIfAbsent(extent.tableId(), t -> new AtomicLong(0)).incrementAndGet(); + failingTables.compute(extent.tableId(), FailureCounts::incrementFailure); } protected void startFailureSummaryLogging() { ScheduledFuture future = getContext().getScheduledExecutor() - .scheduleWithFixedDelay(this::printFailures, 0, 5, TimeUnit.MINUTES); + .scheduleWithFixedDelay(this::printStats, 0, 5, TimeUnit.MINUTES); ThreadPools.watchNonCriticalScheduledTask(future); } - private void printFailures() { + private void printStats(String logPrefix, ConcurrentHashMap failureCounts, + boolean logSuccessAtTrace) { + for (var key : failureCounts.keySet()) { + failureCounts.compute(key, (k, counts) -> { + if (counts != null) { + Level level; + if (counts.failures > 0) { + level = Level.WARN; + } else if (logSuccessAtTrace) { + level = Level.TRACE; + } else { + level = Level.DEBUG; + } + + LOG.atLevel(level).log("{} {} failures:{} successes:{} since last time this was logged ", + logPrefix, k, counts.failures, counts.successes); + } + + // clear the counts so they can start building up for the next logging if this key is ever + // used again + return null; + }); + } + } + + private void printStats() { // Remove down compactors from failing list Map> allCompactors = @@ -663,45 +715,20 @@ private void printFailures() { allCompactors.values().forEach(l -> l.forEach(c -> allCompactorAddrs.add(c.toString()))); failingCompactors.keySet().retainAll(allCompactorAddrs); - LOG.warn("Compaction failure summary:"); - LOG.warn("Queue failures: {}", failingQueues); - LOG.warn("Table failures: {}", failingTables); - LOG.warn("Compactor failures: {}", failingCompactors); + printStats("Queue", failingQueues, false); + printStats("Table", failingTables, false); + printStats("Compactor", failingCompactors, true); } private void captureSuccess(ExternalCompactionId ecid, KeyExtent extent) { var rc = RUNNING_CACHE.get(ecid); if (rc != null) { final String queue = rc.getQueueName(); - failingQueues.computeIfPresent(queue, (q, curr) -> { - long currentValue = Math.max(0L, curr.get() - 1); - if (currentValue == 0) { - return null; // remove from the map - } else { - curr.set(currentValue); - return curr; - } - }); + failingQueues.compute(queue, FailureCounts::incrementSuccess); final String compactor = rc.getCompactorAddress(); - failingCompactors.computeIfPresent(compactor, (c, curr) -> { - long currentValue = Math.max(0L, curr.get() - 1); - if (currentValue == 0) { - return null; // remove from the map - } else { - curr.set(currentValue); - return curr; - } - }); + failingCompactors.compute(compactor, FailureCounts::incrementSuccess); } - failingTables.computeIfPresent(extent.tableId(), (t, curr) -> { - long currentValue = Math.max(0L, curr.get() - 1); - if (currentValue == 0) { - return null; // remove from the map - } else { - curr.set(currentValue); - return curr; - } - }); + failingTables.compute(extent.tableId(), FailureCounts::incrementSuccess); } void compactionFailed(Map compactions) { From 22fe25744d0bc59e7e521f9797c9f7d44aa453ae Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 16 Jul 2025 19:55:13 +0000 Subject: [PATCH 17/19] Created new exception class for ContextClassLoaderFactory --- .../core/classloader/ClassLoaderUtil.java | 22 ++++++++++----- .../DefaultContextClassLoaderFactory.java | 10 +++++-- .../accumulo/core/conf/ConfigCheckUtil.java | 3 +- .../core/conf/ConfigurationTypeHelper.java | 5 ++-- .../core/iterators/TypedValueCombiner.java | 2 +- .../iteratorsImpl/IteratorConfigUtil.java | 2 +- .../conf/ColumnToClassMapping.java | 5 ++-- .../core/sample/impl/SamplerFactory.java | 2 +- .../spi/common/ContextClassLoaderFactory.java | 28 +++++++++++++++---- .../core/spi/crypto/CryptoServiceFactory.java | 3 +- .../core/summary/SummarizerFactory.java | 7 ++--- .../ContextClassLoaderFactoryTest.java | 4 +-- .../accumulo/core/crypto/CryptoTest.java | 2 +- .../server/ServiceEnvironmentImpl.java | 6 ++-- .../server/client/ClientServiceHandler.java | 2 +- .../server/metrics/MetricsInfoImpl.java | 5 ++-- .../tserver/tablet/CompactableUtils.java | 2 +- .../java/org/apache/accumulo/shell/Shell.java | 5 ++-- .../shell/commands/SetIterCommand.java | 3 +- .../shell/commands/SetScanIterCommand.java | 3 +- .../shell/commands/SetShellIterCommand.java | 3 +- 21 files changed, 74 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java index 5ecc498f2db..63221a92578 100644 --- a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java @@ -23,6 +23,7 @@ import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory; +import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.accumulo.core.util.ConfigurationImpl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -76,12 +77,15 @@ public static synchronized void resetContextFactoryForTests() { } @SuppressWarnings("deprecation") - public static ClassLoader getClassLoader(String context) - throws IOException, ReflectiveOperationException { + public static ClassLoader getClassLoader(String context) throws ContextClassLoaderException { if (context != null && !context.isEmpty()) { return FACTORY.getClassLoader(context); } else { - return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader.getClassLoader(); + try { + return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader.getClassLoader(); + } catch (IOException e) { + throw new ContextClassLoaderException(context, e); + } } } @@ -95,7 +99,7 @@ public static boolean isValidContext(String context) { return false; } return true; - } catch (IOException | ReflectiveOperationException e) { + } catch (ContextClassLoaderException e) { LOG.debug("Context {} is not valid.", context, e); return false; } @@ -105,12 +109,16 @@ public static boolean isValidContext(String context) { } public static Class loadClass(String context, String className, - Class extension) throws IOException, ReflectiveOperationException { - return getClassLoader(context).loadClass(className).asSubclass(extension); + Class extension) throws ReflectiveOperationException { + try { + return getClassLoader(context).loadClass(className).asSubclass(extension); + } catch (ContextClassLoaderException e) { + throw new ClassNotFoundException("Error loading class from context: " + context, e); + } } public static Class loadClass(String className, Class extension) - throws IOException, ReflectiveOperationException { + throws ReflectiveOperationException { return loadClass(null, className, extension); } diff --git a/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java b/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java index be22708c15e..84b76c226af 100644 --- a/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java @@ -98,8 +98,12 @@ private static void removeUnusedContexts(Set contextsInUse) { @SuppressWarnings("deprecation") @Override - public ClassLoader getClassLoader(String contextName) throws IOException { - return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader - .getContextClassLoader(contextName); + public ClassLoader getClassLoader(String contextName) throws ContextClassLoaderException { + try { + return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader + .getContextClassLoader(contextName); + } catch (IOException e) { + throw new ContextClassLoaderException(contextName, e); + } } } diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java index eba5cc420f3..db5cb305a6d 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.core.conf; -import java.io.IOException; import java.util.Map.Entry; import java.util.Objects; @@ -160,7 +159,7 @@ private static void verifyValidClassName(String confOption, String className, Class requiredBaseClass) { try { ConfigurationTypeHelper.getClassInstance(null, className, requiredBaseClass); - } catch (IOException | ReflectiveOperationException e) { + } catch (ReflectiveOperationException e) { fatal(confOption + " has an invalid class name: " + className); } catch (ClassCastException e) { fatal(confOption + " must implement " + requiredBaseClass diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationTypeHelper.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationTypeHelper.java index cd286c1cd71..ae0d4088e5c 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationTypeHelper.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationTypeHelper.java @@ -24,7 +24,6 @@ import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; -import java.io.IOException; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; @@ -175,7 +174,7 @@ public static T getClassInstance(String context, String clazzName, Class try { instance = getClassInstance(context, clazzName, base); - } catch (RuntimeException | IOException | ReflectiveOperationException e) { + } catch (RuntimeException | ReflectiveOperationException e) { log.error("Failed to load class {} in classloader context {}", clazzName, context, e); } @@ -196,7 +195,7 @@ public static T getClassInstance(String context, String clazzName, Class * @return a new instance of the class */ public static T getClassInstance(String context, String clazzName, Class base) - throws IOException, ReflectiveOperationException { + throws ReflectiveOperationException { T instance; Class clazz = ClassLoaderUtil.loadClass(context, clazzName, base); diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java b/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java index df0cc359636..12656720a87 100644 --- a/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java +++ b/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java @@ -132,7 +132,7 @@ protected void setEncoder(String encoderClass) { Class> clazz = (Class>) ClassLoaderUtil.loadClass(encoderClass, Encoder.class); encoder = clazz.getDeclaredConstructor().newInstance(); - } catch (IOException | ReflectiveOperationException e) { + } catch (ReflectiveOperationException e) { throw new IllegalArgumentException(e); } } diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java index 85369129966..93ac01fbbc9 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java @@ -236,7 +236,7 @@ public static SortedKeyValueIterator convertItersAndLoad(IteratorScop } private static Class> loadClass(boolean useAccumuloClassLoader, - String context, IterInfo iterInfo) throws IOException, ReflectiveOperationException { + String context, IterInfo iterInfo) throws ReflectiveOperationException { if (useAccumuloClassLoader) { @SuppressWarnings("unchecked") var clazz = (Class>) ClassLoaderUtil.loadClass(context, diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/conf/ColumnToClassMapping.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/conf/ColumnToClassMapping.java index 1e6ff268b0f..419aecc5cbf 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/conf/ColumnToClassMapping.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/conf/ColumnToClassMapping.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.core.iteratorsImpl.conf; -import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -44,12 +43,12 @@ public ColumnToClassMapping() { } public ColumnToClassMapping(Map objectStrings, Class c) - throws ReflectiveOperationException, IOException { + throws ReflectiveOperationException { this(objectStrings, c, null); } public ColumnToClassMapping(Map objectStrings, Class c, - String context) throws ReflectiveOperationException, IOException { + String context) throws ReflectiveOperationException { this(); for (Entry entry : objectStrings.entrySet()) { diff --git a/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java b/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java index 7f2d289879a..7d7e1ab0cb1 100644 --- a/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java @@ -48,7 +48,7 @@ public static Sampler newSampler(SamplerConfigurationImpl config, AccumuloConfig return sampler; - } catch (ReflectiveOperationException | IOException | RuntimeException e) { + } catch (ReflectiveOperationException | RuntimeException e) { log.error("Cannot initialize sampler {}: {}", config.getClassName(), e.getMessage(), e); return null; } diff --git a/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java b/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java index 481f5ae4739..950b0a59e1c 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java @@ -18,8 +18,6 @@ */ package org.apache.accumulo.core.spi.common; -import java.io.IOException; - /** * The ContextClassLoaderFactory provides a mechanism for various Accumulo components to use a * custom ClassLoader for specific contexts, such as loading table iterators. This factory is @@ -49,6 +47,26 @@ */ public interface ContextClassLoaderFactory { + class ContextClassLoaderException extends Exception { + + private static final long serialVersionUID = 1L; + private static final String msg = "Error getting classloader for context: "; + + public ContextClassLoaderException(String context, Throwable cause, boolean enableSuppression, + boolean writableStackTrace) { + super(msg + context, cause, enableSuppression, writableStackTrace); + } + + public ContextClassLoaderException(String context, Throwable cause) { + super(msg + context, cause); + } + + public ContextClassLoaderException(String context) { + super(msg + context); + } + + } + /** * Pass the service environment to allow for additional class loader configuration * @@ -59,8 +77,8 @@ default void init(ContextClassLoaderEnvironment env) {} /** * Get the class loader for the given contextName. Callers should not cache the ClassLoader result * as it may change if/when the ClassLoader reloads. Implementations should throw a - * RuntimeException of some type (such as IllegalArgumentException) if the provided contextName is - * not supported or fails to be constructed. + * ContextClassLoaderException if the provided contextName is not supported or fails to be + * constructed. * * @param contextName the name of the context that represents a class loader that is managed by * this factory. Currently, Accumulo will only call this method for non-null and non-empty @@ -68,5 +86,5 @@ default void init(ContextClassLoaderEnvironment env) {} * consulting this plugin. * @return the class loader for the given contextName */ - ClassLoader getClassLoader(String contextName) throws IOException, ReflectiveOperationException; + ClassLoader getClassLoader(String contextName) throws ContextClassLoaderException; } diff --git a/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java b/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java index 4fc6af3585a..d3683326b17 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/crypto/CryptoServiceFactory.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.core.spi.crypto; -import java.io.IOException; import java.util.Map; import org.apache.accumulo.core.classloader.ClassLoaderUtil; @@ -47,7 +46,7 @@ default CryptoService newCryptoService(String cryptoServiceName) { try { return ClassLoaderUtil.loadClass(null, cryptoServiceName, CryptoService.class) .getDeclaredConstructor().newInstance(); - } catch (ReflectiveOperationException | IOException e) { + } catch (ReflectiveOperationException e) { throw new RuntimeException(e); } } diff --git a/core/src/main/java/org/apache/accumulo/core/summary/SummarizerFactory.java b/core/src/main/java/org/apache/accumulo/core/summary/SummarizerFactory.java index 48b123b3a7f..2ab51b6323d 100644 --- a/core/src/main/java/org/apache/accumulo/core/summary/SummarizerFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/summary/SummarizerFactory.java @@ -18,8 +18,6 @@ */ package org.apache.accumulo.core.summary; -import java.io.IOException; - import org.apache.accumulo.core.classloader.ClassLoaderUtil; import org.apache.accumulo.core.client.summary.Summarizer; import org.apache.accumulo.core.client.summary.SummarizerConfiguration; @@ -41,8 +39,7 @@ public SummarizerFactory(AccumuloConfiguration tableConfig) { this.context = ClassLoaderUtil.tableContext(tableConfig); } - private Summarizer newSummarizer(String classname) - throws IOException, ReflectiveOperationException { + private Summarizer newSummarizer(String classname) throws ReflectiveOperationException { if (classloader != null) { return classloader.loadClass(classname).asSubclass(Summarizer.class).getDeclaredConstructor() .newInstance(); @@ -55,7 +52,7 @@ private Summarizer newSummarizer(String classname) public Summarizer getSummarizer(SummarizerConfiguration conf) { try { return newSummarizer(conf.getClassName()); - } catch (ReflectiveOperationException | IOException e) { + } catch (ReflectiveOperationException e) { throw new RuntimeException(e); } } diff --git a/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java b/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java index 25f0c4390ab..84ee2f02844 100644 --- a/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java +++ b/core/src/test/java/org/apache/accumulo/core/classloader/ContextClassLoaderFactoryTest.java @@ -22,13 +22,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; -import java.io.IOException; import java.net.URLClassLoader; import java.util.Objects; import org.apache.accumulo.core.WithTestNames; import org.apache.accumulo.core.conf.ConfigurationCopy; import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -65,7 +65,7 @@ public void setup() throws Exception { } @Test - public void differentContexts() throws IOException, ReflectiveOperationException { + public void differentContexts() throws ContextClassLoaderException { ConfigurationCopy cc = new ConfigurationCopy(); cc.set(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY.getKey(), diff --git a/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java b/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java index 43a04aa53d5..08ef7f660fd 100644 --- a/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java +++ b/core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java @@ -388,7 +388,7 @@ public void testReadNoCryptoWithCryptoConfigured() throws Exception { } @Test - public void testMissingConfigProperties() throws ReflectiveOperationException, IOException { + public void testMissingConfigProperties() throws ReflectiveOperationException { var cryptoProps = getAllCryptoProperties(ConfigMode.CRYPTO_TABLE_ON); var droppedProperty = cryptoProps.remove(AESCryptoService.KEY_URI_PROPERTY); assertNotNull(droppedProperty); diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java b/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java index 0f68a11d487..46c6fdb7bd5 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.server; -import java.io.IOException; import java.util.concurrent.TimeUnit; import org.apache.accumulo.core.classloader.ClassLoaderUtil; @@ -62,14 +61,13 @@ public String getTableName(TableId tableId) throws TableNotFoundException { } @Override - public T instantiate(String className, Class base) - throws ReflectiveOperationException, IOException { + public T instantiate(String className, Class base) throws ReflectiveOperationException { return ConfigurationTypeHelper.getClassInstance(null, className, base); } @Override public T instantiate(TableId tableId, String className, Class base) - throws ReflectiveOperationException, IOException { + throws ReflectiveOperationException { String ctx = ClassLoaderUtil.tableContext(context.getTableConfiguration(tableId)); return ConfigurationTypeHelper.getClassInstance(ctx, className, base); } diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java index 5e8d8aa3ed5..2edf8a55c29 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java @@ -445,7 +445,7 @@ public boolean checkClass(TInfo tinfo, TCredentials credentials, String classNam Class test = ClassLoaderUtil.loadClass(className, shouldMatch); test.getDeclaredConstructor().newInstance(); return true; - } catch (ClassCastException | ReflectiveOperationException | IOException e) { + } catch (ClassCastException | ReflectiveOperationException e) { log.warn("Error checking object types", e); return false; } diff --git a/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java index 22d9e0002db..b8434828a60 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java @@ -20,7 +20,6 @@ import static org.apache.hadoop.util.StringUtils.getTrimmedStrings; -import java.io.IOException; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -161,7 +160,7 @@ public DistributionStatisticConfig configure(Meter.Id id, registry.config().meterFilter(replicationFilter); registry.config().commonTags(commonTags); Metrics.globalRegistry.add(registry); - } catch (ReflectiveOperationException | IOException ex) { + } catch (ReflectiveOperationException ex) { LOG.warn("Could not load registry {}", factoryName, ex); } } @@ -207,7 +206,7 @@ public DistributionStatisticConfig configure(Meter.Id id, @VisibleForTesting @SuppressWarnings("deprecation") static MeterRegistry getRegistryFromFactory(final String factoryName, final ServerContext context) - throws IOException, ReflectiveOperationException { + throws ReflectiveOperationException { try { LOG.info("look for meter spi registry factory {}", factoryName); Class clazz = diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java index c6a2a1544bf..be4672107ad 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java @@ -278,7 +278,7 @@ static T newInstance(AccumuloConfiguration tableConfig, String className, String context = ClassLoaderUtil.tableContext(tableConfig); try { return ConfigurationTypeHelper.getClassInstance(context, className, baseClass); - } catch (IOException | ReflectiveOperationException e) { + } catch (ReflectiveOperationException e) { throw new RuntimeException(e); } } diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java index 3d5cad9bdd2..0eb3214bda2 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java +++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java @@ -67,6 +67,7 @@ import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.dataImpl.thrift.TConstraintViolationSummary; +import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.accumulo.core.tabletserver.thrift.ConstraintViolationException; import org.apache.accumulo.core.util.BadArgumentException; import org.apache.accumulo.core.util.format.DefaultFormatter; @@ -469,8 +470,8 @@ public AccumuloClient getAccumuloClient() { } public ClassLoader getClassLoader(final CommandLine cl, final Shell shellState) - throws AccumuloException, TableNotFoundException, AccumuloSecurityException, IOException, - ReflectiveOperationException { + throws AccumuloException, TableNotFoundException, AccumuloSecurityException, + ContextClassLoaderException { boolean tables = cl.hasOption(OptUtil.tableOpt().getOpt()) || !shellState.getTableName().isEmpty(); diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java index 937b97d8755..f9e8be1f562 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/SetIterCommand.java @@ -41,6 +41,7 @@ import org.apache.accumulo.core.iterators.user.ReqVisFilter; import org.apache.accumulo.core.iterators.user.VersioningIterator; import org.apache.accumulo.core.metadata.MetadataTable; +import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.accumulo.shell.Shell; import org.apache.accumulo.shell.Shell.Command; import org.apache.accumulo.shell.ShellCommandException; @@ -60,7 +61,7 @@ public class SetIterCommand extends Command { @Override public int execute(final String fullCommand, final CommandLine cl, final Shell shellState) throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException, - ShellCommandException, ReflectiveOperationException { + ShellCommandException, ContextClassLoaderException { boolean tables = cl.hasOption(OptUtil.tableOpt().getOpt()) || !shellState.getTableName().isEmpty(); diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java index 2009cab38e8..e5c67291ac4 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/SetScanIterCommand.java @@ -31,6 +31,7 @@ import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.security.Authorizations; +import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.accumulo.shell.Shell; import org.apache.accumulo.shell.ShellCommandException; import org.apache.commons.cli.CommandLine; @@ -43,7 +44,7 @@ public class SetScanIterCommand extends SetIterCommand { @Override public int execute(final String fullCommand, final CommandLine cl, final Shell shellState) throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException, - ShellCommandException, ReflectiveOperationException { + ShellCommandException, ContextClassLoaderException { Shell.log.warn("Deprecated, use {}", new SetShellIterCommand().getName()); return super.execute(fullCommand, cl, shellState); } diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java index 9ac3e1eea48..336ece806f5 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/SetShellIterCommand.java @@ -27,6 +27,7 @@ import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.client.TableNotFoundException; +import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.accumulo.shell.Shell; import org.apache.accumulo.shell.ShellCommandException; import org.apache.commons.cli.CommandLine; @@ -38,7 +39,7 @@ public class SetShellIterCommand extends SetIterCommand { @Override public int execute(final String fullCommand, final CommandLine cl, final Shell shellState) throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException, - ShellCommandException, ReflectiveOperationException { + ShellCommandException, ContextClassLoaderException { return super.execute(fullCommand, cl, shellState); } From d216bde064bce92ab940a8c0d26eb6433a79bede Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Wed, 16 Jul 2025 20:19:42 +0000 Subject: [PATCH 18/19] narrow exceptions --- .../org/apache/accumulo/core/classloader/ClassLoaderUtil.java | 4 ++-- .../accumulo/core/iteratorsImpl/IteratorConfigUtil.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java index 63221a92578..31e69e0a20e 100644 --- a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java @@ -109,7 +109,7 @@ public static boolean isValidContext(String context) { } public static Class loadClass(String context, String className, - Class extension) throws ReflectiveOperationException { + Class extension) throws ClassNotFoundException { try { return getClassLoader(context).loadClass(className).asSubclass(extension); } catch (ContextClassLoaderException e) { @@ -118,7 +118,7 @@ public static Class loadClass(String context, String className, } public static Class loadClass(String className, Class extension) - throws ReflectiveOperationException { + throws ClassNotFoundException { return loadClass(null, className, extension); } diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java index 93ac01fbbc9..8d3a19cf614 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java @@ -226,7 +226,7 @@ public static SortedKeyValueIterator convertItersAndLoad(IteratorScop skvi.init(prev, options, iteratorBuilder.iteratorEnvironment); prev = skvi; - } catch (ReflectiveOperationException e) { + } catch (ClassNotFoundException e) { log.error("Failed to load iterator {}, for table {}, from context {}", iterInfo.className, iteratorBuilder.iteratorEnvironment.getTableId(), iteratorBuilder.context, e); throw e; @@ -236,7 +236,7 @@ public static SortedKeyValueIterator convertItersAndLoad(IteratorScop } private static Class> loadClass(boolean useAccumuloClassLoader, - String context, IterInfo iterInfo) throws ReflectiveOperationException { + String context, IterInfo iterInfo) throws ClassNotFoundException { if (useAccumuloClassLoader) { @SuppressWarnings("unchecked") var clazz = (Class>) ClassLoaderUtil.loadClass(context, From e05434d3f476e82ce07dfa2b760db612aa510463 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 17 Jul 2025 13:10:32 +0000 Subject: [PATCH 19/19] Fixes to get failed ITs working --- .../core/classloader/DefaultContextClassLoaderFactory.java | 2 +- .../accumulo/core/iteratorsImpl/IteratorConfigUtil.java | 5 +++-- .../org/apache/accumulo/core/metrics/MetricsProducer.java | 3 ++- .../java/org/apache/accumulo/test/metrics/MetricsIT.java | 5 +++++ 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java b/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java index 84b76c226af..ee5634c714b 100644 --- a/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java +++ b/core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java @@ -102,7 +102,7 @@ public ClassLoader getClassLoader(String contextName) throws ContextClassLoaderE try { return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader .getContextClassLoader(contextName); - } catch (IOException e) { + } catch (RuntimeException | IOException e) { throw new ContextClassLoaderException(contextName, e); } } diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java index 8d3a19cf614..5758411e04c 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java @@ -226,10 +226,11 @@ public static SortedKeyValueIterator convertItersAndLoad(IteratorScop skvi.init(prev, options, iteratorBuilder.iteratorEnvironment); prev = skvi; - } catch (ClassNotFoundException e) { + } catch (ReflectiveOperationException e) { log.error("Failed to load iterator {}, for table {}, from context {}", iterInfo.className, iteratorBuilder.iteratorEnvironment.getTableId(), iteratorBuilder.context, e); - throw e; + // This has to be a RuntimeException to be handled properly to fail the scan + throw new RuntimeException(e); } } return prev; diff --git a/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java b/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java index ae6bec42899..7ce7a023df6 100644 --- a/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java +++ b/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java @@ -106,7 +106,8 @@ * N/A * {@value #METRICS_COMPACTOR_FAILURES_TERMINATION} * Gauge - * Number of Compactors terminated due to consecutive failures + * Number of Compactors terminated due to consecutive failures. Process exits after this metric + * is incremented, so it's not guaranteed to be seen. * * * diff --git a/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java b/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java index b7cefe3c8b6..026f392a010 100644 --- a/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java @@ -103,6 +103,11 @@ public void confirmMetricsPublished() throws Exception { // @formatter:off Set unexpectedMetrics = Set.of(METRICS_COMPACTOR_MAJC_STUCK, + METRICS_COMPACTOR_COMPACTIONS_CANCELLED, + METRICS_COMPACTOR_COMPACTIONS_COMPLETED, + METRICS_COMPACTOR_COMPACTIONS_FAILED, + METRICS_COMPACTOR_FAILURES_CONSECUTIVE, + METRICS_COMPACTOR_FAILURES_TERMINATION, METRICS_REPLICATION_QUEUE, METRICS_SCAN_YIELDS, METRICS_UPDATE_ERRORS);