[jira] Commented: (QPID-2341) Annotate replicated broker classes with assertions.

["Andrew Stitcher (JIRA)" <qp...@incubator.apache.org>]

    [ https://issues.apache.org/jira/browse/QPID-2341?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12801789#action_12801789 ] 

Andrew Stitcher commented on QPID-2341:
---------------------------------------

If the proposal is to actively mark only state that _is_ replication safe then I think this is a great overall plan (I think having to do the opposite - check/mark non safe state - would be a disaster causing as many problems as it would solve).

I would be a little concerned though if this was a really intrusive/widespread change though.

On a subsiduary note - how is someone to know if a piece of state they just added is replication safe?

> Annotate replicated broker classes with assertions.
> ---------------------------------------------------
>
>                 Key: QPID-2341
>                 URL: https://issues.apache.org/jira/browse/QPID-2341
>             Project: Qpid
>          Issue Type: Improvement
>          Components: C++ Broker
>    Affects Versions: 0.6
>            Reporter: Alan Conway
>            Assignee: Alan Conway
>
> A clustered broker maintains consistency of replicated objects by only modifying them in a "replication safe" thread context: while receiving an update or dispatching cluster events.
> A repeated source of cluster bugs is broker code that unwittingly modifies replicated objects in an unsafe context such as a timer thread. These bugs are intermittent race conditions that are hard to track down.
> Proposal: annotate broker code with assertions to identify code that modifies replicated state and log/abort if such code is called in an unsafe context:
> // New class:
> namespace broker {
> class Replicated {
>  protected: 
>    void assertReplicationSafe();
> }
> // Existing classes
> class Queue : public Replicated { // Mark Queue as state that may be replicated.
>   void someQueueModifier() {
>     assertReplicationSafe(); // This function should only be called in replication-safe context.
>  }
> The assertion is cheap: just testing a thread-local boolean value. In a non-clustered broker it does nothing.
> This technique has already proven valuable in debugging a recent bug, putting the assertions permanently in the code should speed debugging of future bugs.
> This would be the beginning of a formal contract between the broker code and the cluster that should make things more maintainable in the long run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org