added validation

Author: smiklosovic

src/java/org/apache/cassandra/config/DatabaseDescriptor.java

@@ -1220,6 +1220,12 @@ else if (conf.max_value_size.toMebibytes() >= 2048)
         // run audit logging options through sanitation and validation
         if (conf.audit_logging_options != null)
             setAuditLoggingOptions(conf.audit_logging_options);
+
+        // just run through the validation by setting current values back to their setters, so we are sure that their values are valid
+        DatabaseDescriptor.setGCLogThreshold((int) DatabaseDescriptor.getGCLogThreshold());
+        DatabaseDescriptor.setGCWarnThreshold((int) DatabaseDescriptor.getGCWarnThreshold());
+        DatabaseDescriptor.setGCConcurrentPhaseLogThreshold(DatabaseDescriptor.getGCConcurrentPhaseLogThreshold());
+        DatabaseDescriptor.setGCConcurrentPhaseWarnThreshold(DatabaseDescriptor.getGCConcurrentPhaseWarnThreshold());
     }
 
     @VisibleForTesting
@@ -4719,14 +4725,17 @@ public static long getGCLogThreshold()
         return conf.gc_log_threshold.toMilliseconds();
     }
 
-    public static void setGCLogThreshold(int gcLogThreshold)
+    public static void setGCLogThreshold(int threshold)
     {
-        conf.gc_log_threshold = new DurationSpec.IntMillisecondsBound(gcLogThreshold);
-    }
+        if (threshold <= 0)
+            throw new ConfigurationException("Threshold value for gc_log_threshold must be greater than 0");
 
-    public static EncryptionContext getEncryptionContext()
-    {
-        return encryptionContext;
+        long gcWarnThresholdInMs = getGCWarnThreshold();
+        if (gcWarnThresholdInMs != 0 && threshold > gcWarnThresholdInMs)
+            throw new ConfigurationException("Threshold value for gc_log_threshold (" + threshold + ") must be less than gc_warn_threshold which is currently "
+                                             + gcWarnThresholdInMs);
+
+        conf.gc_log_threshold = new DurationSpec.IntMillisecondsBound(threshold);
     }
 
     public static long getGCWarnThreshold()
@@ -4736,6 +4745,14 @@ public static long getGCWarnThreshold()
 
     public static void setGCWarnThreshold(int threshold)
     {
+        if (threshold < 0)
+            throw new ConfigurationException("Threshold value for gc_warn_threshold must be greater than or equal to 0");
+
+        long gcLogThresholdInMs = getGCLogThreshold();
+        if (threshold != 0 && threshold <= gcLogThresholdInMs)
+            throw new ConfigurationException("Threshold value for gc_warn_threshold (" + threshold + ") must be greater than gc_log_threshold which is currently "
+                                             + gcLogThresholdInMs);
+
         conf.gc_warn_threshold = new DurationSpec.IntMillisecondsBound(threshold);
     }
 
@@ -4746,20 +4763,40 @@ public static int getGCConcurrentPhaseLogThreshold()
 
     public static void setGCConcurrentPhaseLogThreshold(int threshold)
     {
+        if (threshold <= 0)
+            throw new ConfigurationException("Threshold must be greater than 0");
+
+        long gcConcurrentPhaseWarnThresholdInMs = getGCConcurrentPhaseWarnThreshold();
+        if (gcConcurrentPhaseWarnThresholdInMs != 0 && threshold > gcConcurrentPhaseWarnThresholdInMs)
+            throw new ConfigurationException("Threshold value for gc_concurrent_phase_log_threshold (" + threshold + ") must be less than gc_concurrent_phase_warn_threshold which is currently "
+                                             + gcConcurrentPhaseWarnThresholdInMs);
+
         conf.gc_concurrent_phase_log_threshold = new DurationSpec.IntMillisecondsBound(threshold);
     }
 
-
     public static int getGCConcurrentPhaseWarnThreshold()
     {
         return conf.gc_concurrent_phase_warn_threshold.toMilliseconds();
     }
 
     public static void setGCConcurrentPhaseWarnThreshold(int threshold)
     {
+        if (threshold < 0)
+            throw new ConfigurationException("Threshold value for gc_concurrent_phase_warn_threshold must be greater than or equal to 0");
+
+        long gcConcurrentPhaseLogThresholdInMs = getGCConcurrentPhaseLogThreshold();
+        if (threshold != 0 && threshold <= gcConcurrentPhaseLogThresholdInMs)
+            throw new ConfigurationException("Threshold value for gc_concurrent_phase_warn_threshold (" + threshold + ") must be greater than gc_concurrent_phase_log_threshold which is currently "
+                                             + gcConcurrentPhaseLogThresholdInMs);
+
         conf.gc_concurrent_phase_warn_threshold = new DurationSpec.IntMillisecondsBound(threshold);
     }
 
+    public static EncryptionContext getEncryptionContext()
+    {
+        return encryptionContext;
+    }
+
     public static boolean isCDCEnabled()
     {
         return conf.cdc_enabled;

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

@@ -408,15 +408,14 @@ private static long getFieldValue(Field field, boolean isAtomicLong)
 
     public void setGcWarnThresholdInMs(long threshold)
     {
-        long gcLogThresholdInMs = getGcLogThresholdInMs();
-        if (threshold < 0)
-            throw new IllegalArgumentException("Threshold must be greater than or equal to 0");
-        if (threshold != 0 && threshold <= gcLogThresholdInMs)
-            throw new IllegalArgumentException("Threshold must be greater than gcLogThresholdInMs which is currently "
-                    + gcLogThresholdInMs);
-        if (threshold > Integer.MAX_VALUE)
-            throw new IllegalArgumentException("Threshold must be less than Integer.MAX_VALUE");
-        DatabaseDescriptor.setGCWarnThreshold((int)threshold);
+        try
+        {
+            DatabaseDescriptor.setGCWarnThreshold((int)threshold);
+        }
+        catch (Throwable t)
+        {
+            throw new IllegalArgumentException(t.getMessage());
+        }
     }
 
     public long getGcWarnThresholdInMs()
@@ -426,15 +425,14 @@ public long getGcWarnThresholdInMs()
 
     public void setGcLogThresholdInMs(long threshold)
     {
-        if (threshold <= 0)
-            throw new IllegalArgumentException("Threshold must be greater than 0");
-
-        long gcWarnThresholdInMs = getGcWarnThresholdInMs();
-        if (gcWarnThresholdInMs != 0 && threshold > gcWarnThresholdInMs)
-            throw new IllegalArgumentException("Threshold must be less than gcWarnThresholdInMs which is currently "
-                                               + gcWarnThresholdInMs);
-
-        DatabaseDescriptor.setGCLogThreshold((int) threshold);
+        try
+        {
+            DatabaseDescriptor.setGCLogThreshold((int) threshold);
+        }
+        catch (Throwable t)
+        {
+            throw new IllegalArgumentException(t.getMessage());
+        }
     }
 
     public int getGcConcurrentPhaseWarnThresholdInMs()
@@ -444,13 +442,14 @@ public int getGcConcurrentPhaseWarnThresholdInMs()
 
     public void setGcConcurrentPhaseWarnThresholdInMs(int threshold)
     {
-        long gcConcurrentPhaseLogThresholdInMs = getGcConcurrentPhaseLogThresholdInMs();
-        if (threshold < 0)
-            throw new IllegalArgumentException("Threshold must be greater than or equal to 0");
-        if (threshold != 0 && threshold <= gcConcurrentPhaseLogThresholdInMs)
-            throw new IllegalArgumentException("Threshold must be greater than gcConcurrentPhaseLogThresholdInMs which is currently "
-                                               + gcConcurrentPhaseLogThresholdInMs);
-        DatabaseDescriptor.setGCConcurrentPhaseWarnThreshold(threshold);
+        try
+        {
+            DatabaseDescriptor.setGCConcurrentPhaseWarnThreshold(threshold);
+        }
+        catch (Throwable t)
+        {
+            throw new IllegalArgumentException(t.getMessage());
+        }
     }
 
     public int getGcConcurrentPhaseLogThresholdInMs()
@@ -460,15 +459,14 @@ public int getGcConcurrentPhaseLogThresholdInMs()
 
     public void setGcConcurrentPhaseLogThresholdInMs(int threshold)
     {
-        if (threshold <= 0)
-            throw new IllegalArgumentException("Threshold must be greater than 0");
-
-        long gcConcurrentPhaseWarnThresholdInMs = getGcConcurrentPhaseWarnThresholdInMs();
-        if (gcConcurrentPhaseWarnThresholdInMs != 0 && threshold > gcConcurrentPhaseWarnThresholdInMs)
-            throw new IllegalArgumentException("Threshold must be less than gcConcurrentPhaseWarnThresholdInMs which is currently "
-                                               + gcConcurrentPhaseWarnThresholdInMs);
-
-        DatabaseDescriptor.setGCConcurrentPhaseLogThreshold(threshold);
+        try
+        {
+            DatabaseDescriptor.setGCConcurrentPhaseLogThreshold(threshold);
+        }
+        catch (Throwable t)
+        {
+            throw new IllegalArgumentException(t.getMessage());
+        }
     }
 
     public long getGcLogThresholdInMs()

test/unit/org/apache/cassandra/service/GCInspectorTest.java

@@ -18,15 +18,15 @@
 package org.apache.cassandra.service;
 
 import org.apache.cassandra.config.DatabaseDescriptor;
+
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
 
 public class GCInspectorTest
 {
@@ -63,24 +63,13 @@ public void ensureStatusIsCalculated()
     @Test
     public void ensureWarnGreaterThanLog()
     {
-        try
-        {
-            gcInspector.setGcWarnThresholdInMs(gcInspector.getGcLogThresholdInMs());
-            fail("Expect IllegalArgumentException");
-        }
-        catch (IllegalArgumentException e)
-        {
-            // expected
-        }
-        try
-        {
-            gcInspector.setGcConcurrentPhaseWarnThresholdInMs(gcInspector.getGcConcurrentPhaseLogThresholdInMs());
-            fail("Expect IllegalArgumentException");
-        }
-        catch (IllegalArgumentException e)
-        {
-            // expected
-        }
+        assertThatThrownBy(() -> gcInspector.setGcWarnThresholdInMs(gcInspector.getGcLogThresholdInMs()))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Threshold value for gc_warn_threshold (200) must be greater than gc_log_threshold which is currently 200");
+
+        assertThatThrownBy(() -> gcInspector.setGcConcurrentPhaseWarnThresholdInMs(gcInspector.getGcConcurrentPhaseLogThresholdInMs()))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Threshold value for gc_concurrent_phase_warn_threshold (1000) must be greater than gc_concurrent_phase_log_threshold which is currently 1000");
     }
 
     @Test
@@ -102,27 +91,18 @@ public void ensureLogLessThanWarn()
         assertEquals(200, gcInspector.getGcLogThresholdInMs());
         gcInspector.setGcWarnThresholdInMs(1000);
         assertEquals(1000, gcInspector.getGcWarnThresholdInMs());
-        try
-        {
-            gcInspector.setGcLogThresholdInMs(gcInspector.getGcWarnThresholdInMs() + 1);
-            fail("Expect IllegalArgumentException");
-        }
-        catch (IllegalArgumentException e)
-        {
-            // expected
-        }
+
+        assertThatThrownBy(() -> gcInspector.setGcLogThresholdInMs(gcInspector.getGcWarnThresholdInMs() + 1))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Threshold value for gc_log_threshold (1001) must be less than gc_warn_threshold which is currently 1000");
+
         assertEquals(1000, gcInspector.getGcConcurrentPhaseLogThresholdInMs());
         gcInspector.setGcConcurrentPhaseWarnThresholdInMs(2000);
         assertEquals(2000, gcInspector.getGcConcurrentPhaseWarnThresholdInMs());
-        try
-        {
-            gcInspector.setGcConcurrentPhaseLogThresholdInMs(gcInspector.getGcConcurrentPhaseWarnThresholdInMs() + 1);
-            fail("Expect IllegalArgumentException");
-        }
-        catch (IllegalArgumentException e)
-        {
-            // expected
-        }
+
+        assertThatThrownBy(() -> gcInspector.setGcConcurrentPhaseLogThresholdInMs(gcInspector.getGcConcurrentPhaseWarnThresholdInMs() + 1))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Threshold value for gc_concurrent_phase_log_threshold (2001) must be less than gc_concurrent_phase_warn_threshold which is currently 2000");
     }
 
     @Test