Re: 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/#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
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.3244.03 18.28 30064
31778 31729 0.2254.36 16.77 31068
31664 31637 0.3969.50 22.11 31014
31303 31301 0.3343.97 17.32 30737
31577 31403 0.3337.73 15.59 30785
$ qpid-cpp-benchmark --repeat 5
send-tp recv-tp l-min l-max l-avg total-tp
47138 47120 0.1329.02 9.3545870
46299 46156 0.1325.24 8.3344803
45910 45793 0.1353.53 10.81 45300
46607 46388 0.1629.70 9.2145245
46905 46766 0.1631.43 9.6245776
- 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
---
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
---
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
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
---
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
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
---
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
---
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
Re: Review Request: Add 64 bit message sequence to enqueued messages
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
---
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
