qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for exclusive queues.
Date Fri, 08 Feb 2013 09:35:07 GMT

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



/trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp
<https://reviews.apache.org/r/9258/#comment34809>

    Ok, how about the following change:
    
    settings.isTemporary = exclusive && autodelete && !arguments.find(AUTO_DELETE_TIMEOUT)
    
    The key thing are are trying to capture at  creation time is whether this is a queue that
would not survive a failover. The fact that a queue was 'declaredExclusive' is  relevant if
it is also going to be immediately autodeleted.
    
    Expanding the role of the new variable and renaming it is in my view  a clearer expression
of the code.
    
    However, I have already given my 'ship it!' approval. It is not a huge deal. I just think
it is very hard to understand why declaredExclusive is needed, and this tweak might make it
a little clearer for anyone puzzling over it in the future.
    
    If I haven't convinced you, feel free to commit!


- Gordon Sim


On Feb. 7, 2013, 6:50 p.m., Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2013, 6:50 p.m.)
> 
> 
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
> 
> 
> Description
> -------
> 
> QPID-4555: HA Add QueueSettings::declaredExclusive for exclusive queues.
> 
> This is set when the queue is created, before calling
> ConfigurationObserver::queueCreate, and does not change thereafter.
> 
> The existing Queue::owner not set until after ConfigurationObserver::queueCreate
> and does change as ownership can be released and acquired.
> 
> QPID-4555: HA Primary sets explicit qpid.replicate in Queue and Exchange arguments.
> 
> Previously both Primary and Backup would calculate the qpid.replicate value
> independently, assuming the result would be the same. In the case of exclusive
> queues, the exclusivity can change over time so its possible that primary and
> backup won't agree.
> 
> Now only Primary does the calculation with exclusive, auto-delete etc. and puts
> an explicity qpid.replicate in the queue or event arguments. Backup uses the
> value set by primary.
> 
> QPID-4555: HA Check for backup ready when new backup joins.
> 
> This test was missing so if there were no backed-up queues the backup would
> never be marked ready. It was workig because of a separte bug:
> auto-delete/exclusive queues were being replicated incorrectly so there were
> always replicated queues (temp queues created by qpid-ha)
> 
> QPID-4555: HA Don't shut down on deleting an exchange that is in use as an alternate.
> 
> Previously threw an exception in this case which shut down the broker.
> Log warning instead, this is not a fatal event.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163 
>   /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/HaBroker.h 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/Primary.h 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.h 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.cpp 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.h 1441163 
>   /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163 
>   /trunk/qpid/cpp/src/tests/ha_tests.py 1441163 
> 
> Diff: https://reviews.apache.org/r/9258/diff/
> 
> 
> Testing
> -------
> 
> make check
> ha tests running 2  hours now without failure.
> 
> 
> Thanks,
> 
> Alan Conway
> 
>


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