Handle leak of non-standard Java types as clients using JMX cannot ha…

…ndle them

-deprecate and replace JMX setters that throw non-standard exceptions
-deprecate and replace respective JMX getters as well to make JMX usage consistent

patch by Leonard Ma; reviewed by Ekaterina Dimitrova and David Capwell for CASSANDRA-17668

CHANGES.txt

@@ -1,4 +1,6 @@
 4.2
+ * Fix leak of non-standard Java types in JMX MBeans `org.apache.cassandra.db:type=StorageService`
+   and `org.apache.cassandra.db:type=RepairService` as clients using JMX cannot handle them. More details in NEWS.txt (CASSANDRA-17668)
  * Deprecate Throwables.propagate usage (CASSANDRA-14218)
  * Allow disabling hotness persistence for high sstable counts (CASSANDRA-17868)
  * Prevent NullPointerException when changing neverPurgeTombstones from true to false (CASSANDRA-17897)

NEWS.txt

@@ -100,7 +100,17 @@ Upgrading
 
 Deprecation
 -----------
-
+    - In the JMX MBean `org.apache.cassandra.db:type=RepairService` (CASSANDRA-17668):
+        - deprecate the getter/setter methods `getRepairSessionSpaceInMebibytes` and `setRepairSessionSpaceInMebibytes`
+          in favor of `getRepairSessionSpaceInMiB` and `setRepairSessionSpaceInMiB` respectively
+    - In the JMX MBean `org.apache.cassandra.db:type=StorageService` (CASSANDRA-17668):
+        - deprecate the getter/setter methods `getRepairSessionMaxTreeDepth` and `setRepairSessionMaxTreeDepth`
+          in favor of `getRepairSessionMaximumTreeDepth` and `setRepairSessionMaximumTreeDepth`
+        - deprecate the setter method `setColumnIndexSize` in favor of `setColumnIndexSizeInKiB`
+        - deprecate the getter/setter methods `getColumnIndexCacheSize` and `setColumnIndexCacheSize` in favor of
+          `getColumnIndexCacheSizeInKiB` and `setColumnIndexCacheSizeInKiB` respectively
+        - deprecate the getter/setter methods `getBatchSizeWarnThreshold` and `setBatchSizeWarnThreshold` in favor of
+          `getBatchSizeWarnThresholdInKiB` and `setBatchSizeWarnThresholdInKiB` respectively
 
 4.1
 ===

src/java/org/apache/cassandra/service/ActiveRepairService.java

@@ -46,6 +46,7 @@
 import org.apache.cassandra.config.Config;
 import org.apache.cassandra.config.DurationSpec;
 import org.apache.cassandra.db.compaction.CompactionManager;
+import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.locator.AbstractReplicationStrategy;
 import org.apache.cassandra.locator.EndpointsByRange;
 import org.apache.cassandra.locator.EndpointsForRange;
@@ -297,18 +298,44 @@ public int getRepairSessionSpaceInMegabytes()
         return DatabaseDescriptor.getRepairSessionSpaceInMiB();
     }
 
+    @Deprecated
     @Override
     public void setRepairSessionSpaceInMebibytes(int sizeInMebibytes)
     {
         DatabaseDescriptor.setRepairSessionSpaceInMiB(sizeInMebibytes);
     }
 
+    @Deprecated
     @Override
     public int getRepairSessionSpaceInMebibytes()
     {
         return DatabaseDescriptor.getRepairSessionSpaceInMiB();
     }
 
+    @Override
+    public void setRepairSessionSpaceInMiB(int sizeInMebibytes)
+    {
+        try
+        {
+            DatabaseDescriptor.setRepairSessionSpaceInMiB(sizeInMebibytes);
+        }
+        catch (ConfigurationException e)
+        {
+            throw new IllegalArgumentException(e.getMessage());
+        }
+    }
+
+    /*
+     * In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do.
+     * For consistency purposes, the respective getter "getRepairSessionSpaceInMebibytes" was also deprecated and
+     * replaced by this method.
+     */
+    @Override
+    public int getRepairSessionSpaceInMiB()
+    {
+        return DatabaseDescriptor.getRepairSessionSpaceInMiB();
+    }
+
     public List<CompositeData> getRepairStats(List<String> schemaArgs, String rangeString)
     {
         List<CompositeData> stats = new ArrayList<>();

src/java/org/apache/cassandra/service/ActiveRepairServiceMBean.java

@@ -34,9 +34,20 @@ public interface ActiveRepairServiceMBean
     @Deprecated
     public int getRepairSessionSpaceInMegabytes();
 
+    /**
+     * @deprecated use setRepairSessionSpaceInMiB instead as it will not throw non-standard exceptions
+     */
+    @Deprecated
     public void setRepairSessionSpaceInMebibytes(int sizeInMebibytes);
+    /**
+     * @deprecated use getRepairSessionSpaceInMiB instead
+     */
+    @Deprecated
     public int getRepairSessionSpaceInMebibytes();
 
+    public void setRepairSessionSpaceInMiB(int sizeInMebibytes);
+    public int getRepairSessionSpaceInMiB();
+
     public boolean getUseOffheapMerkleTrees();
     public void setUseOffheapMerkleTrees(boolean value);
 

src/java/org/apache/cassandra/service/StorageService.java

@@ -4560,16 +4560,44 @@ public List<String> getParentRepairStatus(int cmd)
                ImmutableList.<String>builder().add(pair.left.name()).addAll(pair.right).build();
     }
 
+    @Deprecated
+    @Override
     public void setRepairSessionMaxTreeDepth(int depth)
     {
         DatabaseDescriptor.setRepairSessionMaxTreeDepth(depth);
     }
 
+    @Deprecated
+    @Override
     public int getRepairSessionMaxTreeDepth()
     {
         return DatabaseDescriptor.getRepairSessionMaxTreeDepth();
     }
 
+    @Override
+    public void setRepairSessionMaximumTreeDepth(int depth)
+    {
+        try
+        {
+            DatabaseDescriptor.setRepairSessionMaxTreeDepth(depth);
+        }
+        catch (ConfigurationException e)
+        {
+            throw new IllegalArgumentException(e.getMessage());
+        }
+    }
+
+    /*
+     * In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do.
+     * For consistency purposes, the respective getter "getRepairSessionMaxTreeDepth" was also deprecated and replaced
+     * by this method.
+     */
+    @Override
+    public int getRepairSessionMaximumTreeDepth()
+    {
+        return DatabaseDescriptor.getRepairSessionMaxTreeDepth();
+    }
+
     /* End of MBean interface methods */
 
     /**
@@ -6236,29 +6264,76 @@ public void setCachedReplicaRowsFailThreshold(int threshold)
         logger.info("updated replica_filtering_protection.cached_rows_fail_threshold to {}", threshold);
     }
 
+    @Override
     public int getColumnIndexSizeInKiB()
     {
         return DatabaseDescriptor.getColumnIndexSizeInKiB();
     }
 
+    @Override
+    public void setColumnIndexSizeInKiB(int columnIndexSizeInKiB)
+    {
+        int oldValueInKiB = DatabaseDescriptor.getColumnIndexSizeInKiB();
+        try
+        {
+            DatabaseDescriptor.setColumnIndexSize(columnIndexSizeInKiB);
+        }
+        catch (ConfigurationException e)
+        {
+            throw new IllegalArgumentException(e.getMessage());
+        }
+        logger.info("Updated column_index_size to {} KiB (was {} KiB)", columnIndexSizeInKiB, oldValueInKiB);
+    }
+
+    @Deprecated
+    @Override
     public void setColumnIndexSize(int columnIndexSizeInKB)
     {
         int oldValueInKiB = DatabaseDescriptor.getColumnIndexSizeInKiB();
         DatabaseDescriptor.setColumnIndexSize(columnIndexSizeInKB);
         logger.info("Updated column_index_size to {} KiB (was {} KiB)", columnIndexSizeInKB, oldValueInKiB);
     }
 
+    @Deprecated
+    @Override
     public int getColumnIndexCacheSize()
     {
         return DatabaseDescriptor.getColumnIndexCacheSizeInKiB();
     }
 
+    @Deprecated
+    @Override
     public void setColumnIndexCacheSize(int cacheSizeInKB)
     {
         DatabaseDescriptor.setColumnIndexCacheSize(cacheSizeInKB);
         logger.info("Updated column_index_cache_size to {}", cacheSizeInKB);
     }
 
+    /*
+     * In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do.
+     * For consistency purposes, the respective getter "getColumnIndexCacheSize" was also deprecated and replaced by
+     * this method.
+     */
+    @Override
+    public int getColumnIndexCacheSizeInKiB()
+    {
+        return DatabaseDescriptor.getColumnIndexCacheSizeInKiB();
+    }
+
+    @Override
+    public void setColumnIndexCacheSizeInKiB(int cacheSizeInKiB)
+    {
+        try
+        {
+            DatabaseDescriptor.setColumnIndexCacheSize(cacheSizeInKiB);
+        }
+        catch (ConfigurationException e)
+        {
+            throw new IllegalArgumentException(e.getMessage());
+        }
+        logger.info("Updated column_index_cache_size to {}", cacheSizeInKiB);
+    }
+
     public int getBatchSizeFailureThreshold()
     {
         return DatabaseDescriptor.getBatchSizeFailThresholdInKiB();
@@ -6270,17 +6345,47 @@ public void setBatchSizeFailureThreshold(int threshold)
         logger.info("updated batch_size_fail_threshold to {}", threshold);
     }
 
+    @Deprecated
+    @Override
     public int getBatchSizeWarnThreshold()
     {
         return DatabaseDescriptor.getBatchSizeWarnThresholdInKiB();
     }
 
+    @Deprecated
+    @Override
     public void setBatchSizeWarnThreshold(int threshold)
     {
         DatabaseDescriptor.setBatchSizeWarnThresholdInKiB(threshold);
         logger.info("Updated batch_size_warn_threshold to {}", threshold);
     }
 
+    /*
+     * In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do.
+     * For consistency purposes, the respective getter "getBatchSizeWarnThreshold" was also deprecated and replaced by
+     * this method.
+     */
+    @Override
+    public int getBatchSizeWarnThresholdInKiB()
+    {
+        return DatabaseDescriptor.getBatchSizeWarnThresholdInKiB();
+    }
+
+    @Override
+    public void setBatchSizeWarnThresholdInKiB(int thresholdInKiB)
+    {
+        try
+        {
+            DatabaseDescriptor.setBatchSizeWarnThresholdInKiB(thresholdInKiB);
+        }
+        catch (ConfigurationException e)
+        {
+            throw new IllegalArgumentException(e.getMessage());
+        }
+
+        logger.info("Updated batch_size_warn_threshold to {}", thresholdInKiB);
+    }
+
     public int getInitialRangeTombstoneListAllocationSize()
     {
         return DatabaseDescriptor.getInitialRangeTombstoneListAllocationSize();

src/java/org/apache/cassandra/service/StorageServiceMBean.java

@@ -420,10 +420,22 @@ default int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion,
 
     public void forceTerminateAllRepairSessions();
 
+    /**
+     * @deprecated use setRepairSessionMaximumTreeDepth instead as it will not throw non-standard exceptions
+     */
+    @Deprecated
     public void setRepairSessionMaxTreeDepth(int depth);
 
+    /**
+     * @deprecated use getRepairSessionMaximumTreeDepth instead
+     */
+    @Deprecated
     public int getRepairSessionMaxTreeDepth();
 
+    public void setRepairSessionMaximumTreeDepth(int depth);
+
+    public int getRepairSessionMaximumTreeDepth();
+
     /**
      * Get the status of a given parent repair session.
      * @param cmd the int reference returned when issuing the repair
@@ -868,24 +880,60 @@ default int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion,
 
     /** Returns the granularity of the collation index of rows within a partition **/
     public int getColumnIndexSizeInKiB();
+
     /** Sets the granularity of the collation index of rows within a partition **/
+    public void setColumnIndexSizeInKiB(int columnIndexSizeInKiB);
+
+    /**
+     * Sets the granularity of the collation index of rows within a partition
+     * @deprecated use setColumnIndexSizeInKiB instead as it will not throw non-standard exceptions
+     */
+    @Deprecated
     public void setColumnIndexSize(int columnIndexSizeInKB);
 
-    /** Returns the threshold for skipping the column index when caching partition info **/
+    /**
+     * Returns the threshold for skipping the column index when caching partition info
+     * @deprecated use getColumnIndexCacheSizeInKiB
+     */
+    @Deprecated
     public int getColumnIndexCacheSize();
-    /** Sets the threshold for skipping the column index when caching partition info **/
+
+    /**
+     * Sets the threshold for skipping the column index when caching partition info
+     * @deprecated use setColumnIndexCacheSizeInKiB instead as it will not throw non-standard exceptions
+     */
+    @Deprecated
     public void setColumnIndexCacheSize(int cacheSizeInKB);
 
+    /** Returns the threshold for skipping the column index when caching partition info **/
+    public int getColumnIndexCacheSizeInKiB();
+
+    /** Sets the threshold for skipping the column index when caching partition info **/
+    public void setColumnIndexCacheSizeInKiB(int cacheSizeInKiB);
+
     /** Returns the threshold for rejecting queries due to a large batch size */
     public int getBatchSizeFailureThreshold();
     /** Sets the threshold for rejecting queries due to a large batch size */
     public void setBatchSizeFailureThreshold(int batchSizeDebugThreshold);
 
-    /** Returns the threshold for warning queries due to a large batch size */
+    /**
+     * Returns the threshold for warning queries due to a large batch size
+     * @deprecated use getBatchSizeWarnThresholdInKiB instead
+     */
+    @Deprecated
     public int getBatchSizeWarnThreshold();
-    /** Sets the threshold for warning queries due to a large batch size */
+    /**
+     * Sets the threshold for warning queries due to a large batch size
+     * @deprecated use setBatchSizeWarnThresholdInKiB instead as it will not throw non-standard exceptions
+     */
+    @Deprecated
     public void setBatchSizeWarnThreshold(int batchSizeDebugThreshold);
 
+    /** Returns the threshold for warning queries due to a large batch size */
+    public int getBatchSizeWarnThresholdInKiB();
+    /** Sets the threshold for warning queries due to a large batch size **/
+    public void setBatchSizeWarnThresholdInKiB(int batchSizeDebugThreshold);
+
     /** Sets the hinted handoff throttle in KiB per second, per delivery thread. */
     public void setHintedHandoffThrottleInKB(int throttleInKB);
 

test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java

@@ -415,7 +415,7 @@ public void testRepairSessionMemorySizeToggles()
             try
             {
                 DatabaseDescriptor.setRepairSessionSpaceInMiB(0);
-                fail("Should have received a ConfigurationException for depth of 9");
+                fail("Should have received a ConfigurationException for depth of 0");
             }
             catch (ConfigurationException ignored) { }