Qpid
  1. Qpid
  2. QPID-3690

Allocate a connection per federation bridge (route)

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 0.14
    • Fix Version/s: None
    • Component/s: C++ Broker
    • Labels:
      None

      Description

      All routes configured between two federated brokers share the same underlying connection. Since a connection can be serviced by only one thread, all traffic flowing over those routes will be serviced by just one thread.

      If a connection can be allocated for each route instead, then the traffic for each route could be handled by its own thread, allowing better scaling of federation links in a multiprocessor environment.

      Example: adding two routes to forward from queue1 and queue2 to exchange1 on a peer:

      qpid-route queue add broker1.fizzbin.com broker2.fizzbin.com exchange1 queue1
      qpid-route queue add broker1.fizzbin.com broker2.fizzbin.com exchange1 queue2

      with today's implementation, a single thread would be used to forward the traffic from both queue1 and queue2. If a connection is allocated to each route, then each queue's traffic could be serviced by its own thread.

        Activity

        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3331/
        -----------------------------------------------------------

        (Updated 2012-01-11 19:15:33.626155)

        Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross.

        Summary
        -------

        Proposed set of changes that will create a new connection for each bridge that is created on a link.

        This addresses bug QPID-3690.
        https://issues.apache.org/jira/browse/QPID-3690

        Diffs


        /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178
        /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178
        /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178
        /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178
        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178
        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178
        /trunk/qpid/cpp/src/tests/federation.py 1225178
        /trunk/qpid/specs/management-schema.xml 1225178
        /trunk/qpid/tools/src/py/qpid-tool 1225178

        Diff: https://reviews.apache.org/r/3331/diff

        Testing
        -------

        Unit tests.

        Thanks,

        Kenneth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3331/ ----------------------------------------------------------- (Updated 2012-01-11 19:15:33.626155) Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross. Summary ------- Proposed set of changes that will create a new connection for each bridge that is created on a link. This addresses bug QPID-3690 . https://issues.apache.org/jira/browse/QPID-3690 Diffs /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178 /trunk/qpid/cpp/src/tests/federation.py 1225178 /trunk/qpid/specs/management-schema.xml 1225178 /trunk/qpid/tools/src/py/qpid-tool 1225178 Diff: https://reviews.apache.org/r/3331/diff Testing ------- Unit tests. Thanks, Kenneth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3458/
        -----------------------------------------------------------

        Review request for Ted Ross.

        Summary
        -------

        Ted,

        Here's a set of changes that implement multiple named links between brokers, for early review. Let me know what you think - I'll open it up for further review after you get a chance to comment.

        Remaining work:
        1) federation tests pass, but some unit tests - including clustering -fail
        2) no support for link-groups yet, but some api changes in place
        3) no unit tests to verify the multi-link behavior.

        This addresses bug qpid-3690.
        https://issues.apache.org/jira/browse/qpid-3690

        Diffs


        /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1227616
        /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1227616
        /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1227616
        /trunk/qpid/cpp/src/qpid/broker/Link.h 1227616
        /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1227616
        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1227616
        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1227616
        /trunk/qpid/cpp/src/tests/federation.py 1227616
        /trunk/qpid/specs/management-schema.xml 1227616

        Diff: https://reviews.apache.org/r/3458/diff

        Testing
        -------

        Thanks,

        Kenneth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3458/ ----------------------------------------------------------- Review request for Ted Ross. Summary ------- Ted, Here's a set of changes that implement multiple named links between brokers, for early review. Let me know what you think - I'll open it up for further review after you get a chance to comment. Remaining work: 1) federation tests pass, but some unit tests - including clustering -fail 2) no support for link-groups yet, but some api changes in place 3) no unit tests to verify the multi-link behavior. This addresses bug qpid-3690. https://issues.apache.org/jira/browse/qpid-3690 Diffs /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1227616 /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1227616 /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1227616 /trunk/qpid/cpp/src/qpid/broker/Link.h 1227616 /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1227616 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1227616 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1227616 /trunk/qpid/cpp/src/tests/federation.py 1227616 /trunk/qpid/specs/management-schema.xml 1227616 Diff: https://reviews.apache.org/r/3458/diff Testing ------- Thanks, Kenneth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3331/#review4310
        -----------------------------------------------------------

        Having a connection per bridge would potentially allow a great deal of simplification, by collapsing the link and bridge concepts into a single entity. This change seems to add complexity. It looks neat enough, I'm not complaining about the changes. However I would like to consider whether this is an opportunity for some cleanup of an overly complex and fragile/brittle component. Thoughts?

        • Gordon

        On 2011-12-29 03:04:30, Kenneth Giusti wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3331/

        -----------------------------------------------------------

        (Updated 2011-12-29 03:04:30)

        Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross.

        Summary

        -------

        Proposed set of changes that will create a new connection for each bridge that is created on a link.

        This addresses bug QPID-3690.

        https://issues.apache.org/jira/browse/QPID-3690

        Diffs

        -----

        /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178

        /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178

        /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178

        /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178

        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178

        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178

        /trunk/qpid/cpp/src/tests/federation.py 1225178

        /trunk/qpid/specs/management-schema.xml 1225178

        /trunk/qpid/tools/src/py/qpid-tool 1225178

        Diff: https://reviews.apache.org/r/3331/diff

        Testing

        -------

        Unit tests.

        Thanks,

        Kenneth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3331/#review4310 ----------------------------------------------------------- Having a connection per bridge would potentially allow a great deal of simplification, by collapsing the link and bridge concepts into a single entity. This change seems to add complexity. It looks neat enough, I'm not complaining about the changes. However I would like to consider whether this is an opportunity for some cleanup of an overly complex and fragile/brittle component. Thoughts? Gordon On 2011-12-29 03:04:30, Kenneth Giusti wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3331/ ----------------------------------------------------------- (Updated 2011-12-29 03:04:30) Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross. Summary ------- Proposed set of changes that will create a new connection for each bridge that is created on a link. This addresses bug QPID-3690 . https://issues.apache.org/jira/browse/QPID-3690 Diffs ----- /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178 /trunk/qpid/cpp/src/tests/federation.py 1225178 /trunk/qpid/specs/management-schema.xml 1225178 /trunk/qpid/tools/src/py/qpid-tool 1225178 Diff: https://reviews.apache.org/r/3331/diff Testing ------- Unit tests. Thanks, Kenneth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3331/#review4183
        -----------------------------------------------------------

        Ship it!

        Only minor comments

        /trunk/qpid/cpp/src/qpid/broker/Bridge.h
        <https://reviews.apache.org/r/3331/#comment9418>

        I'd prefer returning strings by value rather than const ref. Returning a ref is a race condition if the string referenced is potentially deleted in a different thread. Probably not the case here but I'd prefer to keep a simple convention of return by value. The cost of an atomic counter inc/dec is pretty small and will be paid anyway if the caller assigns the return value to another string.

        /trunk/qpid/cpp/src/qpid/broker/Link.cpp
        <https://reviews.apache.org/r/3331/#comment9419>

        Why no log message here? Bridge connecting seems like a significant event.

        /trunk/qpid/cpp/src/qpid/broker/Link.cpp
        <https://reviews.apache.org/r/3331/#comment9420>

        I agree that this is OK.

        /trunk/qpid/cpp/src/qpid/broker/Link.cpp
        <https://reviews.apache.org/r/3331/#comment9421>

        Where (which class & thread) do we detect failure of a link connection to call this? Will it be called multiple times if multiple connections fail?

        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp
        <https://reviews.apache.org/r/3331/#comment9423>

        Should be updateAddressLH.

        • Alan

        On 2011-12-29 03:04:30, Kenneth Giusti wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3331/

        -----------------------------------------------------------

        (Updated 2011-12-29 03:04:30)

        Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross.

        Summary

        -------

        Proposed set of changes that will create a new connection for each bridge that is created on a link.

        This addresses bug QPID-3690.

        https://issues.apache.org/jira/browse/QPID-3690

        Diffs

        -----

        /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178

        /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178

        /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178

        /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178

        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178

        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178

        /trunk/qpid/cpp/src/tests/federation.py 1225178

        /trunk/qpid/specs/management-schema.xml 1225178

        /trunk/qpid/tools/src/py/qpid-tool 1225178

        Diff: https://reviews.apache.org/r/3331/diff

        Testing

        -------

        Unit tests.

        Thanks,

        Kenneth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3331/#review4183 ----------------------------------------------------------- Ship it! Only minor comments /trunk/qpid/cpp/src/qpid/broker/Bridge.h < https://reviews.apache.org/r/3331/#comment9418 > I'd prefer returning strings by value rather than const ref. Returning a ref is a race condition if the string referenced is potentially deleted in a different thread. Probably not the case here but I'd prefer to keep a simple convention of return by value. The cost of an atomic counter inc/dec is pretty small and will be paid anyway if the caller assigns the return value to another string. /trunk/qpid/cpp/src/qpid/broker/Link.cpp < https://reviews.apache.org/r/3331/#comment9419 > Why no log message here? Bridge connecting seems like a significant event. /trunk/qpid/cpp/src/qpid/broker/Link.cpp < https://reviews.apache.org/r/3331/#comment9420 > I agree that this is OK. /trunk/qpid/cpp/src/qpid/broker/Link.cpp < https://reviews.apache.org/r/3331/#comment9421 > Where (which class & thread) do we detect failure of a link connection to call this? Will it be called multiple times if multiple connections fail? /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp < https://reviews.apache.org/r/3331/#comment9423 > Should be updateAddressLH. Alan On 2011-12-29 03:04:30, Kenneth Giusti wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3331/ ----------------------------------------------------------- (Updated 2011-12-29 03:04:30) Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross. Summary ------- Proposed set of changes that will create a new connection for each bridge that is created on a link. This addresses bug QPID-3690 . https://issues.apache.org/jira/browse/QPID-3690 Diffs ----- /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178 /trunk/qpid/cpp/src/tests/federation.py 1225178 /trunk/qpid/specs/management-schema.xml 1225178 /trunk/qpid/tools/src/py/qpid-tool 1225178 Diff: https://reviews.apache.org/r/3331/diff Testing ------- Unit tests. Thanks, Kenneth
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3331/
        -----------------------------------------------------------

        Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross.

        Summary
        -------

        Proposed set of changes that will create a new connection for each bridge that is created on a link.

        This addresses bug QPID-3690.
        https://issues.apache.org/jira/browse/QPID-3690

        Diffs


        /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178
        /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178
        /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178
        /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178
        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178
        /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178
        /trunk/qpid/cpp/src/tests/federation.py 1225178
        /trunk/qpid/specs/management-schema.xml 1225178
        /trunk/qpid/tools/src/py/qpid-tool 1225178

        Diff: https://reviews.apache.org/r/3331/diff

        Testing
        -------

        Unit tests.

        Thanks,

        Kenneth

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3331/ ----------------------------------------------------------- Review request for Alan Conway, Gordon Sim, michael goulish, and Ted Ross. Summary ------- Proposed set of changes that will create a new connection for each bridge that is created on a link. This addresses bug QPID-3690 . https://issues.apache.org/jira/browse/QPID-3690 Diffs /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.h 1225178 /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1225178 /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1225178 /trunk/qpid/cpp/src/tests/federation.py 1225178 /trunk/qpid/specs/management-schema.xml 1225178 /trunk/qpid/tools/src/py/qpid-tool 1225178 Diff: https://reviews.apache.org/r/3331/diff Testing ------- Unit tests. Thanks, Kenneth

          People

          • Assignee:
            Ken Giusti
            Reporter:
            Ken Giusti
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Development