> 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?
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? - Andy ----------------------------------------------------------- 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 <[email protected]>. > > > 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 > >
