Skip to content

Commit

Permalink
Changes Exception to a Reaper Exception (thelastpickle#1273)
Browse files Browse the repository at this point in the history 
* Don't exit reaper if endpointToRange is empty and segment count 0

We have seen reaper exiting dueing this condition and this turns
it into a reaper exception instead of hitting the assertion in
computeGlobalSegmentCount.

* Makes an AssertionError a Reaper Exception

This makes it so that reaper doesn't crashes tight away if a
ks has been removed.

---------

Co-authored-by: German Eichberger <geeichbe@microsoft.com>
  • Loading branch information
@xgerman
xgerman and German Eichberger committed Feb 17, 2023
1 parent 7618270 commit d0990b7
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
22 changes: 16 additions & 6 deletions src/server/src/main/java/io/cassandrareaper/jmx/ClusterFacade.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,15 @@
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.ExecutionError;
import org.apache.commons.lang3.tuple.Pair;
import org.joda.time.DateTime;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static io.cassandrareaper.jmx.MetricsProxy.convertToGenericMetrics;
import static io.cassandrareaper.jmx.MetricsProxy.updateGenericMetricAttribute;


public final class ClusterFacade {

Expand Down Expand Up @@ -280,6 +284,12 @@ public Map<List<String>, List<String>> getRangeToEndpointMap(
() -> getRangeToEndpointMapImpl(cluster, keyspace));
} catch (ExecutionException ex) {
throw new ReaperException(ex);
} catch (ExecutionError ex) {
if ((ex.getCause() instanceof AssertionError)) {
throw new ReaperException(String.valueOf(ex));
} else {
throw ex;
}
}
}

Expand Down Expand Up @@ -543,7 +553,7 @@ public List<MetricsHistogram> getClientRequestLatencies(Node node) throws Reaper
if (nodeIsAccessibleThroughJmx(nodeDc, node.getHostname())) {
MetricsProxy metricsProxy = MetricsProxy.create(connect(node));
return convertToMetricsHistogram(
MetricsProxy.convertToGenericMetrics(metricsProxy.collectLatencyMetrics(), node));
convertToGenericMetrics(metricsProxy.collectLatencyMetrics(), node));
} else {
// We look for metrics in the last two time based partitions to make sure we get a result
return convertToMetricsHistogram(((IDistributedStorage)context.storage)
Expand Down Expand Up @@ -572,7 +582,7 @@ public List<DroppedMessages> getDroppedMessages(Node node) throws ReaperExceptio
String nodeDc = getDatacenter(node.getCluster().get(), node.getHostname());
if (nodeIsAccessibleThroughJmx(nodeDc, node.getHostname())) {
MetricsProxy proxy = MetricsProxy.create(connect(node));
return convertToDroppedMessages(MetricsProxy.convertToGenericMetrics(proxy.collectDroppedMessages(), node));
return convertToDroppedMessages(convertToGenericMetrics(proxy.collectDroppedMessages(), node));
} else {
return convertToDroppedMessages(((IDistributedStorage)context.storage)
.getMetrics(
Expand All @@ -596,7 +606,7 @@ public List<DroppedMessages> convertToDroppedMessages(List<GenericMetric> metric
for (Entry<String, List<GenericMetric>> pool : metricsByScope.entrySet()) {
DroppedMessages.Builder builder = DroppedMessages.builder().withName(pool.getKey());
for (GenericMetric stat : pool.getValue()) {
builder = MetricsProxy.updateGenericMetricAttribute(stat, builder);
builder = updateGenericMetricAttribute(stat, builder);
}
droppedMessages.add(builder.build());
}
Expand All @@ -615,7 +625,7 @@ public List<ThreadPoolStat> getTpStats(Node node) throws ReaperException {
String nodeDc = getDatacenter(node.getCluster().get(), node.getHostname());
if (nodeIsAccessibleThroughJmx(nodeDc, node.getHostname())) {
MetricsProxy proxy = MetricsProxy.create(connect(node));
return convertToThreadPoolStats(MetricsProxy.convertToGenericMetrics(proxy.collectTpStats(), node));
return convertToThreadPoolStats(convertToGenericMetrics(proxy.collectTpStats(), node));
} else {
return convertToThreadPoolStats(((IDistributedStorage)context.storage)
.getMetrics(
Expand All @@ -639,7 +649,7 @@ public List<ThreadPoolStat> convertToThreadPoolStats(List<GenericMetric> metrics
for (Entry<String, List<GenericMetric>> pool : metricsByScope.entrySet()) {
ThreadPoolStat.Builder builder = ThreadPoolStat.builder().withName(pool.getKey());
for (GenericMetric stat : pool.getValue()) {
builder = MetricsProxy.updateGenericMetricAttribute(stat, builder);
builder = updateGenericMetricAttribute(stat, builder);
}
tpstats.add(builder.build());
}
Expand All @@ -666,7 +676,7 @@ public List<MetricsHistogram> convertToMetricsHistogram(List<GenericMetric> metr
.withName(metricByScope.getKey())
.withType(metricByName.getKey());
for (GenericMetric stat : metricByName.getValue()) {
builder = MetricsProxy.updateGenericMetricAttribute(stat, builder);
builder = updateGenericMetricAttribute(stat, builder);
}
histograms.add(builder.build());
}
Expand Down
31 changes: 31 additions & 0 deletions src/server/src/test/java/io/cassandrareaper/jmx/ClusterFacadeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@
import io.cassandrareaper.ReaperApplicationConfiguration;
import io.cassandrareaper.ReaperApplicationConfiguration.DatacenterAvailability;
import io.cassandrareaper.ReaperException;
import io.cassandrareaper.core.Cluster;
import io.cassandrareaper.core.Compaction;
import io.cassandrareaper.core.CompactionStats;
import io.cassandrareaper.core.JmxCredentials;
import io.cassandrareaper.core.StreamSession;

import java.io.IOException;
import java.net.URL;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
Expand All @@ -46,6 +50,8 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -191,4 +197,29 @@ public void endpointCleanupScylla() {
}
}

@Test(expected = ReaperException.class)
public void getRangeToEndpointMapArgumentExceptionTest() throws ReaperException {
final AppContext cxt = new AppContext();
cxt.config = new ReaperApplicationConfiguration();
AppContext contextSpy = Mockito.spy(cxt);
Mockito.doReturn("127.0.0.1").when(contextSpy).getLocalNodeAddress();

contextSpy.config.setDatacenterAvailability(DatacenterAvailability.SIDECAR);
JmxConnectionFactory jmxConnectionFactory = mock(JmxConnectionFactory.class);
JmxProxy mockProxy = mock(JmxProxy.class);
when(jmxConnectionFactory.connectAny(any(Collection.class))).thenReturn(mockProxy);
contextSpy.jmxConnectionFactory = jmxConnectionFactory;
final ClusterFacade cf = ClusterFacade.create(contextSpy);
JmxCredentials jmxCredentials = JmxCredentials.builder().withUsername("test").withPassword("testPwd").build();
Set<String> seedHosts = new HashSet<>();
seedHosts.add("127.0.0.1");
Cluster.Builder builder = Cluster.builder();
builder.withName("testCluster");
builder.withJmxCredentials(jmxCredentials);
builder.withSeedHosts(seedHosts);
Cluster mockCluster = builder.build();
when(mockProxy.getRangeToEndpointMap(eq("fake-ks"))).thenThrow(new AssertionError(""));
cf.getRangeToEndpointMap(mockCluster, "fake-ks");
}

}

0 comments on commit d0990b7

Please sign in to comment.