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 Tue, 16 Aug 2011 09:08:13 GMT

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



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

    What's the motivation for this change?



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

    Does this get used?



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

    I don't really like making this a public method on queue. The configuration of queues
certainly needs some consolidation to make it a little more manageable. That seems to be the
primary use case here - i.e. remembering the type of queue in use.
    
    The term Disposition is also best avoided I think as in 1.0 it has a quite different meaning.



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

    I don't think match should be part of the MessageAllocator interface. I think the filter
perhaps should include a filter-type identifier which can be used to determine how the rest
of the map should be interpreted.
    
    We may in future want to use the same filter on different types of queue, or different
filters on the same type of queue.



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

    What is this used for?



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

    I wonder if removeIf() would be a better choice here? Would allow removing the messages
as we iterate.
    
    Could have the count built into the predicate or could have a max-count added to the signature
(or to an doverloaded method).


- Gordon


On 2011-08-16 00:35:20, Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1312/
> -----------------------------------------------------------
> 
> (Updated 2011-08-16 00:35:20)
> 
> 
> 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/Broker.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 
>   /branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 
>   /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144319 
>   /branches/qpid-3346/qpid/specs/management-schema.xml 1144324 
>   /branches/qpid-3346/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 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