-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19566/#review38601
-----------------------------------------------------------
The broker currently has a concept of a boot sequence number, which is
incremented each time that qpidd restarts. This number is not the boot sequence
number of the host computer.
$ ./qpidd --log-enable debug+
... debug ManagementAgent boot sequence: 44
$ ./qpidd --log-enable debug+
... debug ManagementAgent boot sequence: 45
The exchange Message sequence number gets reset to zero at each qpidd restart.
Trying to track each message sequence number through nonvolatile storage is a
huge load. Also, it is likely that there are reliability problems tracking the
sequence numbers through a broker crash or restart, just when the sequence
numbers are critical.
What about extending the concept of a "sequence number" to include "<broker
boot sequence>:<exchange Message sequence number>"?
These numbers exist today so there's no change to generate them. The new scheme
would be a way to pack both numbers into a message instead of just the Message
sequence number.
- Chug Rolke
On March 23, 2014, 11:52 a.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19566/
> -----------------------------------------------------------
>
> (Updated March 23, 2014, 11:52 a.m.)
>
>
> Review request for qpid, 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 1577475
> /trunk/qpid/cpp/src/qpid/broker/MessageStore.h 1577475
> /trunk/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1577475
> /trunk/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1577475
> /trunk/qpid/cpp/src/qpid/broker/NullMessageStore.h 1577475
> /trunk/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 1577475
> /trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.h 1577475
> /trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.cpp 1577475
> /trunk/qpid/cpp/src/qpid/linearstore/MessageStoreImpl.h 1577475
> /trunk/qpid/cpp/src/qpid/linearstore/MessageStoreImpl.cpp 1577475
> /trunk/qpid/cpp/src/qpid/store/MessageStorePlugin.h 1577475
> /trunk/qpid/cpp/src/qpid/store/MessageStorePlugin.cpp 1577475
>
> 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
>
>