> On May 8, 2013, 6:03 p.m., Andrew Stitcher wrote:
> > I'm not sure I buy the argument against a 32 bit sequence no  (or reusing 
> > the 32 bit value already there). You detect dropped messages by gaps in the 
> > sequence numbers. You'd have to miss a hell of a lot of messages (4 billion 
> > or so) before you could think you'd not missed a message when you had 
> > actually missed one, and even then you'd have to be extremely unlucky to 
> > miss exactly 2^32 messages.

I was concerned about the comparison logic needed when the sequence number 
overflows to 0, but after some testing, it appears that concern was unfounded. 
I'm now using the existing 32 but sequence number.


- Ernie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/#review20331
-----------------------------------------------------------


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
> 
>

Reply via email to