You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Andrew Stitcher <as...@apache.org> on 2013/01/02 20:44:06 UTC
Review Request: Change mechanism for specifying Connection management id for
federation Connections
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/
-----------------------------------------------------------
Review request for qpid and Kenneth Giusti.
Description
-------
This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
This addresses bug qpid-4315.
https://issues.apache.org/jira/browse/qpid-4315
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Broker.h 1426152
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1426152
/trunk/qpid/cpp/src/qpid/broker/Link.h 1426152
/trunk/qpid/cpp/src/qpid/broker/Link.cpp 1426152
/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1426152
/trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1426152
/trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1426152
Diff: https://reviews.apache.org/r/8804/diff/
Testing
-------
Passes make check
Thanks,
Andrew Stitcher
Re: Review Request: Change mechanism for specifying Connection management id
for federation Connections
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/#review15158
-----------------------------------------------------------
Ship it!
Ship It!
- Kenneth Giusti
On Jan. 8, 2013, 7:36 p.m., Andrew Stitcher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8804/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2013, 7:36 p.m.)
>
>
> Review request for qpid and Kenneth Giusti.
>
>
> Description
> -------
>
> This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
>
> Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
>
> Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
>
>
> This addresses bug qpid-4315.
> https://issues.apache.org/jira/browse/qpid-4315
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.h 1428722
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1428722
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1428722
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1428722
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1428722
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1428722
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1428722
> /trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1428722
> /trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1428722
> /trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1428722
> /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1428722
>
> Diff: https://reviews.apache.org/r/8804/diff/
>
>
> Testing
> -------
>
> Passes make check
>
>
> Thanks,
>
> Andrew Stitcher
>
>
Re: Review Request: Change mechanism for specifying Connection management id
for federation Connections
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/
-----------------------------------------------------------
(Updated Jan. 8, 2013, 7:36 p.m.)
Review request for qpid and Kenneth Giusti.
Changes
-------
This version of the change alters the way Connection names are constructed by adding a "qpid." to the fron t to ensure that the user can't use the name.
I rebased the change which has caused a bunch of include churn as well - that can be ignored.
Description
-------
This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
This addresses bug qpid-4315.
https://issues.apache.org/jira/browse/qpid-4315
Diffs (updated)
-----
/trunk/qpid/cpp/src/qpid/broker/Broker.h 1428722
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1428722
/trunk/qpid/cpp/src/qpid/broker/Link.h 1428722
/trunk/qpid/cpp/src/qpid/broker/Link.cpp 1428722
/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1428722
/trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1428722
/trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1428722
/trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1428722
/trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1428722
/trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1428722
/trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1428722
Diff: https://reviews.apache.org/r/8804/diff/
Testing
-------
Passes make check
Thanks,
Andrew Stitcher
Re: Review Request: Change mechanism for specifying Connection management id
for federation Connections
Posted by Andrew Stitcher <as...@apache.org>.
> On Jan. 2, 2013, 9:09 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 758
> > <https://reviews.apache.org/r/8804/diff/2/?file=244412#file244412line758>
> >
> > One (potential) issue: the link name is supplied by the user and is pretty much free-format (with the exception of starting it with the prefix "qpid").
> >
> > Would there be an issue if the user assigned link name collided with an incoming connection name?
Hmm, yes I think that if an administrator chose a link name that was identical to another connection name, then Connection events coming from that Connection might be confused with events coming from the real Connection associated with the Link. This would be a good reason to do a small amount of "uniquifying" to the name we use for the connection.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/#review15019
-----------------------------------------------------------
On Jan. 2, 2013, 8:25 p.m., Andrew Stitcher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8804/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2013, 8:25 p.m.)
>
>
> Review request for qpid and Kenneth Giusti.
>
>
> Description
> -------
>
> This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
>
> Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
>
> Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
>
>
> This addresses bug qpid-4315.
> https://issues.apache.org/jira/browse/qpid-4315
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.h 1426152
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1426152
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1426152
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1426152
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1426152
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1426152
> /trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1426152
>
> Diff: https://reviews.apache.org/r/8804/diff/
>
>
> Testing
> -------
>
> Passes make check
>
>
> Thanks,
>
> Andrew Stitcher
>
>
Re: Review Request: Change mechanism for specifying Connection management id
for federation Connections
Posted by Andrew Stitcher <as...@apache.org>.
> On Jan. 2, 2013, 9:09 p.m., Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 758
> > <https://reviews.apache.org/r/8804/diff/2/?file=244412#file244412line758>
> >
> > One (potential) issue: the link name is supplied by the user and is pretty much free-format (with the exception of starting it with the prefix "qpid").
> >
> > Would there be an issue if the user assigned link name collided with an incoming connection name?
>
> Andrew Stitcher wrote:
> Hmm, yes I think that if an administrator chose a link name that was identical to another connection name, then Connection events coming from that Connection might be confused with events coming from the real Connection associated with the Link. This would be a good reason to do a small amount of "uniquifying" to the name we use for the connection.
I've fixed this potential problem by making all the broker chosen Connection names be prefixed by qpid. which is illegal for user specified link names.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/#review15019
-----------------------------------------------------------
On Jan. 2, 2013, 8:25 p.m., Andrew Stitcher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8804/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2013, 8:25 p.m.)
>
>
> Review request for qpid and Kenneth Giusti.
>
>
> Description
> -------
>
> This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
>
> Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
>
> Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
>
>
> This addresses bug qpid-4315.
> https://issues.apache.org/jira/browse/qpid-4315
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.h 1426152
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1426152
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1426152
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1426152
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1426152
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1426152
> /trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1426152
>
> Diff: https://reviews.apache.org/r/8804/diff/
>
>
> Testing
> -------
>
> Passes make check
>
>
> Thanks,
>
> Andrew Stitcher
>
>
Re: Review Request: Change mechanism for specifying Connection management id
for federation Connections
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/#review15019
-----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/8804/#comment32452>
One (potential) issue: the link name is supplied by the user and is pretty much free-format (with the exception of starting it with the prefix "qpid").
Would there be an issue if the user assigned link name collided with an incoming connection name?
- Kenneth Giusti
On Jan. 2, 2013, 8:25 p.m., Andrew Stitcher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8804/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2013, 8:25 p.m.)
>
>
> Review request for qpid and Kenneth Giusti.
>
>
> Description
> -------
>
> This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
>
> Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
>
> Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
>
>
> This addresses bug qpid-4315.
> https://issues.apache.org/jira/browse/qpid-4315
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.h 1426152
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1426152
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1426152
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1426152
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1426152
> /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1426152
> /trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1426152
> /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1426152
>
> Diff: https://reviews.apache.org/r/8804/diff/
>
>
> Testing
> -------
>
> Passes make check
>
>
> Thanks,
>
> Andrew Stitcher
>
>
Re: Review Request: Change mechanism for specifying Connection management id
for federation Connections
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8804/
-----------------------------------------------------------
(Updated Jan. 2, 2013, 8:25 p.m.)
Review request for qpid and Kenneth Giusti.
Changes
-------
Removed a now unused function
Description
-------
This change removes the ad hoc deconstruction of Connection management ids to discover the end points of the Connection so that they can be used by to look up a Link in the LinkRegistry.
Instead we allow the federation code to specify the management id of the Connection directly then use this specified name as an index to find the Link directly.
Currently this makes the Connnection id identical to the Link name that is using the connection, it would be easy to change this to some other convention, but this seems simple.
This addresses bug qpid-4315.
https://issues.apache.org/jira/browse/qpid-4315
Diffs (updated)
-----
/trunk/qpid/cpp/src/qpid/broker/Broker.h 1426152
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1426152
/trunk/qpid/cpp/src/qpid/broker/Link.h 1426152
/trunk/qpid/cpp/src/qpid/broker/Link.cpp 1426152
/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.h 1426152
/trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/ProtocolFactory.h 1426152
/trunk/qpid/cpp/src/qpid/sys/RdmaIOPlugin.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/SslPlugin.cpp 1426152
/trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1426152
Diff: https://reviews.apache.org/r/8804/diff/
Testing
-------
Passes make check
Thanks,
Andrew Stitcher