----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11009/#review20370 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp <https://reviews.apache.org/r/11009/#comment41953> Just a suggestion: instead of turning sequencing on with a boolean option, you could allow users to choose the property name they wish the sequence to be inserted as. The default is the empty string meaning no sequencing. Gives a little more flexibility to the user without much cost. As I say however, just a suggestion, not a blocking issue. /trunk/qpid/cpp/src/qpid/broker/Queue.cpp <https://reviews.apache.org/r/11009/#comment41952> Note: Alan is reporting significant impact to performance from the addAnnotation() method. This is just fyi really as there is as yet no alternative. /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp <https://reviews.apache.org/r/11009/#comment41951> This means that even if you set qpid.queue_msg_sequence=False in the arguments, sequencing will be turned on. Replacing 'true' with 'value' would I think be better. - Gordon Sim On May 8, 2013, 7:30 p.m., Ernie Allen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11009/ > ----------------------------------------------------------- > > (Updated May 8, 2013, 7:30 p.m.) > > > Review request for qpid, Alan Conway, Gordon Sim, and Justin Ross. > > > Description > ------- > > The primary purpose of this is to detect when a ring queue overwrites > messages. The client can examine the sequence number and look for gaps which > indicate that messages have been dropped. > > To enable, create the queue with the qpid.queue_msg_sequence arg: > std::string addr("my-queue;" > " {create:sender, delete:always," > " node: {type: queue, x-declare: {arguments:" > " {qpid.queue_msg_sequence:1, qpid.policy_type:ring, > qpid.max_count:4}}}}"); > > Sender sender = session.createSender(addr); > This enables sequencing when a message is enqueued. Sequence numbers start at > 1 and are unsigned 64 bit numbers. > The exchange level sequencing uses "qpid.msg_sequence" as the key. > > *** Question *** Should queues use the same key to enable sequencing as > exchanges? "qpid.msg_sequence" The page > https://cwiki.apache.org/qpid/qpid-extensions-to-amqp.html indicates that > qpid.msg_sequence is supported when declaring a queue, but that key was never > supported for queues. > > > To get the sequence number: > uint64_t seqNo; > seqNo = response.getProperties()["qpid.queue_msg_sequence"]; > > *** Note *** > I added a uint64 to the queue class. There is an existing message sequence > number for queues. It is a SequenceNumber class. > However, this is a 32 bit number. I was leery of changing that to a 64 bit > number since it is used throughout the broker and not just as a msg counter. > I'm assuming a 32 bit number is not sufficient in this case since "wrapping > around" to zero would complicate the logic of detecting overwritten messages. > > There is a public class variable in Queue.h named std::string seqNoKey; It is > unused. My guess is that it was intended for use with the SequenceNumber > class. Can it be removed? > > This does not address the issue of persisting the message sequence after a > broker restart. This will be a future issue. > > Before committing this, C++ and python tests would need to be added. > > > This addresses bug QPID-4591. > https://issues.apache.org/jira/browse/QPID-4591 > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Queue.h 1478851 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1478851 > /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1478851 > /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1478851 > > Diff: https://reviews.apache.org/r/11009/diff/ > > > Testing > ------- > > See attached testring.cpp and mm (make script) > They create a ring queue that holds 4 messages with qpid.queue_msg_sequence, > sends enough messages to overflow the queue, and then retrieves the messages. > It outputs the message sequence numbers. > > > Thanks, > > Ernie Allen > >
