modules/plugins/rhq-storage/src/main/java/org/rhq/plugins/storage/StorageNodeConfigDelegate.java
| 35 +++++
modules/plugins/rhq-storage/src/main/resources/META-INF/rhq-plugin.xml
| 18 ++
modules/plugins/rhq-storage/src/test/java/org/rhq/plugins/storage/StorageNodeConfigDelegateTest.java
| 61 +++++++++-
3 files changed, 107 insertions(+), 7 deletions(-)
New commits:
commit 632eb18cc8984f12c794678c6d13a455048a2096
Author: John Sanda <jsanda(a)redhat.com>
Date: Mon Jul 15 15:35:48 2013 -0400
adding validation checks for heap and thread args
diff --git
a/modules/plugins/rhq-storage/src/main/java/org/rhq/plugins/storage/StorageNodeConfigDelegate.java
b/modules/plugins/rhq-storage/src/main/java/org/rhq/plugins/storage/StorageNodeConfigDelegate.java
index c87d2ac..a0220ba 100644
---
a/modules/plugins/rhq-storage/src/main/java/org/rhq/plugins/storage/StorageNodeConfigDelegate.java
+++
b/modules/plugins/rhq-storage/src/main/java/org/rhq/plugins/storage/StorageNodeConfigDelegate.java
@@ -42,7 +42,8 @@ public class StorageNodeConfigDelegate implements ConfigurationFacet {
config.put(new PropertySimple("minHeapSize",
heapMinProp.substring(4)));
config.put(new PropertySimple("maxHeapSize",
heapMaxProp.substring(4)));
config.put(new PropertySimple("heapNewSize",
heapNewProp.substring(4)));
- config.put(new PropertySimple("threadStackSize",
threadStackSizeProp.substring(4)));
+ config.put(new PropertySimple("threadStackSize",
threadStackSizeProp.substring(4,
+ threadStackSizeProp.length() - 1)));
if (!StringUtil.isEmpty(heapDumpOnOOMError)) {
config.put(new PropertySimple("heapDumpOnOOMError", true));
@@ -70,6 +71,7 @@ public class StorageNodeConfigDelegate implements ConfigurationFacet {
String maxHeapSize = config.getSimpleValue("maxHeapSize");
if (!StringUtil.isEmpty(maxHeapSize)) {
+ validateHeapArg("maxHeapSize", maxHeapSize);
// We want min and max heap to be the same
properties.setProperty("heap_min", "-Xms" +
maxHeapSize);
properties.setProperty("heap_max", "-Xmx" +
maxHeapSize);
@@ -77,12 +79,14 @@ public class StorageNodeConfigDelegate implements ConfigurationFacet
{
String heapNewSize = config.getSimpleValue("heapNewSize");
if (!StringUtil.isEmpty(heapNewSize)) {
+ validateHeapArg("heapNewSize", heapNewSize);
properties.setProperty("heap_new", "-Xmn" +
heapNewSize);
}
String threadStackSize = config.getSimpleValue("threadStackSize");
if (!StringUtil.isEmpty(threadStackSize)) {
- properties.setProperty("thread_stack_size", "-Xss" +
threadStackSize);
+ validateStackArg(threadStackSize);
+ properties.setProperty("thread_stack_size", "-Xss" +
threadStackSize + "k");
}
PropertySimple heapDumpOnOMMError =
config.getSimple("heapDumpOnOOMError");
@@ -102,9 +106,36 @@ public class StorageNodeConfigDelegate implements ConfigurationFacet
{
propertiesUpdater.update(properties);
configurationUpdateReport.setStatus(ConfigurationUpdateStatus.SUCCESS);
+ } catch (IllegalArgumentException e) {
+ configurationUpdateReport.setErrorMessage("No configuration update was
applied: " + e.getMessage());
} catch (IOException e) {
configurationUpdateReport.setErrorMessageFromThrowable(e);
}
+ }
+
+ private void validateHeapArg(String name, String value) {
+ if (value.length() < 2) {
+ throw new IllegalArgumentException(value + " is not a legal value for
the property [" + name + "]");
+ }
+
+ char[] chars = value.toCharArray();
+ for (int i = 0; i < chars.length - 1; ++i) {
+ if (!Character.isDigit(chars[i])) {
+ throw new IllegalArgumentException(value + " is not a legal value
for the property [" + name + "]");
+ }
+ }
+ char lastChar = Character.toUpperCase(chars[chars.length - 1]);
+ if (!(lastChar == 'M' || lastChar == 'G')) {
+ throw new IllegalArgumentException(value + " is not a legal value for
the property [" + name + "]");
+ }
+ }
+
+ private void validateStackArg(String value) {
+ try {
+ Integer.parseInt(value);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(value + " is not a legal value for
the property [threadStackSize]");
+ }
}
}
diff --git a/modules/plugins/rhq-storage/src/main/resources/META-INF/rhq-plugin.xml
b/modules/plugins/rhq-storage/src/main/resources/META-INF/rhq-plugin.xml
index c700257..5a60d19 100644
--- a/modules/plugins/rhq-storage/src/main/resources/META-INF/rhq-plugin.xml
+++ b/modules/plugins/rhq-storage/src/main/resources/META-INF/rhq-plugin.xml
@@ -118,12 +118,24 @@
description="The minimum heap size. This value will be
used with the -Xms JVM option. This
is read only because it is automatically set to the same value
as Max Heap Size."/>
<c:simple-property name="maxHeapSize"
- description="The maximum heap size. This value will be
used with the -Xmx JVM option."/>
+ description="The maximum heap size. This value will be
used with the -Xmx JVM option. The
+ value should be an integer with a suffix of M or G to indicate
megabytes or gigabytes.">
+ <c:constraint>
+ <c:regex-constraint expression="\d+[mMgG]"/>
+ </c:constraint>
+ </c:simple-property>
<c:simple-property name="heapNewSize"
description="The size of the new generation portion of
the heap. This value will be used with
- the -Xmn JVM option."/>
+ the -Xmn JVM option. The value should be an integer with a
suffix of M or G to indicate
+ megabytes or gigabytes.">
+ <c:constraint>
+ <c:regex-constraint expression="\d+[mMgG]"/>
+ </c:constraint>
+ </c:simple-property>
<c:simple-property name="threadStackSize"
- description="The thread stack size. This memory is
allocated to each thread off heap."/>
+ type="integer"
+ description="The thread stack size. This memory is
allocated to each thread off heap. The
+ value should be an integer that will be interpreted in
kilobytes."/>
<c:simple-property name="heapDumpOnOOMError" displayName="Heap
Dump on OutOfMemoryError" type="boolean"
default="true"
description="Generate a heap dump when an
OutOfMemoryError occurs"/>
diff --git
a/modules/plugins/rhq-storage/src/test/java/org/rhq/plugins/storage/StorageNodeConfigDelegateTest.java
b/modules/plugins/rhq-storage/src/test/java/org/rhq/plugins/storage/StorageNodeConfigDelegateTest.java
index ee34f43..64d14b2 100644
---
a/modules/plugins/rhq-storage/src/test/java/org/rhq/plugins/storage/StorageNodeConfigDelegateTest.java
+++
b/modules/plugins/rhq-storage/src/test/java/org/rhq/plugins/storage/StorageNodeConfigDelegateTest.java
@@ -13,8 +13,10 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import org.rhq.core.domain.configuration.Configuration;
+import org.rhq.core.domain.configuration.ConfigurationUpdateStatus;
import org.rhq.core.domain.configuration.PropertySimple;
import org.rhq.core.pluginapi.configuration.ConfigurationUpdateReport;
+import org.rhq.core.util.file.FileUtil;
/**
* @author John Sanda
@@ -29,6 +31,7 @@ public class StorageNodeConfigDelegateTest {
public void initDirs(Method test) throws Exception {
File dir = new File(getClass().getResource(".").toURI());
basedir = new File(dir, getClass().getSimpleName() + "/" +
test.getName());
+ FileUtil.purge(basedir, true);
configDelegate = new StorageNodeConfigDelegate(basedir);
}
@@ -42,7 +45,7 @@ public class StorageNodeConfigDelegateTest {
assertEquals(config.getSimpleValue("minHeapSize"), "512M",
"Failed to load property [minHeapSize]");
assertEquals(config.getSimpleValue("maxHeapSize"), "512M",
"Failed to load property [maxHepSize]");
assertEquals(config.getSimpleValue("heapNewSize"), "128M",
"Failed to load property [heapNewSize]");
- assertEquals(config.getSimpleValue("threadStackSize"),
"180k", "Failed to load property [threadStackSize]");
+ assertEquals(config.getSimpleValue("threadStackSize"), "180",
"Failed to load property [threadStackSize]");
assertEquals(config.getSimple("heapDumpOnOOMError").getBooleanValue(),
(Boolean) true,
"Failed to load property [heapDumpOnOOMError]");
assertEquals(new File(config.getSimpleValue("heapDumpDir")), binDir(),
"Failed to load property [heapDumpDir]");
@@ -56,7 +59,7 @@ public class StorageNodeConfigDelegateTest {
config.put(new PropertySimple("minHeapSize", "1024M"));
config.put(new PropertySimple("maxHeapSize", "1024M"));
config.put(new PropertySimple("heapNewSize", "256M"));
- config.put(new PropertySimple("threadStackSize", "240k"));
+ config.put(new PropertySimple("threadStackSize", "240"));
config.put(new PropertySimple("heapDumpOnOOMError", true));
config.put(new PropertySimple("heapDumpDir", confDir()));
@@ -96,6 +99,60 @@ public class StorageNodeConfigDelegateTest {
"should be the same as [maxHeapSize].");
}
+ @Test
+ public void disableHeapDumps() throws Exception {
+ createDefaultConfig();
+
+ ConfigurationUpdateReport report = new
ConfigurationUpdateReport(Configuration.builder()
+ .addSimple("heapDumpOnOOMError", false).build());
+
+ configDelegate.updateResourceConfiguration(report);
+
+ Properties properties = loadCassandraJvmProps();
+
+ assertEquals(properties.getProperty("heap_dump_on_OOMError"),
"", "Failed to disable property " +
+ "[heapDumpOnOOMError]");
+ }
+
+ @Test
+ public void updateShouldFailWhenMaxHeapSizeIsInvalid() throws Exception {
+ createDefaultConfig();
+
+ ConfigurationUpdateReport report = new
ConfigurationUpdateReport(Configuration.builder()
+ .addSimple("maxHeapSize", "256GB").build());
+
+ configDelegate.updateResourceConfiguration(report);
+
+ assertEquals(report.getStatus(), ConfigurationUpdateStatus.FAILURE, "The
configuration update should fail " +
+ "when [maxHeapSize] has an invalid value.");
+ }
+
+ @Test
+ public void updateShouldFailWhenHeapNewSizeIsInvalid() throws Exception {
+ createDefaultConfig();
+
+ ConfigurationUpdateReport report = new
ConfigurationUpdateReport(Configuration.builder()
+ .addSimple("heapNewSize", "25^G").build());
+
+ configDelegate.updateResourceConfiguration(report);
+
+ assertEquals(report.getStatus(), ConfigurationUpdateStatus.FAILURE, "The
configuration update should fail " +
+ "when [heapNewSize] has an invalid value.");
+ }
+
+ @Test
+ public void updateShouldFailWhenThreadStackSizeIsInvalid() throws Exception {
+ createDefaultConfig();
+
+ ConfigurationUpdateReport report = new
ConfigurationUpdateReport(Configuration.builder()
+ .addSimple("threadStackSize", "128M").build());
+
+ configDelegate.updateResourceConfiguration(report);
+
+ assertEquals(report.getStatus(), ConfigurationUpdateStatus.FAILURE, "The
configuration update should fail " +
+ "when [threadStackSize] has an invalid value.");
+ }
+
private void createDefaultConfig() throws IOException {
Properties properties = new Properties();
properties.setProperty("heap_min", "-Xms512M");