-----------------------------------------------------------
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.
Changes
-------
Trivial fix that adds - in parallel to qpid.msg_sequence - also
qpid.boot_sequence message annotation. End user then should lexicographically
compare the pair of values (qpid.boot_sequence, qpid.msg_sequence).
Then message headers look like:
Properties: {qpid.boot_sequence:16, qpid.msg_sequence:2, sn:1,
ts:1395909295605937918, x-amqp-0-10.routing-key:}
Properties: {qpid.boot_sequence:16, qpid.msg_sequence:3, sn:2,
ts:1395909295606126626, x-amqp-0-10.routing-key:}
Properties: {qpid.boot_sequence:16, qpid.msg_sequence:4, sn:3,
ts:1395909295606179836, x-amqp-0-10.routing-key:}
(broker restart)
Properties: {qpid.boot_sequence:17, qpid.msg_sequence:1, sn:4,
ts:1395909314496162977, x-amqp-0-10.routing-key:}
Properties: {qpid.boot_sequence:17, qpid.msg_sequence:2, sn:5,
ts:1395909314496364823, x-amqp-0-10.routing-key:}
Properties: {qpid.boot_sequence:17, qpid.msg_sequence:3, sn:6,
ts:1395909314496437384, x-amqp-0-10.routing-key:}
An optional fix that sets just qpid.msg_sequence as string
"boot_sequence:exchange_sequence". End user then has to parse the string but
gets the sequence stuff in just one annotation :
Index: cpp/src/qpid/broker/Exchange.cpp
===================================================================
--- cpp/src/qpid/broker/Exchange.cpp (revision 1582207)
+++ cpp/src/qpid/broker/Exchange.cpp (working copy)
@@ -63,7 +63,9 @@
if (parent->sequence){
parent->sequenceNo++;
- msg.getMessage().addAnnotation(qpidMsgSequence,parent->sequenceNo);
+ char s[129];
+ sprintf(s, "%d:%ld",
parent->getBroker()->getManagementAgent()->getBootSequence(),
parent->sequenceNo);
+ msg.getMessage().addAnnotation(qpidMsgSequence,s);
}
if (parent->ive) {
parent->lastMsg = msg.getMessage();
Then message headers look like:
Properties: {qpid.msg_sequence:18:1, sn:1, ts:1395910232986094505,
x-amqp-0-10.routing-key:}
Properties: {qpid.msg_sequence:18:2, sn:2, ts:1395910232986239616,
x-amqp-0-10.routing-key:}
Properties: {qpid.msg_sequence:18:3, sn:3, ts:1395910232986274364,
x-amqp-0-10.routing-key:}
(broker restart)
Properties: {qpid.msg_sequence:19:1, sn:4, ts:1395910238014147248,
x-amqp-0-10.routing-key:}
Properties: {qpid.msg_sequence:19:2, sn:5, ts:1395910238014284816,
x-amqp-0-10.routing-key:}
Properties: {qpid.msg_sequence:19:3, sn:6, ts:1395910238014317980,
x-amqp-0-10.routing-key:}
I am in favour of the first option, let me know your preference.
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 (updated)
-----
/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
