qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Stitcher" <astitc...@apache.org>
Subject Re: Review Request: QPID-3280: When sending a large number of messages with nonzero TTLs to a cluster, overall message throughput drops by around 20-30% compared to messages with TTL 0.
Date Fri, 27 May 2011 18:53:41 GMT


> On 2011-05-27 16:59:51, Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/qpid/sys/Timer.cpp, line 65
> > <https://reviews.apache.org/r/791/diff/1/?file=19676#file19676line65>
> >
> >     As my previous comment I think this test should now be:
> >     
> >       if (period && fired) {
> >     
> >     And as this seems to be the only user of readyToFire() it could be removed
> >     
> >     However I think this might be the wrong behaviour in any case.
> >     
> >     If a periodic event is preempted by the elder why isn't the elder going to preempt
the next firing too. In other words why are we setting up a next firing on the local timer
when the elders timer is going to do the work for us?
> 
> Andy Goldstein wrote:
>     if (period && fired) seems cleaner to me too.
>     
>     Presumably the elder will preempt the next firing too, but the task (in this case,
QueueCleaner), isn't aware that it was run in a non-elder was it?  The general pattern for
periodic tasks seems to be
>     
>     1) add to timer
>     2) timer fires task
>     3) as part of task's fire() method, call setupNextFire() and then add the task to
the timer to reschedule it
>     
>     Is there some way to avoid step 3) if you're not the elder?  Does this make sense?

I see. I think it probably means that the TimerTask class needs to have a clustered counterpart
as well as Timer so that when the a TimerTask is added to a ClusterTimer the ClusterTimer
transparently creates a ClusterTimerTask wrapping a TimerTask and overrides TimerTasks member
functions to do "the right thing".

This seems like a design issue in the ClusterTimer rather than this fix specifically - Alan
what do you think?


- Andrew


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


On 2011-05-26 20:53:00, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/791/
> -----------------------------------------------------------
> 
> (Updated 2011-05-26 20:53:00)
> 
> 
> Review request for qpid, Andrew Stitcher, Alan Conway, and Gordon Sim.
> 
> 
> Summary
> -------
> 
> QPID-3280: When sending a large number of messages with nonzero TTLs to a cluster, overall
message throughput drops by around 20-30% compared to messages with TTL 0.
> 
> Replaced the complicated message expirly logic in the cluster with a simpler "cluster
clock" for expiry of messages with TTL.
> 
> Patch supplied by Andy Goldstein <agoldste@redhat.com>.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/broker/ExpiryPolicy.h 1128002 
>   /trunk/qpid/cpp/src/qpid/broker/ExpiryPolicy.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/broker/QueueCleaner.h 1128002 
>   /trunk/qpid/cpp/src/qpid/broker/QueueCleaner.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/Cluster.h 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/ClusterPlugin.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/ClusterSettings.h 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/Connection.h 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/ExpiryPolicy.h 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/ExpiryPolicy.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1128002 
>   /trunk/qpid/cpp/src/qpid/sys/Timer.h 1128002 
>   /trunk/qpid/cpp/src/qpid/sys/Timer.cpp 1128002 
>   /trunk/qpid/cpp/xml/cluster.xml 1128002 
>   /trunk/qpid/python/examples/api/spout 1128002 
> 
> Diff: https://reviews.apache.org/r/791/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alan
> 
>


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