qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request: Prototype of infrastructure changes needed to support message groups [PRELIMINARY]
Date Mon, 08 Aug 2011 10:11:17 GMT

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



/branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp
<https://reviews.apache.org/r/1312/#comment2990>

    Not too keen on this lookup. Can it be avoided?
    
    E.g. can we modify the Queue::acquire() to simply take the consumer name as the second
parameter? (That is for the present at least all that is required)
    
    Alternatively the DeliveryRecord could be modified to hold a pointer to the Consumer rather
than simply the tag.



/branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp
<https://reviews.apache.org/r/1312/#comment2991>

    The purpose of the check is to ensure that an acquire attempt for a message that has since
been replaced, does not acquire the message that has replaced it instead.
    
    I believe it is still necessary, though I concede the form is ugly and unintuitive.



/branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/1312/#comment2992>

    I'm not keen on the terminology here. For me selector implies something subtly different
from the role this object is serving (at least from the role I *think* it is serving).
    
    I'd prefer something like 'allocator'.



/branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/1312/#comment2993>

    Again, I'm not too keen on the term 'consumed' in this context. Though I can see how it
is justified, it is potentially confusing in my view (could imply the actual dequeue of a
message).
    
    I'd prefer 'acquired', 'allocated' or even just 'locked' as they are all less ambiguous
on the state in question.


- Gordon


On 2011-08-05 20:46:15, Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1312/
> -----------------------------------------------------------
> 
> (Updated 2011-08-05 20:46:15)
> 
> 
> Review request for qpid.
> 
> 
> Summary
> -------
> 
> Some initial refactoring of Queue/Consumer interface to allow for message grouping support.
 Very preliminary.
> 
> 
> This addresses bug qpid-3346.
>     https://issues.apache.org/jira/browse/qpid-3346
> 
> 
> Diffs
> -----
> 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144324 
> 
> Diff: https://reviews.apache.org/r/1312/diff
> 
> 
> Testing
> -------
> 
> minimal - passes unit tests on linux.
> 
> 
> Thanks,
> 
> Kenneth
> 
>


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