You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2012/04/05 16:45:55 UTC
Review Request: QPID-3767: identify Link and Bridge objects by an assigned
name, rather than by the remote Host and Port.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4659/
-----------------------------------------------------------
Review request for qpid, Alan Conway, Gordon Sim, Ted Ross, and Rob Godfrey.
Summary
-------
This set of changes modifies the Broker's Link and Bridge objects by changing the way they are indexed. This change is visible to users as it modifies the existing management model for this objects.
The current implementation uses the remote broker's Host and Port value to identify the Link, and also as part of the index for identifiying the Bridge.
The problem with this approach is that it does not support failover well, where the remote Host and Port values are likely to change.
This patch modifies the Link and Bridge object to now be indexed by a unique name (expressed as a string). It removes the host & port indexing from the QMF model. With this patch, host and port are allowed to change over time without invalidating the Link or Bridge.
I've also added a reference to the connection that the Link is using - this useful information was missing from the existing model.
This patch also introduces the ability to create Links and Bridges via the QMF Broker object's "create" and "delete" method. This feature is yet to be used by qpid-route, however, and the old methods to create Links and Bridges using host and ports are preserved for backward compatibility. We can remove this if desired in a later release.
Things this patch does not do, and I'll like to track as separate feature requests if possible:
o) Modify the qpid-route tool to allow the ability to name links and bridges if desired - and allowe more control over the management of Links & Bridges - details TBD.
o) Java broker support of Link and Bridge naming, and the ability to manage Links/Bridges via the broker's create/delete methods.
Alan - my changes to the HA code are at best a guess, can you take a look?
Rob - ditto my changes to the Java broker.
thanks
This addresses bug qpid-3767.
https://issues.apache.org/jira/browse/qpid-3767
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1306454
/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1306454
/trunk/qpid/cpp/src/qpid/broker/Link.cpp 1306454
/trunk/qpid/cpp/src/qpid/broker/Link.h 1306454
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1306454
/trunk/qpid/cpp/src/qpid/broker/Bridge.h 1306454
/trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1306454
/trunk/qpid/cpp/src/qpid/broker/NameGenerator.h 1306454
/trunk/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1306454
/trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1306454
/trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1306454
/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1306454
/trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1306454
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1306454
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1306454
/trunk/qpid/cpp/src/tests/federation.py 1306454
/trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1306454
/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/LinkConfigType.java 1306454
/trunk/qpid/specs/management-schema.xml 1306454
/trunk/qpid/tools/src/py/qpid-tool 1306454
Diff: https://reviews.apache.org/r/4659/diff
Testing
-------
make check,
ant build/test
windows build via Hudson
Thanks,
Kenneth
Re: Review Request: QPID-3767: identify Link and Bridge objects by an
assigned name, rather than by the remote Host and Port.
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4659/#review6841
-----------------------------------------------------------
Ship it!
- Gordon
On 2012-04-05 14:45:55, Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4659/
> -----------------------------------------------------------
>
> (Updated 2012-04-05 14:45:55)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Ted Ross, and Rob Godfrey.
>
>
> Summary
> -------
>
> This set of changes modifies the Broker's Link and Bridge objects by changing the way they are indexed. This change is visible to users as it modifies the existing management model for this objects.
>
> The current implementation uses the remote broker's Host and Port value to identify the Link, and also as part of the index for identifiying the Bridge.
>
> The problem with this approach is that it does not support failover well, where the remote Host and Port values are likely to change.
>
> This patch modifies the Link and Bridge object to now be indexed by a unique name (expressed as a string). It removes the host & port indexing from the QMF model. With this patch, host and port are allowed to change over time without invalidating the Link or Bridge.
>
> I've also added a reference to the connection that the Link is using - this useful information was missing from the existing model.
>
> This patch also introduces the ability to create Links and Bridges via the QMF Broker object's "create" and "delete" method. This feature is yet to be used by qpid-route, however, and the old methods to create Links and Bridges using host and ports are preserved for backward compatibility. We can remove this if desired in a later release.
>
> Things this patch does not do, and I'll like to track as separate feature requests if possible:
>
> o) Modify the qpid-route tool to allow the ability to name links and bridges if desired - and allowe more control over the management of Links & Bridges - details TBD.
> o) Java broker support of Link and Bridge naming, and the ability to manage Links/Bridges via the broker's create/delete methods.
>
> Alan - my changes to the HA code are at best a guess, can you take a look?
> Rob - ditto my changes to the Java broker.
>
> thanks
>
>
> This addresses bug qpid-3767.
> https://issues.apache.org/jira/browse/qpid-3767
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/NameGenerator.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1306454
> /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1306454
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1306454
> /trunk/qpid/cpp/src/tests/federation.py 1306454
> /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1306454
> /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/LinkConfigType.java 1306454
> /trunk/qpid/specs/management-schema.xml 1306454
> /trunk/qpid/tools/src/py/qpid-tool 1306454
>
> Diff: https://reviews.apache.org/r/4659/diff
>
>
> Testing
> -------
>
> make check,
> ant build/test
> windows build via Hudson
>
>
> Thanks,
>
> Kenneth
>
>
Re: Review Request: QPID-3767: identify Link and Bridge objects by
an assigned name, rather than by the remote Host and Port.
Posted by Alan Conway <ac...@redhat.com>.
On 04/11/2012 09:06 AM, Kenneth Giusti wrote:
>
>
>> On 2012-04-10 14:26:01, Alan Conway wrote:
>>> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp, line 57
>>> <https://reviews.apache.org/r/4659/diff/1/?file=100190#file100190line57>
>>>
>>> replicatorName(q->getName()) is already unique within the broker (actually its an exchange name) so the UUID is not necessary.
>
> Hi Alan - thanks for the review. I'm cleaning up these nits, however:
>
> If I change the setting of the bridgeName to remove the addition of the uuid, and do this instead:
> bridgeName = replicatorName(q->getName());
> then the following ha tests start failing:
>
> ha_tests.ShortTests.test_backup_failover:
> AssertionError: Lists differ: ['a'] != []
> ha_tests.ShortTests.test_queue_replica_failover:
> AssertionError: Lists differ: ['a'] != []
> ha_tests.ShortTests.test_send_receive:
> AssertionError: Lists differ: [991L, 992L, 993L, 994L, 995L,... != []
> ha_tests.ShortTests.test_sync:
> AssertionError: Lists differ: ['0', '1', '2', '3', '4', '5',... != []
>
> I'll re-try this change after I merge the branch to your latest changes on trunk.
Just leave in the UUID, I must be overlooking something...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: Review Request: QPID-3767: identify Link and Bridge objects by an
assigned name, rather than by the remote Host and Port.
Posted by Kenneth Giusti <kg...@apache.org>.
> On 2012-04-10 14:26:01, Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp, line 57
> > <https://reviews.apache.org/r/4659/diff/1/?file=100190#file100190line57>
> >
> > replicatorName(q->getName()) is already unique within the broker (actually its an exchange name) so the UUID is not necessary.
Hi Alan - thanks for the review. I'm cleaning up these nits, however:
If I change the setting of the bridgeName to remove the addition of the uuid, and do this instead:
bridgeName = replicatorName(q->getName());
then the following ha tests start failing:
ha_tests.ShortTests.test_backup_failover:
AssertionError: Lists differ: ['a'] != []
ha_tests.ShortTests.test_queue_replica_failover:
AssertionError: Lists differ: ['a'] != []
ha_tests.ShortTests.test_send_receive:
AssertionError: Lists differ: [991L, 992L, 993L, 994L, 995L,... != []
ha_tests.ShortTests.test_sync:
AssertionError: Lists differ: ['0', '1', '2', '3', '4', '5',... != []
I'll re-try this change after I merge the branch to your latest changes on trunk.
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4659/#review6823
-----------------------------------------------------------
On 2012-04-05 14:45:55, Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4659/
> -----------------------------------------------------------
>
> (Updated 2012-04-05 14:45:55)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Ted Ross, and Rob Godfrey.
>
>
> Summary
> -------
>
> This set of changes modifies the Broker's Link and Bridge objects by changing the way they are indexed. This change is visible to users as it modifies the existing management model for this objects.
>
> The current implementation uses the remote broker's Host and Port value to identify the Link, and also as part of the index for identifiying the Bridge.
>
> The problem with this approach is that it does not support failover well, where the remote Host and Port values are likely to change.
>
> This patch modifies the Link and Bridge object to now be indexed by a unique name (expressed as a string). It removes the host & port indexing from the QMF model. With this patch, host and port are allowed to change over time without invalidating the Link or Bridge.
>
> I've also added a reference to the connection that the Link is using - this useful information was missing from the existing model.
>
> This patch also introduces the ability to create Links and Bridges via the QMF Broker object's "create" and "delete" method. This feature is yet to be used by qpid-route, however, and the old methods to create Links and Bridges using host and ports are preserved for backward compatibility. We can remove this if desired in a later release.
>
> Things this patch does not do, and I'll like to track as separate feature requests if possible:
>
> o) Modify the qpid-route tool to allow the ability to name links and bridges if desired - and allowe more control over the management of Links & Bridges - details TBD.
> o) Java broker support of Link and Bridge naming, and the ability to manage Links/Bridges via the broker's create/delete methods.
>
> Alan - my changes to the HA code are at best a guess, can you take a look?
> Rob - ditto my changes to the Java broker.
>
> thanks
>
>
> This addresses bug qpid-3767.
> https://issues.apache.org/jira/browse/qpid-3767
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/NameGenerator.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1306454
> /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1306454
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1306454
> /trunk/qpid/cpp/src/tests/federation.py 1306454
> /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1306454
> /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/LinkConfigType.java 1306454
> /trunk/qpid/specs/management-schema.xml 1306454
> /trunk/qpid/tools/src/py/qpid-tool 1306454
>
> Diff: https://reviews.apache.org/r/4659/diff
>
>
> Testing
> -------
>
> make check,
> ant build/test
> windows build via Hudson
>
>
> Thanks,
>
> Kenneth
>
>
Re: Review Request: QPID-3767: identify Link and Bridge objects by an
assigned name, rather than by the remote Host and Port.
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4659/#review6823
-----------------------------------------------------------
Ship it!
Nit-picking only
/trunk/qpid/cpp/src/qpid/broker/Bridge.h
<https://reviews.apache.org/r/4659/#comment15188>
Why passing just the name rather than the Bridge pointer in cancellationlistener? The bridge is more useful.
/trunk/qpid/cpp/src/qpid/broker/Bridge.cpp
<https://reviews.apache.org/r/4659/#comment15196>
Nit: for consistency use size() rather than length()
/trunk/qpid/cpp/src/qpid/broker/Link.cpp
<https://reviews.apache.org/r/4659/#comment15199>
ostream << Address does print transport:host:protocol if I recall..
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp
<https://reviews.apache.org/r/4659/#comment15197>
replicatorName(q->getName()) is already unique within the broker (actually its an exchange name) so the UUID is not necessary.
/trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp
<https://reviews.apache.org/r/4659/#comment15198>
Lock is not needed. Activate is called before any concurrent activity starts. (actually not quite true on trunk now, will be true with my next checkin...)
- Alan
On 2012-04-05 14:45:55, Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4659/
> -----------------------------------------------------------
>
> (Updated 2012-04-05 14:45:55)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Ted Ross, and Rob Godfrey.
>
>
> Summary
> -------
>
> This set of changes modifies the Broker's Link and Bridge objects by changing the way they are indexed. This change is visible to users as it modifies the existing management model for this objects.
>
> The current implementation uses the remote broker's Host and Port value to identify the Link, and also as part of the index for identifiying the Bridge.
>
> The problem with this approach is that it does not support failover well, where the remote Host and Port values are likely to change.
>
> This patch modifies the Link and Bridge object to now be indexed by a unique name (expressed as a string). It removes the host & port indexing from the QMF model. With this patch, host and port are allowed to change over time without invalidating the Link or Bridge.
>
> I've also added a reference to the connection that the Link is using - this useful information was missing from the existing model.
>
> This patch also introduces the ability to create Links and Bridges via the QMF Broker object's "create" and "delete" method. This feature is yet to be used by qpid-route, however, and the old methods to create Links and Bridges using host and ports are preserved for backward compatibility. We can remove this if desired in a later release.
>
> Things this patch does not do, and I'll like to track as separate feature requests if possible:
>
> o) Modify the qpid-route tool to allow the ability to name links and bridges if desired - and allowe more control over the management of Links & Bridges - details TBD.
> o) Java broker support of Link and Bridge naming, and the ability to manage Links/Bridges via the broker's create/delete methods.
>
> Alan - my changes to the HA code are at best a guess, can you take a look?
> Rob - ditto my changes to the Java broker.
>
> thanks
>
>
> This addresses bug qpid-3767.
> https://issues.apache.org/jira/browse/qpid-3767
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1306454
> /trunk/qpid/cpp/src/qpid/broker/NameGenerator.h 1306454
> /trunk/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1306454
> /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1306454
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1306454
> /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1306454
> /trunk/qpid/cpp/src/tests/federation.py 1306454
> /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1306454
> /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/LinkConfigType.java 1306454
> /trunk/qpid/specs/management-schema.xml 1306454
> /trunk/qpid/tools/src/py/qpid-tool 1306454
>
> Diff: https://reviews.apache.org/r/4659/diff
>
>
> Testing
> -------
>
> make check,
> ant build/test
> windows build via Hudson
>
>
> Thanks,
>
> Kenneth
>
>