You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Ernie Allen <ea...@redhat.com> on 2013/05/08 19:45:13 UTC
Review Request: Add 64 bit message sequence to enqueued messages
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/
-----------------------------------------------------------
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
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Ernie Allen <ea...@redhat.com>.
> 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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/#review20331
-----------------------------------------------------------
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.
/trunk/qpid/cpp/src/qpid/broker/Queue.h
<https://reviews.apache.org/r/11009/#comment41882>
Do you actually need a member in Queue when the member in settings will still be accessible as settings.sequencing anyway?
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/11009/#comment41883>
This may not be necessary (as above)
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/11009/#comment41881>
[possibly controversial point]
I'd prefer to see this in the initialisation list as:
sequencing(settings.sequencing),
sequenceNo(settings.sequencing ? 1 : 0),
In general in a C++ constructor you should do as much of the initialisation as possible in the initialisation list rather than in the body of the constructor.
(but note this while comment may be rendered moot, by other comments above and below)
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/11009/#comment41884>
Why not initialise sequenceNo to 0 unconditionally and use ++sequenceNo here?
- Andrew Stitcher
On May 8, 2013, 5:45 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, 5:45 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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Gordon Sim <gs...@redhat.com>.
> On May 9, 2013, 8:12 a.m., Gordon Sim wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 743
> > <https://reviews.apache.org/r/11009/diff/2/?file=289011#file289011line743>
> >
> > Note: Alan is reporting significant impact to performance from the addAnnotation() method. This is just fyi really as there is as yet no alternative.
$ qpid-cpp-benchmark --repeat 5 --create-option "node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}"
send-tp recv-tp l-min l-max l-avg total-tp
30720 30671 0.32 44.03 18.28 30064
31778 31729 0.22 54.36 16.77 31068
31664 31637 0.39 69.50 22.11 31014
31303 31301 0.33 43.97 17.32 30737
31577 31403 0.33 37.73 15.59 30785
$ qpid-cpp-benchmark --repeat 5
send-tp recv-tp l-min l-max l-avg total-tp
47138 47120 0.13 29.02 9.35 45870
46299 46156 0.13 25.24 8.33 44803
45910 45793 0.13 53.53 10.81 45300
46607 46388 0.16 29.70 9.21 45245
46905 46766 0.16 31.43 9.62 45776
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/#review20370
-----------------------------------------------------------
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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/#review20340
-----------------------------------------------------------
Ship it!
This looks good to me (subject to passing tests)
- Andrew Stitcher
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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Ernie Allen <ea...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/
-----------------------------------------------------------
(Updated May 9, 2013, 5:40 p.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Justin Ross.
Changes
-------
This patch is based on an updated trunk.
The annotation key under which sequence numbers are stored is now specified in the settings.
In other words, to enable queue sequencing, declare the queue with
arguments: {'qpid.queue_msg_sequence':'user_key_name'...}
The user_key_name can be any client supplied non-empty string.
Also included are the python tests for creating the queue and retrieving the sequence numbers.
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 (updated)
-----
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1480722
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1480722
/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1480722
/trunk/qpid/tests/src/py/qpid_tests/broker_0_10/new_api.py 1480722
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
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Andrew Stitcher <as...@apache.org>.
> On May 9, 2013, 1:47 p.m., Alan Conway wrote:
> > Like Gordon said, I have been working on adding a HA ID to messages, and there is a dramatic performance degradation. I'm torn between optimizing addAnnotations or handling the HA ID in a different way for now. If we both end up needing a sequence number we should use the same one.
addAnnotation() is likely a lot better performant for amqp-1.0 than for amqp-0_10. It would be worth testing this. It's probably not worth fixing for 0_10, it will be difficult anyway I think.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/#review20377
-----------------------------------------------------------
On May 9, 2013, 12:35 p.m., Ernie Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11009/
> -----------------------------------------------------------
>
> (Updated May 9, 2013, 12:35 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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/#review20377
-----------------------------------------------------------
Like Gordon said, I have been working on adding a HA ID to messages, and there is a dramatic performance degradation. I'm torn between optimizing addAnnotations or handling the HA ID in a different way for now. If we both end up needing a sequence number we should use the same one.
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/11009/#comment41958>
Suggest renaming to "qpid.seq" to reduce per-message overhead.
- Alan Conway
On May 9, 2013, 12:35 p.m., Ernie Allen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11009/
> -----------------------------------------------------------
>
> (Updated May 9, 2013, 12:35 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
>
>
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Ernie Allen <ea...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11009/
-----------------------------------------------------------
(Updated May 9, 2013, 12:35 p.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Justin Ross.
Changes
-------
Changed QueueSettings initialization to use the value of qpid.queue_msg_sequencing so declaring the queue with qpid.queue_msg_sequence=False will not turn on sequencing
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 (updated)
-----
/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
Re: Review Request: Add 64 bit message sequence to enqueued messages
Posted by Ernie Allen <ea...@redhat.com>.
-----------------------------------------------------------
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.
Changes
-------
This patch uses the existing 32 bit sequence number.
I used a 64 bit sequence number because I was concerned about what would happen when the sequence number overflowed and wrapped to 0. I thought the logic to detect gaps when that condition occurred would be tricky. However, Andrew is quite correct. The existing unisgned 32 bit sequence number works perfectly with the logic:
if (seqNo - lastSeqNo > 1) even when lastSeqNo is at MAX and seqNo has just overflowed.
I removed the local bool sequencing and used the setting.sequencing.
I removed the initialization code since I'm now using the existing sequence number and that is initialized to 0 upon construction.
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 (updated)
-----
/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