> On March 31, 2014, 2:13 p.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/Exchange.cpp, line 210 > > <https://reviews.apache.org/r/19566/diff/4/?file=539793#file539793line210> > > > > The reason the value was originally set in args was so that it would be > > replicated in old cluster. > > > > A question would be how the new ha handles sequencing. In the ideal > > world the boot sequence of a new master would always be one higher than any > > previous master. However that is really orthogonal to this issue. > > > > Maybe add a comment around the likelhood of wraparound be considered > > remote enough that it is not handled? > > > > Looks good to me though. > > Pavel Moravec wrote: > Good point with new HA cluster. I see two possible ways how to get boot > sequence lower within the cluster: > > 1) A user deletes <data-dir>/.mbrokerdata file, thus resetting boot > sequence to 1. I spotted this user case sometimes when troubleshooting / > attempting to resolve broker startup problems - let start broker with "clean > table" if it helps. > > 2) Assume 2node cluster with brokers A (primary) and B (backup). > Restarting B five times sets B boot sequence to 6, then restarting A once > sets its boot sequence to 2. Now B is the primary broker. Then restarting B > causes primary is A again, but with lower boot sequence. > > I tried to come up with a mechanism of updating boot sequence during a > new joiner recovery, but there are two contradicting requirements: > a) every new joiner has to have bigger boot sequence than the current > primary node - to ensure failover to this new joiner from primary will not > lower "boot_sequence<<47 + exchange.seqeuenceNo" number. So any two new > joiners B and C should have boot sequence bigger than primary A. > b) But also B should have boot sequence bigger than C (if it will become > primary after A) while C should have it bigger than B for the same reason. > > The only way is maintaining boot sequence cluster-wide on the same value, > and during promoting _any_ broker, the number is incremented by one - cluster > wide. (and of course, during recovery, it is updated to new joiner). > > Alan, is the above paragraph (easily) feasible? (if so I can implement it)
If the boot sequence is incremented during any broker promotion then having only 16 bits for the boot sequence is probably too small. - Chug ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19566/#review39038 ----------------------------------------------------------- On April 1, 2014, 10:45 a.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > 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 > >
