This is an automated email from the ASF dual-hosted git repository.
dcapwell pushed a commit to branch cassandra-4.1
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-4.1 by this push:
new 6396562f71 StorageService read threshold get methods throw
NullPointerException due to not handling null configs
6396562f71 is described below
commit 6396562f71316838083618714b142fd982ae0155
Author: David Capwell <[email protected]>
AuthorDate: Tue May 10 09:53:24 2022 -0700
StorageService read threshold get methods throw NullPointerException due to
not handling null configs
patch by David Capwell; reviewed by Berenguer Blasi for CASSANDRA-17593
---
CHANGES.txt | 1 +
.../cassandra/config/DatabaseDescriptor.java | 20 ++-
.../apache/cassandra/locator/TokenMetadata.java | 2 +-
.../apache/cassandra/service/StorageService.java | 17 ++-
.../cassandra/distributed/impl/Instance.java | 17 ++-
.../distributed/test/jmx/JMXGetterCheckTest.java | 143 +++++++++++++++++++++
test/unit/org/apache/cassandra/cql3/CQLTester.java | 2 +-
7 files changed, 187 insertions(+), 15 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index efc1a2fbf1..5105707d4b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.1
+ * StorageService read threshold get methods throw NullPointerException due to
not handling null configs (CASSANDRA-17593)
* Rename truncate_drop guardrail to drop_truncate_table (CASSANDRA-17592)
* nodetool enablefullquerylog can NPE when directory has no files
(CASSANDRA-17595)
* Add auto_snapshot_ttl configuration (CASSANDRA-16790)
diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
index 7ed6ce742f..6dee232ee7 100644
--- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
+++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
@@ -38,6 +38,8 @@ import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.Supplier;
+import javax.annotation.Nullable;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
@@ -4037,67 +4039,73 @@ public class DatabaseDescriptor
}
}
+ @Nullable
public static DataStorageSpec getCoordinatorReadSizeWarnThreshold()
{
return conf.coordinator_read_size_warn_threshold;
}
- public static void setCoordinatorReadSizeWarnThreshold(DataStorageSpec
value)
+ public static void setCoordinatorReadSizeWarnThreshold(@Nullable
DataStorageSpec value)
{
logger.info("updating coordinator_read_size_warn_threshold to {}",
value);
conf.coordinator_read_size_warn_threshold = value;
}
+ @Nullable
public static DataStorageSpec getCoordinatorReadSizeFailThreshold()
{
return conf.coordinator_read_size_fail_threshold;
}
- public static void setCoordinatorReadSizeFailThreshold(DataStorageSpec
value)
+ public static void setCoordinatorReadSizeFailThreshold(@Nullable
DataStorageSpec value)
{
logger.info("updating coordinator_read_size_fail_threshold to {}",
value);
conf.coordinator_read_size_fail_threshold = value;
}
+ @Nullable
public static DataStorageSpec getLocalReadSizeWarnThreshold()
{
return conf.local_read_size_warn_threshold;
}
- public static void setLocalReadSizeWarnThreshold(DataStorageSpec value)
+ public static void setLocalReadSizeWarnThreshold(@Nullable DataStorageSpec
value)
{
logger.info("updating local_read_size_warn_threshold to {}", value);
conf.local_read_size_warn_threshold = value;
}
+ @Nullable
public static DataStorageSpec getLocalReadSizeFailThreshold()
{
return conf.local_read_size_fail_threshold;
}
- public static void setLocalReadSizeFailThreshold(DataStorageSpec value)
+ public static void setLocalReadSizeFailThreshold(@Nullable DataStorageSpec
value)
{
logger.info("updating local_read_size_fail_threshold to {}", value);
conf.local_read_size_fail_threshold = value;
}
+ @Nullable
public static DataStorageSpec getRowIndexReadSizeWarnThreshold()
{
return conf.row_index_read_size_warn_threshold;
}
- public static void setRowIndexReadSizeWarnThreshold(DataStorageSpec value)
+ public static void setRowIndexReadSizeWarnThreshold(@Nullable
DataStorageSpec value)
{
logger.info("updating row_index_size_warn_threshold to {}", value);
conf.row_index_read_size_warn_threshold = value;
}
+ @Nullable
public static DataStorageSpec getRowIndexReadSizeFailThreshold()
{
return conf.row_index_read_size_fail_threshold;
}
- public static void setRowIndexReadSizeFailThreshold(DataStorageSpec value)
+ public static void setRowIndexReadSizeFailThreshold(@Nullable
DataStorageSpec value)
{
logger.info("updating row_index_read_size_fail_threshold to {}",
value);
conf.row_index_read_size_fail_threshold = value;
diff --git a/src/java/org/apache/cassandra/locator/TokenMetadata.java
b/src/java/org/apache/cassandra/locator/TokenMetadata.java
index 51dd12063f..0be64abce4 100644
--- a/src/java/org/apache/cassandra/locator/TokenMetadata.java
+++ b/src/java/org/apache/cassandra/locator/TokenMetadata.java
@@ -601,7 +601,7 @@ public class TokenMetadata
lock.readLock().lock();
try
{
- assert isMember(endpoint); // don't want to return nulls
+ assert isMember(endpoint): String.format("Unable to get tokens for
%s; it is not a member", endpoint); // don't want to return nulls
return new ArrayList<>(tokenToEndpointMap.inverse().get(endpoint));
}
finally
diff --git a/src/java/org/apache/cassandra/service/StorageService.java
b/src/java/org/apache/cassandra/service/StorageService.java
index dd35c70fdd..d1e9f48f27 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -6441,7 +6441,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
@Override
public String getCoordinatorLargeReadWarnThreshold()
{
- return
DatabaseDescriptor.getCoordinatorReadSizeWarnThreshold().toString();
+ return
toString(DatabaseDescriptor.getCoordinatorReadSizeWarnThreshold());
}
@Override
@@ -6453,7 +6453,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
@Override
public String getCoordinatorLargeReadAbortThreshold()
{
- return
DatabaseDescriptor.getCoordinatorReadSizeFailThreshold().toString();
+ return
toString(DatabaseDescriptor.getCoordinatorReadSizeFailThreshold());
}
@Override
@@ -6465,7 +6465,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
@Override
public String getLocalReadTooLargeWarnThreshold()
{
- return DatabaseDescriptor.getLocalReadSizeWarnThreshold().toString();
+ return toString(DatabaseDescriptor.getLocalReadSizeWarnThreshold());
}
@Override
@@ -6477,7 +6477,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
@Override
public String getLocalReadTooLargeAbortThreshold()
{
- return DatabaseDescriptor.getLocalReadSizeFailThreshold().toString();
+ return toString(DatabaseDescriptor.getLocalReadSizeFailThreshold());
}
@Override
@@ -6489,7 +6489,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
@Override
public String getRowIndexReadSizeWarnThreshold()
{
- return
DatabaseDescriptor.getRowIndexReadSizeWarnThreshold().toString();
+ return toString(DatabaseDescriptor.getRowIndexReadSizeWarnThreshold());
}
@Override
@@ -6501,7 +6501,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
@Override
public String getRowIndexReadSizeAbortThreshold()
{
- return
DatabaseDescriptor.getRowIndexReadSizeFailThreshold().toString();
+ return toString(DatabaseDescriptor.getRowIndexReadSizeFailThreshold());
}
@Override
@@ -6510,6 +6510,11 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
DatabaseDescriptor.setRowIndexReadSizeFailThreshold(parseDataStorageSpec(threshold));
}
+ private static String toString(DataStorageSpec value)
+ {
+ return value == null ? null : value.toString();
+ }
+
public void setDefaultKeyspaceReplicationFactor(int value)
{
DatabaseDescriptor.setDefaultKeyspaceRF(value);
diff --git
a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
index 32d3bccf7d..d03f2f1b5e 100644
--- a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
+++ b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
@@ -33,6 +33,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.UUID;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
@@ -750,11 +751,25 @@ public class Instance extends IsolatedExecutor implements
IInvokableInstance
error = parallelRun(error, executor,
StorageService.instance::disableAutoCompaction);
+ // trigger init early or else it could try to init and touch a
thread pool that got shutdown
+ HintsService hints = HintsService.instance;
+ ThrowingRunnable shutdownHints = () -> {
+ // this is to allow shutdown in the case hints were halted
already
+ try
+ {
+ HintsService.instance.shutdownBlocking();
+ }
+ catch (IllegalStateException e)
+ {
+ if (!"HintsService has already been shut
down".equals(e.getMessage()))
+ throw e;
+ }
+ };
error = parallelRun(error, executor,
() ->
Gossiper.instance.stopShutdownAndWait(1L, MINUTES),
CompactionManager.instance::forceShutdown,
() ->
BatchlogManager.instance.shutdownAndWait(1L, MINUTES),
- HintsService.instance::shutdownBlocking,
+ shutdownHints,
() -> CompactionLogger.shutdownNowAndWait(1L,
MINUTES),
() -> AuthCache.shutdownAllAndWait(1L,
MINUTES),
() -> Sampler.shutdownNowAndWait(1L, MINUTES),
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/jmx/JMXGetterCheckTest.java
b/test/distributed/org/apache/cassandra/distributed/test/jmx/JMXGetterCheckTest.java
new file mode 100644
index 0000000000..217ec4cf7a
--- /dev/null
+++
b/test/distributed/org/apache/cassandra/distributed/test/jmx/JMXGetterCheckTest.java
@@ -0,0 +1,143 @@
+/*
+ * 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
+ *
+ * http://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.cassandra.distributed.test.jmx;
+
+import java.net.InetAddress;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
+import javax.management.JMRuntimeException;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanInfo;
+import javax.management.MBeanOperationInfo;
+import javax.management.MBeanServerConnection;
+import javax.management.ObjectName;
+import javax.management.remote.JMXConnector;
+import javax.management.remote.JMXConnectorFactory;
+import javax.management.remote.JMXConnectorServer;
+import javax.management.remote.JMXServiceURL;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.Test;
+
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.Feature;
+import org.apache.cassandra.distributed.test.TestBaseImpl;
+import org.apache.cassandra.utils.JMXServerUtils;
+
+import static
org.apache.cassandra.config.CassandraRelevantProperties.IS_DISABLED_MBEAN_REGISTRATION;
+import static
org.apache.cassandra.cql3.CQLTester.getAutomaticallyAllocatedPort;
+
+public class JMXGetterCheckTest extends TestBaseImpl
+{
+ private static final Set<String> IGNORE_ATTRIBUTES = ImmutableSet.of(
+ "org.apache.cassandra.net:type=MessagingService:BackPressurePerHost" //
throws unsupported saying the feature was removed... dropped in CASSANDRA-15375
+ );
+ private static final Set<String> IGNORE_OPERATIONS = ImmutableSet.of(
+ "org.apache.cassandra.db:type=StorageService:stopDaemon", // halts the
instance, which then causes the JVM to exit
+ "org.apache.cassandra.db:type=StorageService:drain", // don't drain, it
stops things which can cause other APIs to be unstable as we are in a stopped
state
+ "org.apache.cassandra.db:type=StorageService:stopGossiping" // if we stop
gossip this can causes other issues, so avoid
+ );
+
+ @Test
+ public void test() throws Exception
+ {
+ // start JMX server, which the instance will register with
+ InetAddress loopback = InetAddress.getLoopbackAddress();
+ String jmxHost = loopback.getHostAddress();
+ int jmxPort = getAutomaticallyAllocatedPort(loopback);
+ JMXConnectorServer jmxServer = JMXServerUtils.createJMXServer(jmxPort,
true);
+ jmxServer.start();
+ String url = "service:jmx:rmi:///jndi/rmi://" + jmxHost + ":" +
jmxPort + "/jmxrmi";
+
+ IS_DISABLED_MBEAN_REGISTRATION.setBoolean(false);
+ try (Cluster cluster = Cluster.build(1).withConfig(c ->
c.with(Feature.values())).start())
+ {
+ List<Named> errors = new ArrayList<>();
+ try (JMXConnector jmxc = JMXConnectorFactory.connect(new
JMXServiceURL(url), null))
+ {
+ MBeanServerConnection mbsc = jmxc.getMBeanServerConnection();
+ Set<ObjectName> metricNames = new
TreeSet<>(mbsc.queryNames(null, null));
+ for (ObjectName name : metricNames)
+ {
+ if (!name.getDomain().startsWith("org.apache.cassandra"))
+ continue;
+ MBeanInfo info = mbsc.getMBeanInfo(name);
+ for (MBeanAttributeInfo a : info.getAttributes())
+ {
+ String fqn = String.format("%s:%s", name, a.getName());
+ if (!a.isReadable() || IGNORE_ATTRIBUTES.contains(fqn))
+ continue;
+ try
+ {
+ mbsc.getAttribute(name, a.getName());
+ }
+ catch (JMRuntimeException e)
+ {
+ errors.add(new Named(String.format("Attribute %s",
fqn), e.getCause()));
+ }
+ }
+
+ for (MBeanOperationInfo o : info.getOperations())
+ {
+ String fqn = String.format("%s:%s", name, o.getName());
+ if (o.getSignature().length != 0 ||
IGNORE_OPERATIONS.contains(fqn))
+ continue;
+ try
+ {
+ mbsc.invoke(name, o.getName(), new Object[0], new
String[0]);
+ }
+ catch (JMRuntimeException e)
+ {
+ errors.add(new Named(String.format("Operation %s",
fqn), e.getCause()));
+ }
+ }
+ }
+ }
+ if (!errors.isEmpty())
+ {
+ AssertionError root = new AssertionError();
+ errors.forEach(root::addSuppressed);
+ throw root;
+ }
+ }
+ }
+
+ /**
+ * This class is meant to make new errors easier to read, by adding the
JMX endpoint, and cleaning up the unneded JMX/Reflection logic cluttering the
stacktrace
+ */
+ private static class Named extends RuntimeException
+ {
+ public Named(String msg, Throwable cause)
+ {
+ super(msg + "\nCaused by: " + cause.getClass().getCanonicalName()
+ ": " + cause.getMessage(), cause.getCause());
+ StackTraceElement[] stack = cause.getStackTrace();
+ List<StackTraceElement> copy = new ArrayList<>();
+ for (StackTraceElement s : stack)
+ {
+ if (!s.getClassName().startsWith("org.apache.cassandra"))
+ break;
+ copy.add(s);
+ }
+ Collections.reverse(copy);
+ setStackTrace(copy.toArray(new StackTraceElement[0]));
+ }
+ }
+}
diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java
b/test/unit/org/apache/cassandra/cql3/CQLTester.java
index 0255cc9e4d..d8a23fdad8 100644
--- a/test/unit/org/apache/cassandra/cql3/CQLTester.java
+++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java
@@ -226,7 +226,7 @@ public abstract class CQLTester
*
* @return a port number
*/
- private static int getAutomaticallyAllocatedPort(InetAddress address)
+ public static int getAutomaticallyAllocatedPort(InetAddress address)
{
try
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
