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-4886: Pass non-const reference to Message in QueueObserver functions.
Date Fri, 24 May 2013 20:58:05 GMT


> On May 24, 2013, 2:35 p.m., Gordon Sim wrote:
> > This makes me a little nervous, specifically around modification while the message
is being concurrently read by some other thread. At present the observers are only called
from queue while lock is held but can we rule out any other concurrent access? Having the
queue be the only place where the message can be modified (modifications elsewhere requiring
a copy) seems like a good practice.
> > 
> > With regard to annotations, the observeEnqueue() method is only called after the
message has been written to disk, so any modifications at that point would certainly not be
durable.
> 
> Gordon Sim wrote:
>     Perhaps a better solution would be to have a new callback for things outside the
queue that wish to modify the message. This can be done at specific points (before writing
to disk and before making it available to consumers) that are easier to reason about with
regard to concurrent access. What do you think? Would that work for your case?
> 
> Kenneth Giusti wrote:
>     +1 something like Gordon's suggestion - if possible.  The notion that an Observer
can actually modify in addition to simply observing isn't intuitive and overloads the original
intent more than I think it should.

Agreed, I'll post an updated approach.


- Alan


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


On May 24, 2013, 2:16 p.m., Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11378/
> -----------------------------------------------------------
> 
> (Updated May 24, 2013, 2:16 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Kenneth Giusti.
> 
> 
> Description
> -------
> 
> QPID-4886: Pass non-const reference to Message in QueueObserver functions.
> 
> The HA plugin needs to modify a message in QueueObserver::enqueued to attach a HA ID
to it. Other plugins may need to similarly annotate messages at QueueObserver call points.
> Need to remove the "const" qualifier for Message arguments to QueueObserver methods to
enable this.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.h 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1486017 
>   /trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.cpp 1486017 
>   /trunk/qpid/cpp/src/qpid/ha/QueueGuard.h 1486017 
>   /trunk/qpid/cpp/src/qpid/ha/QueueGuard.cpp 1486017 
>   /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1486017 
> 
> Diff: https://reviews.apache.org/r/11378/diff/
> 
> 
> Testing
> -------
> 
> make test
> 
> 
> Thanks,
> 
> Alan Conway
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message