> On May 27, 2014, 8:40 a.m., Gordon Sim wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Message.h, line 117
> > <https://reviews.apache.org/r/21153/diff/2/?file=594322#file594322line117>
> >
> > Rather than have a hasExpired() method that takes the tie to compare
> > against, it might be simpler to remove this method entirely and just use
> > getExpiration() which can then be compared against whatever value is
> > desired.
>
> Pavel Moravec wrote:
> hasExpired(Message, AbsTime) is required by Queue::purgeExpired:
>
> uint32_t count = remove(0, boost::bind(&Message::hasExpired, _1, time),
> 0, CONSUMER, settings.autodelete);
>
> Or is there a way how to boost::bind getExpiration() method there?
>
> The implicit setting time = sys::AbsTime::now() can be removed as that is
> used only in Queue::getNextMessage, but how to deal with the bind?
The second argument to remove() is any function/functor. So e.g. you could
define
namespace{
bool hasExpired(const Message* m, AbsTime now)
{
return m->getExpiration() < now;
}
}
and then change the call to:
uint32_t count = remove(0, boost::bind(&hasExpired, _1, time), 0, CONSUMER,
settings.autodelete);
The advantage here is that it isn't part of the Message interface, and is
defined closer to where it is actually used.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21153/#review43955
-----------------------------------------------------------
On May 26, 2014, 12:14 p.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21153/
> -----------------------------------------------------------
>
> (Updated May 26, 2014, 12:14 p.m.)
>
>
> Review request for qpid, Gordon Sim and mick goulish.
>
>
> Bugs: QPID-5748
> https://issues.apache.org/jira/browse/QPID-5748
>
>
> Repository: qpid
>
>
> Description
> -------
>
> Queue::purgeExpired method (to remove messages from all queues with TTL
> expired) currently calls Message::hasExpired() method that calls
> AbsTime::now() i.e. ::clock_gettime for every individual message with TTL set.
>
> That system call is redundant to be called in that way. It is enough to call
> it once before traversing the very first queue and use the value as an
> argument.
>
> The patch is not ideal: I would rather see just one function like:
>
> bool ExpiryPolicy::hasExpired(const Message& m, qpid::sys::AbsTime time = 0)
> {
> if time==0 time=sys::AbsTime::now();
> return m.getExpiration() < sys::AbsTime::now();
> }
>
> but not sure what value from AbsTime to be used instead of 0 (note that such
> value should be get in as short time as possible). And I havent played with
> AbsTime class and dont have time now to do so :-/
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/CMakeLists.txt 1597555
> /trunk/qpid/cpp/src/qpid/broker/Broker.h 1597555
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/Message.h 1597555
> /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/PagedQueue.h 1597555
> /trunk/qpid/cpp/src/qpid/broker/PagedQueue.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/QueueFactory.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/RecoverableMessage.h 1597555
> /trunk/qpid/cpp/src/qpid/broker/RecoverableMessageImpl.h 1597555
> /trunk/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1597555
> /trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.h 1597555
> /trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1597555
> /trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.cpp 1597555
> /trunk/qpid/cpp/src/qpid/linearstore/MessageStoreImpl.cpp 1597555
> /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1597555
> /trunk/qpid/cpp/src/tests/MessageUtils.h 1597555
> /trunk/qpid/cpp/src/tests/QueueTest.cpp 1597555
>
> Diff: https://reviews.apache.org/r/21153/diff/
>
>
> Testing
> -------
>
> Attached patch improved:
> - _traversing_ queues and _checking_ messages with TTL by approx. 10%
> - with purging _durable_ expired messages, performance improvement is just 1%
> (still better than nothing)
>
> No automated tests run so far (lack of time+HW).
>
>
> Thanks,
>
> Pavel Moravec
>
>
