QPID-3266: Tidy up housekeeping configuration option for scheduling frequency

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1162121 13f79535-47bb-0310-9956-ffa450edef68

11 files changed, 86 insertions, 22 deletions

diff --git a/qpid/java/broker/etc/virtualhosts.xml b/qpid/java/broker/etc/virtualhosts.xml
index 5860bfe2cb..33a48a1349 100644
--- a/qpid/java/broker/etc/virtualhosts.xml
+++ b/qpid/java/broker/etc/virtualhosts.xml
@@ -31,7 +31,7 @@
 
             <housekeeping>
                 <threadCount>2</threadCount>
-                <expiredMessageCheckPeriod>20000</expiredMessageCheckPeriod>
+                <checkPeriod>20000</checkPeriod>
             </housekeeping>
 
             <exchanges>
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
index 02f8a346cf..dd2df45019 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
@@ -227,6 +227,14 @@ public class ServerConfiguration extends ConfigurationPlugin
                     + (_configFile == null ? "" : " Configuration file : " + _configFile);
             throw new ConfigurationException(message);
         }
+
+        // QPID-3266.  Tidy up housekeeping configuration option for scheduling frequency
+        if (contains("housekeeping.expiredMessageCheckPeriod"))
+        {
+            String message = "Validation error : housekeeping/expiredMessageCheckPeriod must be replaced by housekeeping/checkPeriod."
+                    + (_configFile == null ? "" : " Configuration file : " + _configFile);
+            throw new ConfigurationException(message);
+        }
     }
 
     /*
@@ -707,16 +715,14 @@ public class ServerConfiguration extends ConfigurationPlugin
          getConfig().setProperty("virtualhosts.default", vhost);
     }    
 
-    public void setHousekeepingExpiredMessageCheckPeriod(long value)
+    public void setHousekeepingCheckPeriod(long value)
     {
-        getConfig().setProperty("housekeeping.expiredMessageCheckPeriod", value);
+        getConfig().setProperty("housekeeping.checkPeriod", value);
     }
 
     public long getHousekeepingCheckPeriod()
     {
-        return getLongValue("housekeeping.checkPeriod",
-                                   getLongValue("housekeeping.expiredMessageCheckPeriod",
-                                                       DEFAULT_HOUSEKEEPING_PERIOD));
+        return getLongValue("housekeeping.checkPeriod", DEFAULT_HOUSEKEEPING_PERIOD);
     }
 
     public long getStatisticsSamplePeriod()
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
index a710230616..6729a5ce0f 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
@@ -86,9 +86,9 @@ public class VirtualHostConfiguration extends ConfigurationPlugin
         return _name;
     }
 
-    public long getHousekeepingExpiredMessageCheckPeriod()
+    public long getHousekeepingCheckPeriod()
     {
-        return getLongValue("housekeeping.expiredMessageCheckPeriod", ApplicationRegistry.getInstance().getConfiguration().getHousekeepingCheckPeriod());
+        return getLongValue("housekeeping.checkPeriod", ApplicationRegistry.getInstance().getConfiguration().getHousekeepingCheckPeriod());
     }
 
     public String getAuthenticationDatabase()
@@ -313,6 +313,14 @@ public class VirtualHostConfiguration extends ConfigurationPlugin
                     + " It appears in virtual host definition : " + _name;
             throw new ConfigurationException(message);
         }
+
+        // QPID-3266.  Tidy up housekeeping configuration option for scheduling frequency
+        if (contains("housekeeping.expiredMessageCheckPeriod"))
+        {
+            String message = "Validation error : housekeeping/expiredMessageCheckPeriod must be replaced by housekeeping/checkPeriod."
+                    + " It appears in virtual host definition : " + _name;
+            throw new ConfigurationException(message);
+        }
     }
 
     public int getHouseKeepingThreadCount()
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java
index 5f3446236c..17c65003e9 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java
@@ -239,19 +239,24 @@ public class VirtualHostImpl implements VirtualHost
 
         _brokerMBean = new AMQBrokerManagerMBean(_virtualHostMBean);
         _brokerMBean.register();
-        initialiseHouseKeeping(hostConfig.getHousekeepingExpiredMessageCheckPeriod());
+        initialiseHouseKeeping(hostConfig.getHousekeepingCheckPeriod());
         
         initialiseStatistics();
     }
 
+    /**
+     * Initialise a housekeeping task to iterate over queues cleaning expired messages with no consumers
+     * and checking for idle or open transactions that have exceeded the permitted thresholds.
+     *
+     * @param period
+     */
 	private void initialiseHouseKeeping(long period)
     {
-        /* add a timer task to iterate over queues, cleaning expired messages from queues with no consumers */
         if (period != 0L)
         {
-            class ExpiredMessagesTask extends HouseKeepingTask
+            class VirtualHostHouseKeepingTask extends HouseKeepingTask
             {
-                public ExpiredMessagesTask(VirtualHost vhost)
+                public VirtualHostHouseKeepingTask(VirtualHost vhost)
                 {
                     super(vhost);
                 }
@@ -296,7 +301,7 @@ public class VirtualHostImpl implements VirtualHost
                 }
             }
 
-            scheduleHouseKeepingTask(period, new ExpiredMessagesTask(this));
+            scheduleHouseKeepingTask(period, new VirtualHostHouseKeepingTask(this));
 
             Map<String, VirtualHostPluginFactory> plugins =
                 ApplicationRegistry.getInstance().getPluginManager().getVirtualHostPlugins();
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
index 37375c2b7b..7d54533632 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
@@ -559,18 +559,17 @@ public class ServerConfigurationTest extends QpidTestCase
         assertEquals(true, _serverConfig.getUseBiasedWrites());
     }
 
-    public void testGetHousekeepingExpiredMessageCheckPeriod() throws ConfigurationException
+    public void testGetHousekeepingCheckPeriod() throws ConfigurationException
     {
         // Check default
         _serverConfig.initialise();
         assertEquals(30000, _serverConfig.getHousekeepingCheckPeriod());
 
         // Check value we set
-        _config.setProperty("housekeeping.expiredMessageCheckPeriod", 23L);
+        _config.setProperty("housekeeping.checkPeriod", 23L);
         _serverConfig = new ServerConfiguration(_config);
         _serverConfig.initialise();
-        assertEquals(23, _serverConfig.getHousekeepingCheckPeriod());
-        _serverConfig.setHousekeepingExpiredMessageCheckPeriod(42L);
+        _serverConfig.setHousekeepingCheckPeriod(42L);
         assertEquals(42, _serverConfig.getHousekeepingCheckPeriod());
     }
 
@@ -1362,4 +1361,29 @@ public class ServerConfigurationTest extends QpidTestCase
                     ce.getMessage());
         }
     }
+
+    /*
+     * Tests that the old element housekeeping.expiredMessageCheckPeriod. ... (that was
+     * replaced by housekeeping.checkPeriod) is rejected.
+     */
+    public void testExpiredMessageCheckPeriodRejected() throws ConfigurationException
+    {
+        _serverConfig.initialise();
+
+        // Check value we set
+        _config.setProperty("housekeeping.expiredMessageCheckPeriod", 23L);
+        _serverConfig = new ServerConfiguration(_config);
+
+        try
+        {
+            _serverConfig.initialise();
+            fail("Exception not thrown");
+        }
+        catch (ConfigurationException ce)
+        {
+            assertEquals("Incorrect error message",
+                    "Validation error : housekeeping/expiredMessageCheckPeriod must be replaced by housekeeping/checkPeriod.",
+                    ce.getMessage());
+        }
+    }
 }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java
index 593119041d..b133d53ac5 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java
@@ -229,5 +229,26 @@ public class VirtualHostConfigurationTest extends InternalBrokerBaseCase
          }
      }
 
+     /*
+      * Tests that the old element housekeeping.expiredMessageCheckPeriod. ... (that was
+      * replaced by housekeeping.checkPeriod) is rejected.
+      */
+     public void testExpiredMessageCheckPeriodRejected() throws Exception
+     {
+         getConfigXml().addProperty("virtualhosts.virtualhost.testExpiredMessageCheckPeriodRejected.housekeeping.expiredMessageCheckPeriod",
+                 5);
 
+         try
+         {
+             super.createBroker();
+             fail("Exception not thrown");
+         }
+         catch (ConfigurationException ce)
+         {
+             assertEquals("Incorrect error message",
+                     "Validation error : housekeeping/expiredMessageCheckPeriod must be replaced by housekeeping/checkPeriod." +
+                     " It appears in virtual host definition : " + getName(),
+                     ce.getMessage());
+         }
+     }
 }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/AMQQueueAlertTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/AMQQueueAlertTest.java
index a8bddcf6bf..4272c77798 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/AMQQueueAlertTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/AMQQueueAlertTest.java
@@ -289,7 +289,7 @@ public class AMQQueueAlertTest extends InternalBrokerBaseCase
     protected void configure()
     {
         // Increase Alert Check period
-        getConfiguration().setHousekeepingExpiredMessageCheckPeriod(200);
+        getConfiguration().setHousekeepingCheckPeriod(200);
     }
 
     private void sendMessages(AMQChannel channel, long messageCount, final long size) throws AMQException
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AlertingTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AlertingTest.java
index 58b2edfee2..a2487b49bf 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AlertingTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AlertingTest.java
@@ -46,7 +46,7 @@ public class AlertingTest extends AbstractTestLogging
     {
         // Update the configuration to make our virtualhost Persistent.
         makeVirtualHostPersistent(VIRTUALHOST);
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.expiredMessageCheckPeriod", "5000");
+        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "5000");
 
         _numMessages = 50;
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java
index 36bac3b715..46e5d214f5 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java
@@ -31,7 +31,7 @@ public class TransactionTimeoutConfigurationTest extends TransactionTimeoutTestC
     protected void configure() throws Exception
     {
         // Setup housekeeping every second
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.expiredMessageCheckPeriod", "100");
+        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
         
         // Set transaction timout properties.
         setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openWarn", "200");
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java
index 71b89bf911..db508143f9 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java
@@ -29,7 +29,7 @@ public class TransactionTimeoutDisabledTest extends TransactionTimeoutTestCase
     protected void configure() throws Exception
     {
         // Setup housekeeping every second
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.expiredMessageCheckPeriod", "100");
+        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
     }
 
     public void testProducerIdleCommit() throws Exception
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java
index 786fc2adb0..ef2de5c592 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java
@@ -72,7 +72,7 @@ public class TransactionTimeoutTestCase extends QpidBrokerTestCase implements Ex
     protected void configure() throws Exception
     {
         // Setup housekeeping every second
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.expiredMessageCheckPeriod", "100");
+        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
         
         /*
          * Set transaction timout properties. The XML in the virtualhosts configuration is as follows: