----------------------------------------------------------- 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