> On 2011-05-31 15:35:19, Andy Goldstein wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Message.cpp, line 374
> > <https://reviews.apache.org/r/791/diff/3/?file=20023#file20023line374>
> >
> >     I had put a TODO/question in my original patch asking if this time 
> > needed to be based on the cluster time, but I didn't change the code here 
> > (talking only about props->setExpiration) because it didn't look like the 
> > broker/cluster used the expiration value from the message 
> > properties...should this be based on the cluster time too, or does it not 
> > matter?

It doesn't really matter in that the expiration property is only used by the 
client receiving the message. It may be more accurate to use the real time. 
I'll add a comment to the code explaining why we don't use cluster time here.


> On 2011-05-31 15:35:19, Andy Goldstein wrote:
> > /trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp, line 1187
> > <https://reviews.apache.org/r/791/diff/3/?file=20027#file20027line1187>
> >
> >     maybe add a comment that this method is called as part of the initial 
> > state transfer when joining a cluster

done


> On 2011-05-31 15:35:19, Andy Goldstein wrote:
> > /trunk/qpid/python/examples/api/spout, line 50
> > <https://reviews.apache.org/r/791/diff/3/?file=20040#file20040line50>
> >
> >     I added this to help me out w/testing - there is already an open JIRA 
> > (QPID-2890) to add support for TTL (among other things) in spout, and the 
> > patch proposes -L for TTL, so this would conflict.  Probably want to 
> > coordinate or just drop this from my patch and pull in the patch for 2890 
> > separately

Dropped. BTW there are a pair of c++ clients: qpid-send and qpid-receive that 
have TTL support.

I am adding a test for this, once that's done I'll post a new patch.


- Alan


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


On 2011-05-31 21:37:57, Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/791/
> -----------------------------------------------------------
> 
> (Updated 2011-05-31 21:37:57)
> 
> 
> Review request for qpid, Andrew Stitcher, Alan Conway, Gordon Sim, and Andy 
> Goldstein.
> 
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/broker/ExpiryPolicy.h 1128070 
>   /trunk/qpid/cpp/src/qpid/broker/ExpiryPolicy.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/broker/QueueCleaner.h 1128070 
>   /trunk/qpid/cpp/src/qpid/broker/QueueCleaner.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/Cluster.h 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/ClusterPlugin.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/ClusterSettings.h 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/ClusterTimer.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/Connection.h 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/ExpiryPolicy.h 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/ExpiryPolicy.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1128070 
>   /trunk/qpid/cpp/src/qpid/sys/Timer.h 1128070 
>   /trunk/qpid/cpp/src/qpid/sys/Timer.cpp 1128070 
>   /trunk/qpid/cpp/src/tests/QueueTest.cpp 1128070 
>   /trunk/qpid/cpp/src/tests/cluster_tests.py 1128070 
>   /trunk/qpid/cpp/xml/cluster.xml 1128070 
> 
> Diff: https://reviews.apache.org/r/791/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alan
> 
>

Reply via email to