> 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 > >
