qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway" <acon...@redhat.com>
Subject Re: Review Request: Modify Bridge & Link indexing to use explicit names.
Date Thu, 19 Jan 2012 16:15:12 GMT

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


Looks good overall.


/trunk/qpid/cpp/src/qpid/broker/Bridge.cpp
<https://reviews.apache.org/r/3546/#comment10023>

    Is there a backward compatibility issue with adding the new fields at the front? 



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

    I'm not keen on failing over in maintenance visit. That is potentially a long delay till
a broker reconnects. I think we need to be able to trigger reconnects immediately on detecting
a close.



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

    Not sure I understand the retry semantics, I think this can be simplified. E.g. if the
next url isn't different you should keep going till you find one that is, not fall thru to
the currentInterval logic. Likewise if next returns false, you should just keep going from
the start, not fall thru.
    



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

    Throw rather than assert?



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

    ditto
    



/trunk/qpid/specs/management-schema.xml
<https://reviews.apache.org/r/3546/#comment10042>

    Why is this RC rather than RO? Can you change durability?
    


- Alan


On 2012-01-19 14:22:45, Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3546/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 14:22:45)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, michael goulish, and Ted Ross.
> 
> 
> Summary
> -------
> 
> This patch modifies the way the broker's Link and Bridge objects are identified and managed.
 Specifically:
> 
> 1) both Bridge and Links are now identified by explict names assigned by management,
rather than destination host/port info.
>    - names beginning with the prefix "qpid." are reserved for qpidd internal use.
>    - for backward compatibility, if no name is assigned on creation, the broker will
generate a name based on UUID
> 2) the corresponding QMF objects have been updated accordingly, with the additions of:
>    - the QMF Link object has been updated to provide a reference to the corresponding
Connection
>    - the QMF Link object has been modified to allow the host/port/connectionRef to change
on failover
>    - the QMF Bridge object has been modified to allow the Channel identifier to change
(allowing Bridges to be reassigned to different links in the future)
> 3) Links/Bridges may now be created/deleted via the QMF Broker's generic "create" and
"delete" methods
> 4) Some consolidation of the Link/Bridge creation APIs, specifically:
>    - Link/Bridges are created via calls to the LinkRegistry's "declare()" methods
>    - Link/Bridges are removed by calling their corresponding "destroy()" methods
> 
> More importantly, the above changes make it possible to create multiple Links between
the same two brokers.  This can be done by creating Links to the same destinations with different
names.  This is a change from the existing behavior, which uses the destination host/port
as the unique Link identifier.
> 
> 
> This addresses bug qpid-3767.
>     https://issues.apache.org/jira/browse/qpid-3767
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Bridge.h 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/Link.h 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h 1233125 
>   /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1233125 
>   /trunk/qpid/cpp/src/tests/federation.py 1233125 
>   /trunk/qpid/specs/management-schema.xml 1233125 
> 
> Diff: https://reviews.apache.org/r/3546/diff
> 
> 
> Testing
> -------
> 
> This patch fails to pass some of the cluster tests - I'm investigating this now.  All
non-cluster federation tests where passing (prior to my latest rebase).
> 
> Work remains, but I wanted to get this patch out for discussion before going much farther.
> 
> 
> Thanks,
> 
> Kenneth
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message