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 2012/05/28 23:35:05 UTC

Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

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

Review request for qpid, Andrew Stitcher and Gordon Sim.


Summary
-------

The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)

How does this look? Any suggestions for better ways to do it?


Diffs
-----

  /trunk/qpid/cpp/src/Makefile.am 1343351 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1343351 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1343351 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1343351 
  /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1343351 
  /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1343351 
  /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1343351 

Diff: https://reviews.apache.org/r/5253/diff


Testing
-------

make check


Thanks,

Alan


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5253/#review8201
-----------------------------------------------------------

Ship it!


I have no real issue with this, it is no worse than what is already there (the lifecycle management is rather messy to begin with). A couple of suggestions included below, but these would be entirely optional. What I would really like would be a more wholesale cleanup of object lifecycle, unifying this sort of pattern with store, management etc. That however is certainly beyond the cope of this change.

Out of curiousity, why do you need this information in the HA plugin?


/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/5253/#comment17745>

    Though I understand why this is here, I wonder if it might not be better moved to QueueRegistry::destroy(). The justification being that removal from the QueueRegistry is the outwardly visible action (the rest is internal cleanup). Having the create and destroy communicated both by the registry would be a little cleaner in my view.



/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/5253/#comment17744>

    I think it would be a little clearer if this were moved to Broker::bind(). That corresponds more directly to the equivalent for unbind()


- Gordon


On 2012-05-28 21:35:05, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5253/
> -----------------------------------------------------------
> 
> (Updated 2012-05-28 21:35:05)
> 
> 
> Review request for qpid, Andrew Stitcher and Gordon Sim.
> 
> 
> Summary
> -------
> 
> The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
> The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)
> 
> How does this look? Any suggestions for better ways to do it?
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/Makefile.am 1343351 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1343351 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1343351 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1343351 
>   /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1343351 
>   /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1343351 
>   /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1343351 
> 
> Diff: https://reviews.apache.org/r/5253/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alan
> 
>


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Alan Conway <ac...@redhat.com>.

> On 2012-05-30 13:22:40, Gordon Sim wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 1185
> > <https://reviews.apache.org/r/5253/diff/4/?file=110786#file110786line1185>
> >
> >     Sorry for missing this last time; you could put this in ExchangeRegistry::destroy() which would again line up nicely with the create event and wouldn't as far as I can see cause any problems.

Done


- Alan


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


On 2012-05-30 13:18:17, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5253/
> -----------------------------------------------------------
> 
> (Updated 2012-05-30 13:18:17)
> 
> 
> Review request for qpid, Andrew Stitcher and Gordon Sim.
> 
> 
> Summary
> -------
> 
> The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
> The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)
> 
> How does this look? Any suggestions for better ways to do it?
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/Makefile.am 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1344052 
> 
> Diff: https://reviews.apache.org/r/5253/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alan
> 
>


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5253/#review8207
-----------------------------------------------------------

Ship it!


One further minor suggestion, but I am perfectly happy with this patch.


/trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/5253/#comment17757>

    Sorry for missing this last time; you could put this in ExchangeRegistry::destroy() which would again line up nicely with the create event and wouldn't as far as I can see cause any problems.


- Gordon


On 2012-05-30 13:18:17, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5253/
> -----------------------------------------------------------
> 
> (Updated 2012-05-30 13:18:17)
> 
> 
> Review request for qpid, Andrew Stitcher and Gordon Sim.
> 
> 
> Summary
> -------
> 
> The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
> The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)
> 
> How does this look? Any suggestions for better ways to do it?
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/Makefile.am 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1344052 
>   /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1344052 
> 
> Diff: https://reviews.apache.org/r/5253/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alan
> 
>


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5253/
-----------------------------------------------------------

(Updated 2012-05-30 13:18:17.583769)


Review request for qpid, Andrew Stitcher and Gordon Sim.


Changes
-------

Clean patch with up-to-date trunk.


Summary
-------

The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)

How does this look? Any suggestions for better ways to do it?


Diffs (updated)
-----

  /trunk/qpid/cpp/src/Makefile.am 1344052 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1344052 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1344052 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1344052 
  /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1344052 
  /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1344052 

Diff: https://reviews.apache.org/r/5253/diff


Testing
-------

make check


Thanks,

Alan


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5253/
-----------------------------------------------------------

(Updated 2012-05-30 13:11:45.840318)


Review request for qpid, Andrew Stitcher and Gordon Sim.


Changes
-------

Fix error in previous patch.


Summary
-------

The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)

How does this look? Any suggestions for better ways to do it?


Diffs (updated)
-----

  /trunk/qpid/cpp/src/Makefile.am 1343762 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1343762 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1343762 
  /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1343762 
  /trunk/qpid/doc/book/src/java-broker/Broker-Configuration-Guide.xml 1343762 
  /trunk/qpid/doc/book/src/java-broker/Producer-Flow-Control.xml 1343762 
  /trunk/qpid/doc/book/src/java-broker/Qpid-Java-FAQ.xml 1343762 
  /trunk/qpid/doc/book/src/programming/Programming-In-Apache-Qpid-Book.xml 1343762 
  /trunk/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java 1343762 
  /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java 1343762 
  /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java 1343762 
  /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreQuotaEventsTest.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java 1343762 
  /trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1343762 
  /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1343762 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java 1343762 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/QuotaMessageStore.java 1343762 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/StoreOverfullTest.java 1343762 
  /trunk/qpid/java/test-profiles/Java010Excludes 1343762 
  /trunk/qpid/java/test-profiles/JavaTransientExcludes 1343762 

Diff: https://reviews.apache.org/r/5253/diff


Testing
-------

make check


Thanks,

Alan


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Alan Conway <ac...@redhat.com>.

> On 2012-05-30 13:10:45, Gordon Sim wrote:
> > You seem to have picked up some unrelated changes outside the 'cpp' tree(?)

No, but I was working with an out-of-date trunk which seems to have confused post-review Clean patch to follow.


- Alan


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


On 2012-05-30 13:11:45, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5253/
> -----------------------------------------------------------
> 
> (Updated 2012-05-30 13:11:45)
> 
> 
> Review request for qpid, Andrew Stitcher and Gordon Sim.
> 
> 
> Summary
> -------
> 
> The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
> The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)
> 
> How does this look? Any suggestions for better ways to do it?
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/Makefile.am 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1343762 
>   /trunk/qpid/doc/book/src/java-broker/Broker-Configuration-Guide.xml 1343762 
>   /trunk/qpid/doc/book/src/java-broker/Producer-Flow-Control.xml 1343762 
>   /trunk/qpid/doc/book/src/java-broker/Qpid-Java-FAQ.xml 1343762 
>   /trunk/qpid/doc/book/src/programming/Programming-In-Apache-Qpid-Book.xml 1343762 
>   /trunk/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java 1343762 
>   /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java 1343762 
>   /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java 1343762 
>   /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreQuotaEventsTest.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java 1343762 
>   /trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1343762 
>   /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1343762 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java 1343762 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/QuotaMessageStore.java 1343762 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/StoreOverfullTest.java 1343762 
>   /trunk/qpid/java/test-profiles/Java010Excludes 1343762 
>   /trunk/qpid/java/test-profiles/JavaTransientExcludes 1343762 
> 
> Diff: https://reviews.apache.org/r/5253/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alan
> 
>


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5253/#review8205
-----------------------------------------------------------


You seem to have picked up some unrelated changes outside the 'cpp' tree(?)

- Gordon


On 2012-05-30 13:07:28, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5253/
> -----------------------------------------------------------
> 
> (Updated 2012-05-30 13:07:28)
> 
> 
> Review request for qpid, Andrew Stitcher and Gordon Sim.
> 
> 
> Summary
> -------
> 
> The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
> The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)
> 
> How does this look? Any suggestions for better ways to do it?
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/Makefile.am 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1343762 
>   /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1343762 
>   /trunk/qpid/doc/book/src/java-broker/Broker-Configuration-Guide.xml 1343762 
>   /trunk/qpid/doc/book/src/java-broker/Producer-Flow-Control.xml 1343762 
>   /trunk/qpid/doc/book/src/java-broker/Qpid-Java-FAQ.xml 1343762 
>   /trunk/qpid/doc/book/src/programming/Programming-In-Apache-Qpid-Book.xml 1343762 
>   /trunk/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java 1343762 
>   /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java 1343762 
>   /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java 1343762 
>   /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreQuotaEventsTest.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1343762 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java 1343762 
>   /trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1343762 
>   /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1343762 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java 1343762 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/QuotaMessageStore.java 1343762 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/StoreOverfullTest.java 1343762 
>   /trunk/qpid/java/test-profiles/Java010Excludes 1343762 
>   /trunk/qpid/java/test-profiles/JavaTransientExcludes 1343762 
> 
> Diff: https://reviews.apache.org/r/5253/diff
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alan
> 
>


Re: Review Request: ConfigurationObserver: allow plugins to observe configuration (aka wiring) changes.

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5253/
-----------------------------------------------------------

(Updated 2012-05-30 13:07:28.144149)


Review request for qpid, Andrew Stitcher and Gordon Sim.


Changes
-------

Incorporated Gordan's suggestions


Summary
-------

The HA plugin needs to know when queues are created. This patch creates a ConfigurationObserver to observe queue and exchange create/delete and bind/unbind.
The MessageStore API has these hooks but you can't have multiple MessageStores on a broker, also calls to the MessageStore interface are conditional on durability which is not what I want for HA. The ConfigurationObserver calls are placed close to the corresponding MessageStore calls, except that the create() calls are placed earlier - just before the queue/exchange is stored in the Registry. That allows an Observer to effectively abort the creation of the queue/exchange (HA doesn't actually need this but it seemed like it might be useful)

How does this look? Any suggestions for better ways to do it?


Diffs (updated)
-----

  /trunk/qpid/cpp/src/Makefile.am 1343762 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1343762 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObserver.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConfigurationObservers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1343762 
  /trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/broker/Observers.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/ha/Backup.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.cpp 1343762 
  /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1343762 
  /trunk/qpid/doc/book/src/java-broker/Broker-Configuration-Guide.xml 1343762 
  /trunk/qpid/doc/book/src/java-broker/Producer-Flow-Control.xml 1343762 
  /trunk/qpid/doc/book/src/java-broker/Qpid-Java-FAQ.xml 1343762 
  /trunk/qpid/doc/book/src/programming/Programming-In-Apache-Qpid-Book.xml 1343762 
  /trunk/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java 1343762 
  /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java 1343762 
  /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java 1343762 
  /trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreQuotaEventsTest.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java 1343762 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java 1343762 
  /trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java 1343762 
  /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1343762 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java 1343762 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/QuotaMessageStore.java 1343762 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/server/store/StoreOverfullTest.java 1343762 
  /trunk/qpid/java/test-profiles/Java010Excludes 1343762 
  /trunk/qpid/java/test-profiles/JavaTransientExcludes 1343762 

Diff: https://reviews.apache.org/r/5253/diff


Testing
-------

make check


Thanks,

Alan