You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2013/02/01 20:37:45 UTC
Review Request: QPID-4555: HA Add declaredExclusive management property to
Queues
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/
-----------------------------------------------------------
Review request for qpid, Gordon Sim and Ted Ross.
Description
-------
QPID-4555: HA Add declaredExclusive management property to Queues
The "exclusive" management property can change over the lifetime of a queue.
This creates race conditions where primary and backup disagree over whether a
queue is replicated. To resolve this, added a new management property
"declaredExclusive" which is set when the queue is created and not modified.
Do we need to co-ordinate with the Java build over this change to the schema?
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
/trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
/trunk/qpid/cpp/src/tests/ha_tests.py 1441163
/trunk/qpid/specs/management-schema.xml 1441163
Diff: https://reviews.apache.org/r/9258/diff/
Testing
-------
Thanks,
Alan Conway
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Alan Conway <ac...@redhat.com>.
> On Feb. 5, 2013, 4:53 p.m., Gordon Sim wrote:
> > I'd be tempted to be more direct about this and if we are adding a new management property I would have it indicate whether or not the queue is replicatable. I.e it would be set false if autodelete && exclusive && !args.isSet(AUTO_DELETE_TIMEOUT) and true otherwise. This seems to result in a more understandable schema (the point of the declaredExclusive is a lot less obvious).
Yes this was an option I considered. I think you're right that it is better that way. Will code it up...
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16116
-----------------------------------------------------------
On Feb. 5, 2013, 3:33 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 3:33 p.m.)
>
>
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
>
>
> Description
> -------
>
> QPID-4555: HA Add declaredExclusive management property to Queues
>
> The "exclusive" management property can change over the lifetime of a queue.
> This creates race conditions where primary and backup disagree over whether a
> queue is replicated. To resolve this, added a new management property
> "declaredExclusive" which is set when the queue is created and not modified.
>
> Do we need to co-ordinate with the Java build over this change to the schema?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
> /trunk/qpid/cpp/src/tests/ha_tests.py 1441163
> /trunk/qpid/specs/management-schema.xml 1441163
>
> Diff: https://reviews.apache.org/r/9258/diff/
>
>
> Testing
> -------
>
> make check
> ha tests running 2 hours now without failure.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Kenneth Giusti <kg...@apache.org>.
> On Feb. 5, 2013, 4:53 p.m., Gordon Sim wrote:
> > I'd be tempted to be more direct about this and if we are adding a new management property I would have it indicate whether or not the queue is replicatable. I.e it would be set false if autodelete && exclusive && !args.isSet(AUTO_DELETE_TIMEOUT) and true otherwise. This seems to result in a more understandable schema (the point of the declaredExclusive is a lot less obvious).
>
> Alan Conway wrote:
> Yes this was an option I considered. I think you're right that it is better that way. Will code it up...
>
I agree with Gordon here - my initial reaction to the addition of a new "declaredExclusive" was 'huh'? I understand now that the 'exclusive' that is published can change, but that's not immediately apparent so the whole naming seems redundant and confusing to read. Since the new flag is visible via mgmt, I'd prefer to see it given a name that is more reflective of it's intended use.
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16116
-----------------------------------------------------------
On Feb. 5, 2013, 3:33 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 3:33 p.m.)
>
>
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
>
>
> Description
> -------
>
> QPID-4555: HA Add declaredExclusive management property to Queues
>
> The "exclusive" management property can change over the lifetime of a queue.
> This creates race conditions where primary and backup disagree over whether a
> queue is replicated. To resolve this, added a new management property
> "declaredExclusive" which is set when the queue is created and not modified.
>
> Do we need to co-ordinate with the Java build over this change to the schema?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
> /trunk/qpid/cpp/src/tests/ha_tests.py 1441163
> /trunk/qpid/specs/management-schema.xml 1441163
>
> Diff: https://reviews.apache.org/r/9258/diff/
>
>
> Testing
> -------
>
> make check
> ha tests running 2 hours now without failure.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16116
-----------------------------------------------------------
I'd be tempted to be more direct about this and if we are adding a new management property I would have it indicate whether or not the queue is replicatable. I.e it would be set false if autodelete && exclusive && !args.isSet(AUTO_DELETE_TIMEOUT) and true otherwise. This seems to result in a more understandable schema (the point of the declaredExclusive is a lot less obvious).
- Gordon Sim
On Feb. 5, 2013, 3:33 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 3:33 p.m.)
>
>
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
>
>
> Description
> -------
>
> QPID-4555: HA Add declaredExclusive management property to Queues
>
> The "exclusive" management property can change over the lifetime of a queue.
> This creates race conditions where primary and backup disagree over whether a
> queue is replicated. To resolve this, added a new management property
> "declaredExclusive" which is set when the queue is created and not modified.
>
> Do we need to co-ordinate with the Java build over this change to the schema?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
> /trunk/qpid/cpp/src/tests/ha_tests.py 1441163
> /trunk/qpid/specs/management-schema.xml 1441163
>
> Diff: https://reviews.apache.org/r/9258/diff/
>
>
> Testing
> -------
>
> make check
> ha tests running 2 hours now without failure.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Gordon Sim <gs...@redhat.com>.
> On Feb. 5, 2013, 3:42 p.m., Ted Ross wrote:
> > This looks ok to me.
> >
> > A couple of thoughts:
> >
> > 1) Java broker... Before you commit, run a build of the Java broker. It will probably fail in the module that handles management requests. It's fairly easy to figure out how to add a placeholder for a new attribute.
> >
> > 2) declaredExclusive in the queue-declare event. Is this not redundant with the "exclusive" attribute since this event is only generated upon declaration of the queue?
>
> Alan Conway wrote:
> 2 - its not equivalent, that's the origin of the problem. "exclusive" can change over the lifetime of the queue if ownership is released or re-acquired.
btw QMF support was removed from the java broker so you no longer need to worry about breaking it
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16112
-----------------------------------------------------------
On Feb. 5, 2013, 3:33 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 3:33 p.m.)
>
>
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
>
>
> Description
> -------
>
> QPID-4555: HA Add declaredExclusive management property to Queues
>
> The "exclusive" management property can change over the lifetime of a queue.
> This creates race conditions where primary and backup disagree over whether a
> queue is replicated. To resolve this, added a new management property
> "declaredExclusive" which is set when the queue is created and not modified.
>
> Do we need to co-ordinate with the Java build over this change to the schema?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
> /trunk/qpid/cpp/src/tests/ha_tests.py 1441163
> /trunk/qpid/specs/management-schema.xml 1441163
>
> Diff: https://reviews.apache.org/r/9258/diff/
>
>
> Testing
> -------
>
> make check
> ha tests running 2 hours now without failure.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Alan Conway <ac...@redhat.com>.
> On Feb. 5, 2013, 3:42 p.m., Ted Ross wrote:
> > This looks ok to me.
> >
> > A couple of thoughts:
> >
> > 1) Java broker... Before you commit, run a build of the Java broker. It will probably fail in the module that handles management requests. It's fairly easy to figure out how to add a placeholder for a new attribute.
> >
> > 2) declaredExclusive in the queue-declare event. Is this not redundant with the "exclusive" attribute since this event is only generated upon declaration of the queue?
2 - its not equivalent, that's the origin of the problem. "exclusive" can change over the lifetime of the queue if ownership is released or re-acquired.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16112
-----------------------------------------------------------
On Feb. 5, 2013, 3:33 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 3:33 p.m.)
>
>
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
>
>
> Description
> -------
>
> QPID-4555: HA Add declaredExclusive management property to Queues
>
> The "exclusive" management property can change over the lifetime of a queue.
> This creates race conditions where primary and backup disagree over whether a
> queue is replicated. To resolve this, added a new management property
> "declaredExclusive" which is set when the queue is created and not modified.
>
> Do we need to co-ordinate with the Java build over this change to the schema?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
> /trunk/qpid/cpp/src/tests/ha_tests.py 1441163
> /trunk/qpid/specs/management-schema.xml 1441163
>
> Diff: https://reviews.apache.org/r/9258/diff/
>
>
> Testing
> -------
>
> make check
> ha tests running 2 hours now without failure.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Ted Ross <tr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16112
-----------------------------------------------------------
This looks ok to me.
A couple of thoughts:
1) Java broker... Before you commit, run a build of the Java broker. It will probably fail in the module that handles management requests. It's fairly easy to figure out how to add a placeholder for a new attribute.
2) declaredExclusive in the queue-declare event. Is this not redundant with the "exclusive" attribute since this event is only generated upon declaration of the queue?
- Ted Ross
On Feb. 5, 2013, 3:33 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9258/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 3:33 p.m.)
>
>
> Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
>
>
> Description
> -------
>
> QPID-4555: HA Add declaredExclusive management property to Queues
>
> The "exclusive" management property can change over the lifetime of a queue.
> This creates race conditions where primary and backup disagree over whether a
> queue is replicated. To resolve this, added a new management property
> "declaredExclusive" which is set when the queue is created and not modified.
>
> Do we need to co-ordinate with the Java build over this change to the schema?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
> /trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
> /trunk/qpid/cpp/src/tests/ha_tests.py 1441163
> /trunk/qpid/specs/management-schema.xml 1441163
>
> Diff: https://reviews.apache.org/r/9258/diff/
>
>
> Testing
> -------
>
> make check
> ha tests running 2 hours now without failure.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Alan Conway <ac...@redhat.com>.
> On Feb. 7, 2013, 6:59 p.m., Gordon Sim wrote:
> > I can certainly live with this. I think I would have made QueueSettings::declaredExclusive be QueueSettings::replicate, again just to be very clear about what it means. However its slightly less an issue when its not actually exposed directly in the schema.
"declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate declared-exclusive+auto-delete+auto-delete-delay==0.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16284
-----------------------------------------------------------
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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Alan Conway <ac...@redhat.com>.
> On Feb. 7, 2013, 6:59 p.m., Gordon Sim wrote:
> > I can certainly live with this. I think I would have made QueueSettings::declaredExclusive be QueueSettings::replicate, again just to be very clear about what it means. However its slightly less an issue when its not actually exposed directly in the schema.
>
> Alan Conway wrote:
> "declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate declared-exclusive+auto-delete+auto-delete-delay==0.
"declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate if declared-exclusive && auto-delete && auto-delete-delay==0 && no-explicit-replicate-argument.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16284
-----------------------------------------------------------
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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Alan Conway <ac...@redhat.com>.
> On Feb. 7, 2013, 6:59 p.m., Gordon Sim wrote:
> > I can certainly live with this. I think I would have made QueueSettings::declaredExclusive be QueueSettings::replicate, again just to be very clear about what it means. However its slightly less an issue when its not actually exposed directly in the schema.
>
> Alan Conway wrote:
> "declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate declared-exclusive+auto-delete+auto-delete-delay==0.
>
> Alan Conway wrote:
> "declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate if declared-exclusive && auto-delete && auto-delete-delay==0 && no-explicit-replicate-argument.
>
> Gordon Sim wrote:
> I understand they are not equivalent, but I think you could as well record the relicate decision as the fact that the queue was declared exclusive. It seems like a more obvious and interesting data point.
I don't follow. The replicate decision is not made at the point when declaredExclusive is set. It is made later using declaredExclusive and other factors - the reason replicateExclusive is set is to make that information available at ConfigurationObserver::queueCreate. The decision is then recorded by adding qpid.replicate to the queues arguments.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16284
-----------------------------------------------------------
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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Gordon Sim <gs...@redhat.com>.
> On Feb. 7, 2013, 6:59 p.m., Gordon Sim wrote:
> > I can certainly live with this. I think I would have made QueueSettings::declaredExclusive be QueueSettings::replicate, again just to be very clear about what it means. However its slightly less an issue when its not actually exposed directly in the schema.
>
> Alan Conway wrote:
> "declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate declared-exclusive+auto-delete+auto-delete-delay==0.
>
> Alan Conway wrote:
> "declared exclusive" means that the queue was declared to be exclusive. It does not mean "replicate". Whether to replicate depends on other factors such as qpid.replicate arguments in the declare, auto-delete, auto-delete-delay and the default replicate mode. We don't replicate if declared-exclusive && auto-delete && auto-delete-delay==0 && no-explicit-replicate-argument.
I understand they are not equivalent, but I think you could as well record the relicate decision as the fact that the queue was declared exclusive. It seems like a more obvious and interesting data point.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16284
-----------------------------------------------------------
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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/#review16284
-----------------------------------------------------------
Ship it!
I can certainly live with this. I think I would have made QueueSettings::declaredExclusive be QueueSettings::replicate, again just to be very clear about what it means. However its slightly less an issue when its not actually exposed directly in the schema.
- 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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Alan Conway <ac...@redhat.com>.
> 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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: QPID-4555: HA Add QueueSettings::declaredExclusive for
exclusive queues.
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
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.
Changes
-------
This is a new tack: instead of relying the same calculation yielding the same result (where have I heard that before?) The primary now decides whether a queue is replicated or not, and adds an explicit qpid.replicate to the arguments. The backup just does what it says in qpid.replicate. I added QueueSettings::declaredExclusive to provide a reliable indication, set before ConfigurationObserver::queueCreate. It had some ripples but I think it's healthier now. If you want you can just focus on the changes I made under broker to make sure I don't break something.
Summary (updated)
-----------------
QPID-4555: HA Add QueueSettings::declaredExclusive for exclusive queues.
Description (updated)
-------
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 (updated)
-----
/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
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/
-----------------------------------------------------------
(Updated Feb. 5, 2013, 3:33 p.m.)
Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross.
Changes
-------
Added Ken as reviewer
Description
-------
QPID-4555: HA Add declaredExclusive management property to Queues
The "exclusive" management property can change over the lifetime of a queue.
This creates race conditions where primary and backup disagree over whether a
queue is replicated. To resolve this, added a new management property
"declaredExclusive" which is set when the queue is created and not modified.
Do we need to co-ordinate with the Java build over this change to the schema?
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
/trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
/trunk/qpid/cpp/src/tests/ha_tests.py 1441163
/trunk/qpid/specs/management-schema.xml 1441163
Diff: https://reviews.apache.org/r/9258/diff/
Testing
-------
make check
ha tests running 2 hours now without failure.
Thanks,
Alan Conway
Re: Review Request: QPID-4555: HA Add declaredExclusive management property
to Queues
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9258/
-----------------------------------------------------------
(Updated Feb. 1, 2013, 10:11 p.m.)
Review request for qpid, Gordon Sim and Ted Ross.
Description
-------
QPID-4555: HA Add declaredExclusive management property to Queues
The "exclusive" management property can change over the lifetime of a queue.
This creates race conditions where primary and backup disagree over whether a
queue is replicated. To resolve this, added a new management property
"declaredExclusive" which is set when the queue is created and not modified.
Do we need to co-ordinate with the Java build over this change to the schema?
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1441163
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1441163
/trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1441163
/trunk/qpid/cpp/src/qpid/ha/ReplicationTest.cpp 1441163
/trunk/qpid/cpp/src/tests/ha_tests.py 1441163
/trunk/qpid/specs/management-schema.xml 1441163
Diff: https://reviews.apache.org/r/9258/diff/
Testing (updated)
-------
make check
ha tests running 2 hours now without failure.
Thanks,
Alan Conway