Author: aconway
Date: Fri Dec 14 15:30:40 2012
New Revision: 1421934
URL: http://svn.apache.org/viewvc?rev=1421934&view=rev
Log:
QPID-4506: Qpid HA's '--ha-public-url' option duplicates the
'--known-hosts-url' option but cannot be disabled
Reverts the previous commit and simplifies the semantics of setting
--ha-public-url and --ha-brokers-url. There is no longer any over-riding or
implicit updating of values. That means you must set --ha-public-url as well as
--ha-brokers-url, it will not be defaulted. Likewise if you *dont* set
ha-public-url, it will remain empty, which is the use case in this bug.
The defaulting was adding complexity without adding much value.
Modified:
qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp
qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.h
qpid/trunk/qpid/cpp/src/qpid/ha/HaPlugin.cpp
qpid/trunk/qpid/cpp/src/qpid/ha/Settings.h
qpid/trunk/qpid/cpp/src/tests/ha_test.py
qpid/trunk/qpid/cpp/src/tests/ha_tests.py
qpid/trunk/qpid/doc/book/src/cpp-broker/Active-Passive-Cluster.xml
Modified: qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp Fri Dec 14 15:30:40 2012
@@ -79,6 +79,11 @@ HaBroker::HaBroker(broker::Broker& b, co
}
}
+namespace {
+const std::string NONE("none");
+bool isNone(const std::string& x) { return x.empty() || x == NONE; }
+}
+
// Called in Plugin::initialize
void HaBroker::initialize() {
@@ -110,11 +115,10 @@ void HaBroker::initialize() {
backup.reset(new Backup(*this, settings));
broker.getKnownBrokers = boost::bind(&HaBroker::getKnownBrokers, this);
statusCheck.reset(new StatusCheck(logPrefix,
broker.getLinkHearbeatInterval(), brokerInfo));
+ if (!isNone(settings.publicUrl)) setPublicUrl(Url(settings.publicUrl));
+ if (!isNone(settings.brokerUrl)) setBrokerUrl(Url(settings.brokerUrl));
}
- if (!settings.clientUrl.empty()) setClientUrl(Url(settings.clientUrl));
- if (!settings.brokerUrl.empty()) setBrokerUrl(Url(settings.brokerUrl));
-
// NOTE: lock is not needed in a constructor, but create one
// to pass to functions that have a ScopedLock parameter.
@@ -182,7 +186,7 @@ Manageable::status_t HaBroker::Managemen
break;
}
case _qmf::HaBroker::METHOD_SETPUBLICURL: {
-
setClientUrl(Url(dynamic_cast<_qmf::ArgsHaBrokerSetPublicUrl&>(args).i_url));
+
setPublicUrl(Url(dynamic_cast<_qmf::ArgsHaBrokerSetPublicUrl&>(args).i_url));
break;
}
case _qmf::HaBroker::METHOD_REPLICATE: {
@@ -217,19 +221,13 @@ Manageable::status_t HaBroker::Managemen
return Manageable::STATUS_OK;
}
-void HaBroker::setClientUrl(const Url& url) {
+void HaBroker::setPublicUrl(const Url& url) {
Mutex::ScopedLock l(lock);
- if (url.empty()) throw Exception("Invalid empty URL for HA client
failover");
- clientUrl = url;
- updateClientUrl(l);
-}
-
-void HaBroker::updateClientUrl(Mutex::ScopedLock&) {
- Url url = clientUrl.empty() ? brokerUrl : clientUrl;
+ publicUrl = url;
mgmtObject->set_publicUrl(url.str());
knownBrokers.clear();
knownBrokers.push_back(url);
- QPID_LOG(debug, logPrefix << "Setting client URL to: " << url);
+ QPID_LOG(debug, logPrefix << "Setting public URL to: " << url);
}
void HaBroker::setBrokerUrl(const Url& url) {
@@ -238,10 +236,8 @@ void HaBroker::setBrokerUrl(const Url& u
Mutex::ScopedLock l(lock);
brokerUrl = url;
mgmtObject->set_brokersUrl(brokerUrl.str());
- QPID_LOG(info, logPrefix << "Broker URL set to: " << url);
+ QPID_LOG(info, logPrefix << "Brokers URL set to: " << url);
if (status == JOINING && statusCheck.get()) statusCheck->setUrl(url);
- // Updating broker URL also updates defaulted client URL:
- if (clientUrl.empty()) updateClientUrl(l);
b = backup;
}
if (b) b->setBrokerUrl(url); // Oustside lock, avoid deadlock
Modified: qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.h?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/HaBroker.h Fri Dec 14 15:30:40 2012
@@ -100,7 +100,7 @@ class HaBroker : public management::Mana
types::Uuid getSystemId() const { return systemId; }
private:
- void setClientUrl(const Url&);
+ void setPublicUrl(const Url&);
void setBrokerUrl(const Url&);
void updateClientUrl(sys::Mutex::ScopedLock&);
@@ -125,7 +125,7 @@ class HaBroker : public management::Mana
boost::shared_ptr<Backup> backup;
boost::shared_ptr<Primary> primary;
qmf::org::apache::qpid::ha::HaBroker::shared_ptr mgmtObject;
- Url clientUrl, brokerUrl;
+ Url publicUrl, brokerUrl;
std::vector<Url> knownBrokers;
BrokerStatus status;
BrokerInfo brokerInfo;
Modified: qpid/trunk/qpid/cpp/src/qpid/ha/HaPlugin.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/HaPlugin.cpp?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/HaPlugin.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/HaPlugin.cpp Fri Dec 14 15:30:40 2012
@@ -37,7 +37,7 @@ struct Options : public qpid::Options {
"Enable replication of specific queues without joining a cluster")
("ha-brokers-url", optValue(settings.brokerUrl,"URL"),
"URL with address of each broker in the cluster.")
- ("ha-public-url", optValue(settings.clientUrl,"URL"),
+ ("ha-public-url", optValue(settings.publicUrl,"URL"),
"URL advertized to clients to connect to the cluster.")
("ha-replicate",
optValue(settings.replicateDefault, "LEVEL"),
Modified: qpid/trunk/qpid/cpp/src/qpid/ha/Settings.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/ha/Settings.h?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/ha/Settings.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/ha/Settings.h Fri Dec 14 15:30:40 2012
@@ -42,7 +42,7 @@ class Settings
bool cluster; // True if we are a cluster member.
bool queueReplication; // True if enabled.
- std::string clientUrl;
+ std::string publicUrl;
std::string brokerUrl;
Enum<ReplicateLevel> replicateDefault;
std::string username, password, mechanism;
Modified: qpid/trunk/qpid/cpp/src/tests/ha_test.py
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/ha_test.py?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/ha_test.py (original)
+++ qpid/trunk/qpid/cpp/src/tests/ha_test.py Fri Dec 14 15:30:40 2012
@@ -100,7 +100,7 @@ class HaBroker(Broker):
self.qpid_ha_script.main_except(["", "-b", url]+args)
def promote(self): self.ready(); self.qpid_ha(["promote"])
- def set_client_url(self, url): self.qpid_ha(["set", "--public-url", url])
+ def set_public_url(self, url): self.qpid_ha(["set", "--public-url", url])
def set_brokers_url(self, url): self.qpid_ha(["set", "--brokers-url", url])
def replicate(self, from_broker, queue): self.qpid_ha(["replicate",
from_broker, queue])
@@ -113,10 +113,12 @@ class HaBroker(Broker):
self._agent = QmfAgent(self.host_port())
return self._agent
- def ha_status(self):
+ def qmf(self):
hb = self.agent().getHaBroker()
hb.update()
- return hb.status
+ return hb
+
+ def ha_status(self): return self.qmf().status
def wait_status(self, status):
def try_get_status():
@@ -234,7 +236,9 @@ class HaCluster(object):
def update_urls(self):
self.url = ",".join([b.host_port() for b in self])
if len(self) > 1: # No failover addresses on a 1 cluster.
- for b in self: b.set_brokers_url(self.url)
+ for b in self:
+ b.set_brokers_url(self.url)
+ b.set_public_url(self.url)
def connect(self, i):
"""Connect with reconnect_urls"""
Modified: qpid/trunk/qpid/cpp/src/tests/ha_tests.py
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/ha_tests.py?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/ha_tests.py (original)
+++ qpid/trunk/qpid/cpp/src/tests/ha_tests.py Fri Dec 14 15:30:40 2012
@@ -782,9 +782,9 @@ acl deny all all
cluster[1].wait_queue("q0")
cluster[1].wait_queue("q1")
cluster[0].kill()
- cluster[1].wait_queue("q1") # Not timed out yet
- cluster[1].wait_no_queue("q1", timeout=2) # Wait for timeout
- cluster[1].wait_no_queue("q0", timeout=2)
+ cluster[1].wait_queue("q1") # Not timed out yet
+ cluster[1].wait_no_queue("q1") # Wait for timeout
+ cluster[1].wait_no_queue("q0")
def test_alt_exchange_dup(self):
"""QPID-4349: if a queue has an alterante exchange and is deleted the
@@ -1135,6 +1135,38 @@ class RecoveryTests(HaBrokerTest):
cluster.bounce(0, promote_next=False)
cluster[0].promote()
+
+class ConfigurationTests(HaBrokerTest):
+ """Tests for configuration settings."""
+
+ def test_client_broker_url(self):
+ """Check that setting of broker and public URLs obeys correct
defaulting
+ and precedence"""
+
+ def check(broker, brokers, public):
+ qmf = broker.qmf()
+ self.assertEqual(brokers, qmf.brokersUrl)
+ self.assertEqual(public, qmf.publicUrl)
+
+ def start(brokers, public, known=None):
+ args=[]
+ if brokers: args.append("--ha-brokers-url="+brokers)
+ if public: args.append("--ha-public-url="+public)
+ if known: args.append("--known-hosts-url="+known)
+ return HaBroker(self, args=args)
+
+ # Both set explictily, no defaulting
+ b = start("foo:123", "bar:456")
+ check(b, "amqp:tcp:foo:123", "amqp:tcp:bar:456")
+ b.set_brokers_url("foo:999")
+ check(b, "amqp:tcp:foo:999", "amqp:tcp:bar:456")
+ b.set_public_url("bar:999")
+ check(b, "amqp:tcp:foo:999", "amqp:tcp:bar:999")
+
+ # Allow "none" to mean "not set"
+ b = start("none", "none")
+ check(b, "", "")
+
if __name__ == "__main__":
shutil.rmtree("brokertest.tmp", True)
qpid_ha = os.getenv("QPID_HA_EXEC")
Modified: qpid/trunk/qpid/doc/book/src/cpp-broker/Active-Passive-Cluster.xml
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/doc/book/src/cpp-broker/Active-Passive-Cluster.xml?rev=1421934&r1=1421933&r2=1421934&view=diff
==============================================================================
--- qpid/trunk/qpid/doc/book/src/cpp-broker/Active-Passive-Cluster.xml
(original)
+++ qpid/trunk/qpid/doc/book/src/cpp-broker/Active-Passive-Cluster.xml Fri Dec
14 15:30:40 2012
@@ -288,8 +288,7 @@ ssl_addr = "ssl:" host [":" port]'
</footnote>
used by cluster brokers to connect to each other. The URL should
contain a comma separated list of the broker addresses, rather
than a
- virtual IP address. For example:
-
<literal>amqp:node1.exaple.com,node2.exaple.com,node3.exaple.com</literal>
+ virtual IP address.
</para>
</entry>
</row>
@@ -297,14 +296,18 @@ ssl_addr = "ssl:" host [":" port]'
<entry><literal>ha-public-url
<replaceable>URL</replaceable></literal> </entry>
<entry>
<para>
- The URL <footnoteref linkend="ha-url-grammar"/> used by clients
to connect to the cluster. This can be a list or
- a single virtual IP address. A virtual IP address is
recommended as it
- simplifies deployment. If not specified this defaults to the
value of
- <literal>ha-brokers-url</literal>.
+ The URL <footnoteref linkend="ha-url-grammar"/> is advertised to
+ clients as the "known-hosts" for fail-over. It can be a list or
+ a single virtual IP address. A virtual IP address is
recommended.
</para>
<para>
- This option allows you to put client traffic on a different
network from
- broker traffic, which is recommended.
+ Using this option you can put client and broker traffic on
+ separate networks, which is recommended.
+ </para>
+ <para>
+ Note: When HA clustering is enabled the broker option
+ <literal>known-hosts-url</literal> is ignored and over-ridden by
+ the <literal>ha-public-url</literal> setting.
</para>
</entry>
</row>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
