qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Moravec" <pmora...@redhat.com>
Subject Re: Review Request 19566: QPID-5642: Message sequence numbers (enabled with qpid.msg_sequence) should be persistent across broker restarts
Date Tue, 01 Apr 2014 10:45:02 GMT

This is an automatically generated e-mail. To reply, visit:

(Updated April 1, 2014, 10:45 a.m.)

Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.

Bugs: QPID-5642

Repository: qpid


Elegant (but not performance optimal) way of patch:

1) In Exchange::PreRoute::PreRoute, update exchange in store (whole entry).
2) The update method is dealed very similarly like MessageStoreImpl::create(db_ptr db, IdSequence&
seq, const qpid::broker::Persistable& p) method, i.e. calls BufferValue that calls Exchange::encode.

Here the code can be unified by merging MessageStoreImpl::update intoMessageStoreImpl::create
method where the code almost duplicates.

However I do not see the patch as performance efficient, as with every message preroute, new
qpid::framing::Buffer is filled in Exchange::encode method, data are copied from it to char*
BufferValue::data and even then they are really written to the BDB. While in fact we just
update the only one number in the Buffer.

I tried to come up with less performance invasive approach (for those interested, see https://bugzilla.redhat.com/attachment.cgi?id=877576&action=diff
- if you dont have access there, let me write), that keeps qpid::framing::Buffer for every
durable exchange with sequencing enabled, but it returned (among others) into the need of
changing the way store encodes/decodes Exchange instance (change in Exchange::encode / decode
methods). What would make the broker backward incompatible.

Is the performance penalty (due to Exchange::encode method called for every message preroute)

Is it worth merging MessageStoreImpl::update intoMessageStoreImpl::create method?


  /trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1582719 

Diff: https://reviews.apache.org/r/19566/diff/


- reproducer from JIRA verified
- automated tests passed (except for those known to fail due to QPID-5641 (valgrind &


Pavel Moravec

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