QPID-3603: HA backup brokers don't connect to self when searching for primary.

Also: BrokerReplicator destroys bridge in destructor.

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

5 files changed, 32 insertions, 14 deletions

diff --git a/qpid/cpp/src/qpid/ha/Backup.cpp b/qpid/cpp/src/qpid/ha/Backup.cpp
index 1ff5578ff4..8b62f89f11 100644
--- a/qpid/cpp/src/qpid/ha/Backup.cpp
+++ b/qpid/cpp/src/qpid/ha/Backup.cpp
@@ -33,6 +33,7 @@
 #include "qpid/framing/AMQFrame.h"
 #include "qpid/framing/FieldTable.h"
 #include "qpid/framing/MessageTransferBody.h"
+#include "qpid/sys/SystemInfo.h"
 #include "qpid/types/Variant.h"
 
 namespace qpid {
@@ -46,13 +47,30 @@ using std::string;
 Backup::Backup(HaBroker& hb, const Settings& s) :
     logPrefix(hb), haBroker(hb), broker(hb.getBroker()), settings(s)
 {
-    // Empty brokerUrl means delay initialization until setUrl() is called.
+    // Empty brokerUrl means delay initialization until seBrokertUrl() is called.
     if (!s.brokerUrl.empty()) initialize(Url(s.brokerUrl));
 }
 
-void Backup::initialize(const Url& url) {
-    if (url.empty()) throw Url::Invalid("HA broker URL is empty");
-    QPID_LOG(info, logPrefix << "initialized for: " << url);
+bool Backup::isSelf(const Address& a) const {
+    return sys::SystemInfo::isLocalHost(a.host) &&
+        a.port == haBroker.getBroker().getPort(a.protocol);
+}
+
+Url Backup::linkUrl(const Url& brokers) const {
+    // linkUrl contains only the addresses of *other* brokers, not this one.
+    Url url;
+    for (Url::const_iterator i = brokers.begin(); i != brokers.end(); ++i)
+        if (!isSelf(*i)) url.push_back(*i);
+    if (url.empty()) throw Url::Invalid("HA broker Link URL is empty");
+    QPID_LOG(debug, logPrefix << "Link URL set to: " << url);
+    return url;
+}
+
+void Backup::initialize(const Url& brokers) {
+    if (brokers.empty()) throw Url::Invalid("HA broker URL is empty");
+    QPID_LOG(info, logPrefix << "Backup initialized with broker URL: " << brokers);
+    sys::Mutex::ScopedLock l(lock);
+    Url url = linkUrl(brokers);
     string protocol = url[0].protocol.empty() ? "tcp" : url[0].protocol;
     framing::Uuid uuid(true);
     // Declare the link
@@ -79,13 +97,11 @@ void Backup::setBrokerUrl(const Url& url) {
     // Ignore empty URLs seen during start-up for some tests.
     if (url.empty()) return;
     sys::Mutex::ScopedLock l(lock);
-    if (link) {                 // URL changed after we initialized.
-        QPID_LOG(info, logPrefix << "broker URL set to " << url);
-        link->setUrl(url);
-    }
-    else {
-        initialize(url);        // Deferred initialization
+    if (link) {
+        QPID_LOG(info, logPrefix << "Broker URL set to: " << url);
+        link->setUrl(linkUrl(url));
     }
+    else initialize(url);        // Deferred initialization
 }
 
 }} // namespace qpid::ha
diff --git a/qpid/cpp/src/qpid/ha/Backup.h b/qpid/cpp/src/qpid/ha/Backup.h
index f794b11a60..92387ece60 100644
--- a/qpid/cpp/src/qpid/ha/Backup.h
+++ b/qpid/cpp/src/qpid/ha/Backup.h
@@ -53,6 +53,8 @@ class Backup
     void setBrokerUrl(const Url&);
 
   private:
+    bool isSelf(const Address& a) const;
+    Url linkUrl(const Url&) const;
     void initialize(const Url&);
 
     LogPrefix logPrefix;
diff --git a/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp b/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
index ea5f4a5fa8..33a4ef4080 100644
--- a/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
+++ b/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
@@ -162,9 +162,6 @@ Variant::Map asMapVoid(const Variant& value) {
 
 } // namespace
 
-
-BrokerReplicator::~BrokerReplicator() {}
-
 BrokerReplicator::BrokerReplicator(HaBroker& hb, const boost::shared_ptr<Link>& l)
     : Exchange(QPID_CONFIGURATION_REPLICATOR),
       logPrefix(hb),
@@ -190,6 +187,9 @@ BrokerReplicator::BrokerReplicator(HaBroker& hb, const boost::shared_ptr<Link>&
     );
 }
 
+// FIXME aconway 2012-05-07: reference cycled between Link and BrokerReplicator.
+BrokerReplicator::~BrokerReplicator() { link->close(); }
+
 // This is called in the connection IO thread when the bridge is started.
 void BrokerReplicator::initializeBridge(Bridge& bridge, SessionHandler& sessionHandler) {
 
diff --git a/qpid/cpp/src/qpid/ha/HaBroker.cpp b/qpid/cpp/src/qpid/ha/HaBroker.cpp
index c3e7b7f758..178535f1af 100644
--- a/qpid/cpp/src/qpid/ha/HaBroker.cpp
+++ b/qpid/cpp/src/qpid/ha/HaBroker.cpp
@@ -202,7 +202,6 @@ void HaBroker::updateClientUrl(const sys::Mutex::ScopedLock&) {
 
 void HaBroker::setBrokerUrl(const Url& url, const sys::Mutex::ScopedLock& l) {
     if (url.empty()) throw Url::Invalid("HA broker URL is empty");
-    QPID_LOG(debug, logPrefix << "Setting broker URL to: " << url);
     brokerUrl = url;
     mgmtObject->set_brokers(brokerUrl.str());
     if (backup.get()) backup->setBrokerUrl(brokerUrl);
diff --git a/qpid/cpp/src/qpid/sys/posix/SystemInfo.cpp b/qpid/cpp/src/qpid/sys/posix/SystemInfo.cpp
index f6250f5831..9e66417aef 100755
--- a/qpid/cpp/src/qpid/sys/posix/SystemInfo.cpp
+++ b/qpid/cpp/src/qpid/sys/posix/SystemInfo.cpp
@@ -18,6 +18,7 @@
  *
  */
 
+#include "qpid/log/Statement.h"
 #include "qpid/sys/SystemInfo.h"
 #include "qpid/sys/posix/check.h"
 #include <set>