> On 2010-11-18 09:42:59, Gordon Sim wrote:
> > Is the additional locking part of this specific issue? i.e. is it required 
> > specifically by the move to moving from 1-1 tp 1-n? Patch looks fine to me, 
> > just curious on that point.

Not related to 1-n, but there was a slightly different assertion seen before 
adding the locking. Also by inspection it's clear that the locks should be 
there.


- Alan


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


On 2010-11-17 12:27:51, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/107/
> -----------------------------------------------------------
> 
> (Updated 2010-11-17 12:27:51)
> 
> 
> Review request for qpid.
> 
> 
> Summary
> -------
> 
> 
>    QPID-2874 Clustered broker crashes in assertion in cluster/ExpiryPolicy.cpp
> 
>     - Added missing lock to ExpiryPolicy
>     - 1-N mapping for expiry ID to mapping when receiving an update.
>     - Regression test.
> 
>     A fan-out message (sent to multiple queues e.g. by fanout or topic
>     exchange) is a single message on multiple queues with a single expiry
>     ID. During an update however each instance is sent as a separate
>     message so we need to allow 1-N mapping of expiry ID to message during
>     update.
> 
> The problem is to do with messages that are fanned-out to multiple queues.
> The cluster update process does not recognize the same message on different
> queues and updates as if it were two distinct messages. The cluster expiry 
> code
> expects a 1-1 correspondence between messages and expiry-ids, which are
> assigned per message, not per-queued-message. 
> 
> This patch allows a 1-n correspondence between expiry-id and messages 
> received via update, and ensures that all messages with the same expiry-id 
> are expired at the same time.
> 
> Arguably it would be better to resolve the underlying problem - have update 
> recognize the same message on different queues and send the actual message 
> only once and inform the updatee of the queues it is on. That would be a more 
> involved fix, and would be redundant if we implement some form the new 
> cluster design.  This patch fix solves the immediate problem for the shorter 
> term. 
> 
> 
> This addresses bug qpid-2874.
>     https://issues.apache.org/jira/browse/qpid-2874
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/cluster/ExpiryPolicy.h 1035880 
>   trunk/qpid/cpp/src/qpid/cluster/ExpiryPolicy.cpp 1035880 
>   trunk/qpid/cpp/src/tests/cluster_tests.py 1035880 
>   trunk/qpid/python/qpid/brokertest.py 1035880 
> 
> Diff: https://reviews.apache.org/r/107/diff
> 
> 
> Testing
> -------
> 
> Passing the reproducers, make check and make check-long. See qpid-2874
> 
> 
> Thanks,
> 
> Alan
> 
>

Reply via email to