QPID-5409: Fix LocalReplicationNode#setAttribute, ignore exception thrown by ReplicatedEnvironmentFacade in LocalReplicationNode#getAttribute

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/java-broker-bdb-ha@1565369 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 59 insertions, 71 deletions

diff --git a/qpid/java/bdbstore/jmx/src/main/java/org/apache/qpid/server/store/berkeleydb/jmx/BDBHAMessageStoreManagerMBean.java b/qpid/java/bdbstore/jmx/src/main/java/org/apache/qpid/server/store/berkeleydb/jmx/BDBHAMessageStoreManagerMBean.java
index bc7a3298a5..1bc9c6ea6e 100644
--- a/qpid/java/bdbstore/jmx/src/main/java/org/apache/qpid/server/store/berkeleydb/jmx/BDBHAMessageStoreManagerMBean.java
+++ b/qpid/java/bdbstore/jmx/src/main/java/org/apache/qpid/server/store/berkeleydb/jmx/BDBHAMessageStoreManagerMBean.java
@@ -159,15 +159,7 @@ public class BDBHAMessageStoreManagerMBean extends AMQManagedObject implements M
     @Override
     public boolean getDesignatedPrimary() throws IOException, JMException
     {
-        try
-        {
-            return (Boolean)_localReplicationNode.getAttribute(DESIGNATED_PRIMARY);
-        }
-        catch (RuntimeException e)
-        {
-            LOGGER.debug("Failed query designated primary", e);
-            throw new JMException(e.getMessage());
-        }
+        return (Boolean)_localReplicationNode.getAttribute(DESIGNATED_PRIMARY);
     }
 
     @Override
@@ -224,7 +216,7 @@ public class BDBHAMessageStoreManagerMBean extends AMQManagedObject implements M
         catch (Exception e)
         {
             LOGGER.error("Failed to set node " + _localReplicationNode.getName() + " to designated primary : " + primary, e);
-            throw new JMException(e.getMessage());
+            throw new JMException("Failed to set node " + _localReplicationNode.getName() + " to designated primary : " + primary + ":" + e.getMessage());
         }
     }
 
diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/LocalReplicationNode.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/LocalReplicationNode.java
index 60e9a463d1..e5e0f866fa 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/LocalReplicationNode.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/LocalReplicationNode.java
@@ -40,6 +40,7 @@ import org.apache.qpid.server.model.adapter.NoStatistics;
 import org.apache.qpid.server.util.MapValueConverter;
 import org.apache.qpid.server.util.ParameterizedTypeImpl;
 
+import com.sleepycat.je.DatabaseException;
 import com.sleepycat.je.Durability;
 import com.sleepycat.je.Durability.ReplicaAckPolicy;
 import com.sleepycat.je.Durability.SyncPolicy;
@@ -221,43 +222,46 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
         }
         if (_replicatedEnvironmentFacade != null)
         {
-            if(ROLE.equals(attributeName))
+            try
             {
-                return _replicatedEnvironmentFacade.getNodeState();
-            }
-            else if(JOIN_TIME.equals(attributeName))
-            {
-                return _replicatedEnvironmentFacade.getJoinTime();
-            }
-            else if(LAST_KNOWN_REPLICATION_TRANSACTION_ID.equals(attributeName))
-            {
-                return _replicatedEnvironmentFacade.getLastKnownReplicationTransactionId();
-            }
-            else if(QUORUM_OVERRIDE.equals(attributeName))
-            {
-                return _replicatedEnvironmentFacade.getElectableGroupSizeOverride();
+                if(ROLE.equals(attributeName))
+                {
+                    return _replicatedEnvironmentFacade.getNodeState();
+                }
+                else if(JOIN_TIME.equals(attributeName))
+                {
+                    return _replicatedEnvironmentFacade.getJoinTime();
+                }
+                else if(LAST_KNOWN_REPLICATION_TRANSACTION_ID.equals(attributeName))
+                {
+                    return _replicatedEnvironmentFacade.getLastKnownReplicationTransactionId();
+                }
+                else if(QUORUM_OVERRIDE.equals(attributeName))
+                {
+                    return _replicatedEnvironmentFacade.getElectableGroupSizeOverride();
+                }
+                else if(DESIGNATED_PRIMARY.equals(attributeName))
+                {
+                    return _replicatedEnvironmentFacade.isDesignatedPrimary();
+                }
+                else if(PRIORITY.equals(attributeName))
+                {
+                    return _replicatedEnvironmentFacade.getPriority();
+                }
             }
-            else if(DESIGNATED_PRIMARY.equals(attributeName))
+            catch(IllegalStateException e)
             {
-                return _replicatedEnvironmentFacade.isDesignatedPrimary();
+                // ignore, as attribute value will be returned from actual/default attribute maps if present
             }
-            else if(PRIORITY.equals(attributeName))
+            catch(DatabaseException e)
             {
-                return _replicatedEnvironmentFacade.getPriority();
+                // ignore, as attribute value will be returned from actual/default attribute maps if present
             }
         }
         return super.getAttribute(attributeName);
     }
 
     @Override
-    public Object setAttribute(String name, Object expected, Object desired)
-            throws IllegalStateException, AccessControlException,
-            IllegalArgumentException
-    {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
     public Statistics getStatistics()
     {
         return NoStatistics.getInstance();
@@ -277,6 +281,13 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
     }
 
     @Override
+    public boolean changeAttribute(final String name, final Object expected, final Object desired)
+    {
+        updateReplicatedEnvironmentFacade(name, desired);
+        return super.changeAttribute(name, expected, desired);
+    }
+
+    @Override
     public void changeAttributes(Map<String, Object> attributes)
             throws IllegalStateException, AccessControlException,
             IllegalArgumentException
@@ -285,18 +296,16 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
 
         checkWhetherImmutableAttributeChanged(convertedAttributes);
 
-        updateReplicatedEnvironmentFacade(convertedAttributes);
-
         super.changeAttributes(convertedAttributes);
     }
 
-    private void updateReplicatedEnvironmentFacade(Map<String, Object> convertedAttributes)
+    private void updateReplicatedEnvironmentFacade(String attributeName, Object attributeValue)
     {
         if (_replicatedEnvironmentFacade != null)
         {
-            if (convertedAttributes.get(PRIORITY) != null)
+            if (PRIORITY.equals(attributeName))
             {
-                int priority = (Integer)convertedAttributes.get(PRIORITY);
+                int priority = (Integer)attributeValue;
                 try
                 {
                     _replicatedEnvironmentFacade.setPriority(priority);
@@ -307,9 +316,9 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
                 }
             }
 
-            if (convertedAttributes.get(DESIGNATED_PRIMARY) != null)
+            if (DESIGNATED_PRIMARY.equals(attributeName))
             {
-                boolean designatedPrimary = (Boolean)convertedAttributes.get(DESIGNATED_PRIMARY);
+                boolean designatedPrimary = (Boolean)attributeValue;
                 try
                 {
                     _replicatedEnvironmentFacade.setDesignatedPrimary(designatedPrimary);
@@ -320,9 +329,9 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
                 }
             }
 
-            if (convertedAttributes.get(QUORUM_OVERRIDE) != null)
+            if (QUORUM_OVERRIDE.equals(attributeName))
             {
-                int quorumOverride = (Integer)convertedAttributes.get(QUORUM_OVERRIDE);
+                int quorumOverride = (Integer)attributeValue;
                 try
                 {
                     _replicatedEnvironmentFacade.setElectableGroupSizeOverride(quorumOverride);
@@ -334,7 +343,7 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
             }
         }
 
-        if (convertedAttributes.containsKey(ROLE))
+        if (ROLE.equals(attributeName))
         {
             String currentRole = (String)getAttribute(ROLE);
             if (!ReplicatedEnvironment.State.REPLICA.name().equals(currentRole))
@@ -343,7 +352,7 @@ public class LocalReplicationNode extends AbstractAdapter implements Replication
             }
 
             // we do not want to write role into the store
-            String role  = (String)convertedAttributes.remove(ROLE);
+            String role  = (String)attributeValue;
 
             if (ReplicatedEnvironment.State.MASTER.name().equals(role) )
             {
diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
index 97fae5e76b..e32ded5dd5 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
@@ -432,16 +432,8 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
 
     public String getNodeState()
     {
-        try
-        {
-            ReplicatedEnvironment.State state = _environment.getState();
-            return state.toString();
-        }
-        catch (IllegalStateException ise)
-        {
-            // Environment must be being recreated
-            return ReplicatedEnvironment.State.UNKNOWN.name();
-        }
+        ReplicatedEnvironment.State state = _environment.getState();
+        return state.toString();
     }
 
     public boolean isDesignatedPrimary()
@@ -502,19 +494,6 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         }
     }
 
-    private void checkIsOpenAndEnvironmentIsValid()
-    {
-        if (_state.get() != State.OPEN)
-        {
-            throw new IllegalStateException("Environment facade is not in open state");
-        }
-
-        if (!_environment.isValid())
-        {
-            throw new IllegalStateException("Environment is not valid");
-        }
-    }
-
     int getElectableGroupSizeOverride()
     {
         ReplicationMutableConfig repConfig = _environment.getRepMutableConfig();
@@ -553,7 +532,6 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         {
             VLSNRange range = RepInternal.getRepImpl(_environment).getVLSNIndex().getRange();
             VLSN lastTxnEnd = range.getLastTxnEnd();
-            LOGGER.debug("VLSN Range is " + range );
             return lastTxnEnd.getSequence();
         }
         else
diff --git a/qpid/java/bdbstore/systests/src/main/java/org/apache/qpid/server/store/berkeleydb/HAClusterManagementTest.java b/qpid/java/bdbstore/systests/src/main/java/org/apache/qpid/server/store/berkeleydb/HAClusterManagementTest.java
index 057940c036..52b873384d 100644
--- a/qpid/java/bdbstore/systests/src/main/java/org/apache/qpid/server/store/berkeleydb/HAClusterManagementTest.java
+++ b/qpid/java/bdbstore/systests/src/main/java/org/apache/qpid/server/store/berkeleydb/HAClusterManagementTest.java
@@ -198,6 +198,15 @@ public class HAClusterManagementTest extends QpidBrokerTestCase
         }
     }
 
+    public void testSetDesignatedPrimary() throws Exception
+    {
+        int brokerPort = _clusterCreator.getBrokerPortNumbersForNodes().iterator().next();
+        final ManagedBDBHAMessageStore storeBean = getStoreBeanForNodeAtBrokerPort(brokerPort);
+        assertFalse("Unexpected designated primary before change", storeBean.getDesignatedPrimary());
+        storeBean.setDesignatedPrimary(true);
+        assertTrue("Unexpected designated primary after change", storeBean.getDesignatedPrimary());
+    }
+
     private ManagedBDBHAMessageStore getStoreBeanForNodeAtBrokerPort(final int brokerPortNumber) throws Exception
     {
         _jmxUtils.open(brokerPortNumber);