ColumnFamilyStoreMBean.getCompactionParameters() is only available si…

…nce Cassandra 2.1 - Restore compatibility with versions prior to 2.1 which doesn't expose this method. - Make unknown compaction strategy (an info that can only be remotely retrieved since Cassandra 2.1) more explicit. ref: #631
thelastpickle · Mar 21, 2019 · b075113 · b075113
1 parent f39b55a
commit b075113
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 11 deletions.
diff --git a/src/server/src/main/java/io/cassandrareaper/core/Table.java b/src/server/src/main/java/io/cassandrareaper/core/Table.java
@@ -21,12 +21,22 @@
 
 public final class Table {
 
+  /**
+   * Default compaction strategy, used when the strategy cannot be retrieved from JMX. The
+   * information is only available since Cassandra 2.1
+   */
+  private static final String DEFAULT_COMPACTION_STRATEGY = "UNKNOWN";
+
   private final String name;
   private final String compactionStrategy;
 
   private Table(Builder builder) {
     this.name = builder.name;
-    this.compactionStrategy = builder.compactionStrategy;
+    if (builder.compactionStrategy != null) {
+      this.compactionStrategy = builder.compactionStrategy;
+    } else {
+      this.compactionStrategy = DEFAULT_COMPACTION_STRATEGY;
+    }
   }
 
   public String getName() {
@@ -41,6 +51,7 @@ public static Builder builder() {
     return new Builder();
   }
 
+  @Override
   public String toString() {
     return String.format("{name=%s, compactionStrategy=%s}", name, compactionStrategy);
   }
@@ -69,7 +80,6 @@ public Table.Builder withCompactionStrategy(String compactionStrategy) {
 
     public Table build() {
       Preconditions.checkNotNull(name, "`.withName(..)` must be called before `.build()`");
-      Preconditions.checkNotNull(compactionStrategy, "`.withCompactionStrategy(..)` must be called before `.build()`");
       return new Table(this);
     }
   }

diff --git a/src/server/src/main/java/io/cassandrareaper/jmx/JmxProxyImpl.java b/src/server/src/main/java/io/cassandrareaper/jmx/JmxProxyImpl.java
@@ -141,7 +141,7 @@ private JmxProxyImpl(
   }
 
   /**
-   * @see JmxProxy#connect(Optional, String, int, String, String, EC2MultiRegionAddressTranslator)
+   * @see #connect(String, int, String, String, EC2MultiRegionAddressTranslator, int, MetricRegistry)
    */
   static JmxProxy connect(
       String host,
@@ -389,8 +389,10 @@ public List<String> getKeyspaces() {
 
   @Override
   public Set<Table> getTablesForKeyspace(String keyspace) throws ReaperException {
-    Set<Table> tables = new HashSet<>();
-    Iterator<Map.Entry<String, ColumnFamilyStoreMBean>> proxies;
+    final boolean canUseCompactionStrategy = versionCompare(getCassandraVersion(), "2.1") >= 0;
+
+    final Set<Table> tables = new HashSet<>();
+    final Iterator<Map.Entry<String, ColumnFamilyStoreMBean>> proxies;
     try {
       proxies = ColumnFamilyStoreMBeanIterator.getColumnFamilyStoreMBeanProxies(mbeanServer);
     } catch (IOException | MalformedObjectNameException e) {
@@ -401,10 +403,15 @@ public Set<Table> getTablesForKeyspace(String keyspace) throws ReaperException {
       String keyspaceName = proxyEntry.getKey();
       if (keyspace.equalsIgnoreCase(keyspaceName)) {
         ColumnFamilyStoreMBean columnFamilyMBean = proxyEntry.getValue();
-        tables.add(Table.builder()
-                .withName(columnFamilyMBean.getColumnFamilyName())
-                .withCompactionStrategy(columnFamilyMBean.getCompactionParameters().get("class"))
-                .build());
+
+        Table.Builder tableBuilder = Table.builder()
+              .withName(columnFamilyMBean.getColumnFamilyName());
+
+        if (canUseCompactionStrategy) {
+          tableBuilder.withCompactionStrategy(columnFamilyMBean.getCompactionParameters().get("class"));
+        }
+
+        tables.add(tableBuilder.build());
       }
     }
     return tables;
@@ -564,8 +571,7 @@ public int triggerRepair(
 
     Preconditions.checkNotNull(ssProxy, "Looks like the proxy is not connected");
     String cassandraVersion = getCassandraVersion();
-    boolean canUseDatacenterAware = false;
-    canUseDatacenterAware = versionCompare(cassandraVersion, "2.0.12") >= 0;
+    final boolean canUseDatacenterAware = versionCompare(cassandraVersion, "2.0.12") >= 0;
 
     String msg = String.format(
         "Triggering repair of range (%s,%s] for keyspace \"%s\" on "

diff --git a/src/server/src/test/java/io/cassandrareaper/service/RepairUnitServiceTest.java b/src/server/src/test/java/io/cassandrareaper/service/RepairUnitServiceTest.java
@@ -89,6 +89,32 @@ public void getTablesToRepairRemoveOneTableTest() throws ReaperException {
     assertEquals(Sets.newHashSet("table2", "table3"), service.getTablesToRepair(proxy, cluster, unit));
   }
 
+  @Test
+  public void getTablesToRepairDefaultCompactionStrategyTable() throws ReaperException {
+    JmxProxy proxy = JmxProxyTest.mockJmxProxyImpl();
+
+    when(context.jmxConnectionFactory.connectAny(cluster))
+          .thenReturn(proxy);
+    when(context.jmxConnectionFactory.connectAny(Mockito.any(Collection.class)))
+          .thenReturn(proxy);
+
+    when(proxy.getTablesForKeyspace(Mockito.anyString()))
+          .thenReturn(Sets.newHashSet(
+                Table.builder().withName("table1").build(),
+                Table.builder().withName("table2").build(),
+                Table.builder().withName("table3").build()));
+
+    RepairUnit unit = RepairUnit.builder()
+          .clusterName(cluster.getName())
+          .keyspaceName("test")
+          .blacklistedTables(Sets.newHashSet("table1"))
+          .incrementalRepair(false)
+          .repairThreadCount(4)
+          .build(UUIDs.timeBased());
+
+    assertEquals(Sets.newHashSet("table2", "table3"), service.getTablesToRepair(proxy, cluster, unit));
+  }
+
   @Test
   public void getTablesToRepairRemoveOneTableWithTwcsTest() throws ReaperException {
     JmxProxy proxy = JmxProxyTest.mockJmxProxyImpl();