----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19566/#review38730 -----------------------------------------------------------
The boot sequence number is 16 bits (uint16_t) and the message sequence number is 63 bits (int64_t, where the top bit is unused as counts are never negative (hopefully)). Another alternative is to merge the two fields by overlaying the boot sequence number into the upper bits of the message sequence. Off the cuff: qpid.msg_sequence = ((int64_t) getBootSequence()) << 47 + exchange.msg_sequence; Each broker restart would then have 47 bits (1.41e+14) of counting to do before an exchange got into trouble with "wrapping" into the next boot sequence start point. Each broker would have 16 bits (65,535) of counting restarts before the BootSequence wrapped back to zero. Then the generated number fits into the current int64_t field and client code doesn't change at all. I think that these numbers are generous enough then I would go this way. Otherwise I'd vote for scheme 1. Maybe a customer how needs this feature could help choose which scheme would be best. - Chug Rolke On March 27, 2014, 8:58 a.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19566/ > ----------------------------------------------------------- > > (Updated March 27, 2014, 8:58 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 1582207 > > 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 > >
