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: QPID-4555: HA Add QueueSettings::declaredExclusive for exclusive queues.
Date Fri, 08 Feb 2013 19:35:18 GMT


> On Feb. 8, 2013, 9:35 a.m., Gordon Sim wrote:
> > /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp, line 297
> > <https://reviews.apache.org/r/9258/diff/2/?file=256761#file256761line297>
> >
> >     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!

I'm convinced, done r1444200


- Alan


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


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