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:
https://reviews.apache.org/r/19566/
-----------------------------------------------------------

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


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


Bugs: QPID-5642
    https://issues.apache.org/jira/browse/QPID-5642


Repository: qpid


Description
-------

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)
acceptable?

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


Diffs
-----

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

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


Testing
-------

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


Thanks,

Pavel Moravec


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