You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by mick goulish <mi...@redhat.com> on 2012/09/06 15:51:35 UTC
Review Request: browse-only queues
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6943/
-----------------------------------------------------------
Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
Description
-------
browse-only queues
prevent consumer from acquiring, and prevent queue from dequeing -- no change to any class except Queue, and QueueSettings.
yes, questions:
1. is it OK to *not* change anything else. i.e. i don't see that read-credit should be affected.
2. also, i did not want to change anything except Queue -- i.e. no change to Consumer cursor. Let it think that it is consuming -- but Queue keeps messages. This might allow queue to change back and forth from browse-only to normal, if we ever care.
Diffs
-----
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1376905
trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1376905
trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1376905
Diff: https://reviews.apache.org/r/6943/diff/
Testing
-------
passed make check except for 1 HA test, on recent trunk
Create queue with "browse-only" arg, load 10 messages, and then see that consumer can 'consume' any number of time, but same messages are always there.
Here are some scripts for testing:
---- script: start_broker -------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
rm -f ${LOG}
echo "Starting broker..."
${QPIDD} \
--auth=no \
--log-enable info+ \
--log-to-file ${LOG} \
--log-hires-timestamp=yes \
--log-source=yes \
-p ${PORT} \
-d
echo "Broker started."
---- end script -----------------
---- script: load_messages -------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
echo "============================================" >> ${LOG}
echo "Loading the messages" >> ${LOG}
echo "============================================" >> ${LOG}
echo "Loading the messages"
sleep 3
# address was: 'q; {create: always, node:{ type: queue }}'
COUNT=10
echo "Sending $COUNT messages..."
${SPOUT} \
-c 10 \
--broker "localhost:${PORT}" \
--content "All Work and No Play Make Mick a Dull Boy" \
'q; {create: always, node:{type:queue , x-declare:{arguments:{"browse-only":1}}}}'
echo " "
echo "Done sending messages."
echo " "
---- end script ------------------
---- script: consume -------------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
echo "============================================" >> ${LOG}
echo "Consuming the messages" >> ${LOG}
echo "============================================" >> ${LOG}
echo "Consuming the messages"
sleep 5
${DRAIN} \
--broker 'localhost:5801' \
'q;'
echo "done."
---- end script ------------------
---- script: browse --------------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
echo "============================================" >> ${LOG}
echo "Browsing the messages" >> ${LOG}
echo "============================================" >> ${LOG}
echo "Browsing the messages"
sleep 5
${DRAIN} \
--broker 'localhost:5801' \
'q; {mode: browse}'
echo "done."
---- end script ------------------
Thanks,
mick goulish
Re: Review Request: browse-only queues
Posted by Alan Conway <ac...@redhat.com>.
> On Sept. 6, 2012, 4:57 p.m., Gordon Sim wrote:
> > I'm not hugely keen on this approach, in particular the changes to Queue::getNextMessage() and Queue::dequeue(). I don't understand why you want to keep the changes within the queue. It seems cleaner for me to do this at the earliest point possible, when actually handling the subscribe request. E.g. in conjunction with your change to QueueSettings:
> >
> > Index: cpp/src/qpid/broker/SessionAdapter.cpp
> > ===================================================================
> > --- cpp/src/qpid/broker/SessionAdapter.cpp (revision 1378042)
> > +++ cpp/src/qpid/broker/SessionAdapter.cpp (working copy)
> > @@ -422,6 +422,10 @@
> > if(!destination.empty() && state.exists(destination))
> > throw NotAllowedException(QPID_MSG("Consumer tags must be unique"));
> >
> > + if (queue->getSettings().isBrowseOnly && acquireMode == 0) {
> > + QPID_LOG(info, "Overriding request to consume from browse only queue " << queue->getName());
> > + acquireMode = 1;
> > + }
> > // We allow browsing (acquireMode == 1) of exclusive queues, this is required by HA.
> > if (queue->hasExclusiveOwner() && !queue->isExclusiveOwner(&session) && acquireMode == 0)
> > throw ResourceLockedException(QPID_MSG("Cannot subscribe to exclusive queue
> >
> > To me that seems like a cleaner an more direct implementation of the functionality.
I agree with Gordon.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6943/#review11104
-----------------------------------------------------------
On Sept. 6, 2012, 1:52 p.m., mick goulish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6943/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2012, 1:52 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
>
>
> Description
> -------
>
> browse-only queues
>
> prevent consumer from acquiring, and prevent queue from dequeing -- no change to any class except Queue, and QueueSettings.
>
>
> yes, questions:
>
> 1. is it OK to *not* change anything else. i.e. i don't see that read-credit should be affected.
>
> 2. also, i did not want to change anything except Queue -- i.e. no change to Consumer cursor. Let it think that it is consuming -- but Queue keeps messages. This might allow queue to change back and forth from browse-only to normal, if we ever care.
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1376905
> trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1376905
> trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1376905
>
> Diff: https://reviews.apache.org/r/6943/diff/
>
>
> Testing
> -------
>
> passed make check except for 1 HA test, on recent trunk
> Create queue with "browse-only" arg, load 10 messages, and then see that consumer can 'consume' any number of time, but same messages are always there.
>
>
> Here are some scripts for testing:
>
> ---- script: start_broker -------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> rm -f ${LOG}
>
>
> echo "Starting broker..."
> ${QPIDD} \
> --auth=no \
> --log-enable info+ \
> --log-to-file ${LOG} \
> --log-hires-timestamp=yes \
> --log-source=yes \
> -p ${PORT} \
> -d
> echo "Broker started."
> ---- end script -----------------
>
>
> ---- script: load_messages -------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> echo "============================================" >> ${LOG}
> echo "Loading the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Loading the messages"
>
> sleep 3
>
>
> # address was: 'q; {create: always, node:{ type: queue }}'
>
> COUNT=10
> echo "Sending $COUNT messages..."
> ${SPOUT} \
> -c 10 \
> --broker "localhost:${PORT}" \
> --content "All Work and No Play Make Mick a Dull Boy" \
> 'q; {create: always, node:{type:queue , x-declare:{arguments:{"browse-only":1}}}}'
>
> echo " "
> echo "Done sending messages."
> echo " "
> ---- end script ------------------
>
>
> ---- script: consume -------------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> echo "============================================" >> ${LOG}
> echo "Consuming the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Consuming the messages"
>
> sleep 5
>
> ${DRAIN} \
> --broker 'localhost:5801' \
> 'q;'
>
>
> echo "done."
> ---- end script ------------------
>
>
> ---- script: browse --------------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
> echo "============================================" >> ${LOG}
> echo "Browsing the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Browsing the messages"
>
> sleep 5
>
> ${DRAIN} \
> --broker 'localhost:5801' \
> 'q; {mode: browse}'
>
>
> echo "done."
> ---- end script ------------------
>
>
> Thanks,
>
> mick goulish
>
>
Re: Review Request: browse-only queues
Posted by Alan Conway <ac...@redhat.com>.
On Thu, 2012-09-06 at 13:24 -0400, mick wrote:
> It seemed reasonable to me to keep it within the queue code, because
> browse-only-ness is a property of queues, and not of sessions.
>
> By moving the enforcement of browse-only-ness out of the queue code, it
> seems to me that opens a possibility that some other pathway will
> someday be created -- not through the SessionAdapter code -- that will
> unknowingly violate the browse-only-ness of the Queue.
>
> Innit ?
I think it's safer and cleaner to say a browse-only queue is one that
allows only browsers. We already have code paths to distinguish browsers
vs. consumers, I think it's better to use the existing browser path than
create a 3rd path that makes consumers sometimes browse.
As for future changes, I don't think we can say now what will be best
then, but in general an approach involves less code will be easier to
maintain.
>
> On Thu, 2012-09-06 at 16:57 +0000, Gordon Sim wrote:
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/6943/
> >
> > I'm not hugely keen on this approach, in particular the changes to Queue::getNextMessage() and Queue::dequeue(). I don't understand why you want to keep the changes within the queue. It seems cleaner for me to do this at the earliest point possible, when actually handling the subscribe request. E.g. in conjunction with your change to QueueSettings:
> >
> > Index: cpp/src/qpid/broker/SessionAdapter.cpp
> > ===================================================================
> > --- cpp/src/qpid/broker/SessionAdapter.cpp (revision 1378042)
> > +++ cpp/src/qpid/broker/SessionAdapter.cpp (working copy)
> > @@ -422,6 +422,10 @@
> > if(!destination.empty() && state.exists(destination))
> > throw NotAllowedException(QPID_MSG("Consumer tags must be unique"));
> >
> > + if (queue->getSettings().isBrowseOnly && acquireMode == 0) {
> > + QPID_LOG(info, "Overriding request to consume from browse only queue " << queue->getName());
> > + acquireMode = 1;
> > + }
> > // We allow browsing (acquireMode == 1) of exclusive queues, this is required by HA.
> > if (queue->hasExclusiveOwner() && !queue->isExclusiveOwner(&session) && acquireMode == 0)
> > throw ResourceLockedException(QPID_MSG("Cannot subscribe to exclusive queue
> >
> > To me that seems like a cleaner an more direct implementation of the functionality.
> >
> >
> >
> > - Gordon
> >
> >
> > On September 6th, 2012, 1:52 p.m., mick goulish wrote:
> >
> > Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
> > By mick goulish.
> > Updated Sept. 6, 2012, 1:52 p.m.
> >
> >
> > Description
> > browse-only queues
> >
> > prevent consumer from acquiring, and prevent queue from dequeing -- no change to any class except Queue, and QueueSettings.
> >
> >
> > yes, questions:
> >
> > 1. is it OK to *not* change anything else. i.e. i don't see that read-credit should be affected.
> >
> > 2. also, i did not want to change anything except Queue -- i.e. no change to Consumer cursor. Let it think that it is consuming -- but Queue keeps messages. This might allow queue to change back and forth from browse-only to normal, if we ever care.
> > Testing
> > passed make check except for 1 HA test, on recent trunk
> > Create queue with "browse-only" arg, load 10 messages, and then see that consumer can 'consume' any number of time, but same messages are always there.
> >
> >
> >
> > Here are some scripts for testing:
> >
> > ---- script: start_broker -------
> > #! /bin/bash
> >
> >
> > PORT=5801
> > TRUNK=/home/mick/redhat/trunk
> > QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> > QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> > SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> > DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> > LOG=/tmp/qpidd.log
> >
> >
> > rm -f ${LOG}
> >
> >
> > echo "Starting broker..."
> > ${QPIDD} \
> > --auth=no \
> > --log-enable info+ \
> > --log-to-file ${LOG} \
> > --log-hires-timestamp=yes \
> > --log-source=yes \
> > -p ${PORT} \
> > -d
> > echo "Broker started."
> > ---- end script -----------------
> >
> >
> >
> > ---- script: load_messages -------
> > #! /bin/bash
> >
> >
> > PORT=5801
> > TRUNK=/home/mick/redhat/trunk
> > QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> > QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> > SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> > DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> > LOG=/tmp/qpidd.log
> >
> >
> > echo "============================================" >> ${LOG}
> > echo "Loading the messages" >> ${LOG}
> > echo "============================================" >> ${LOG}
> > echo "Loading the messages"
> >
> > sleep 3
> >
> >
> > # address was: 'q; {create: always, node:{ type: queue }}'
> >
> > COUNT=10
> > echo "Sending $COUNT messages..."
> > ${SPOUT} \
> > -c 10 \
> > --broker "localhost:${PORT}" \
> > --content "All Work and No Play Make Mick a Dull Boy" \
> > 'q; {create: always, node:{type:queue , x-declare:{arguments:{"browse-only":1}}}}'
> >
> > echo " "
> > echo "Done sending messages."
> > echo " "
> > ---- end script ------------------
> >
> >
> >
> >
> > ---- script: consume -------------
> > #! /bin/bash
> >
> >
> > PORT=5801
> > TRUNK=/home/mick/redhat/trunk
> > QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> > QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> > SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> > DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> > LOG=/tmp/qpidd.log
> >
> >
> > echo "============================================" >> ${LOG}
> > echo "Consuming the messages" >> ${LOG}
> > echo "============================================" >> ${LOG}
> > echo "Consuming the messages"
> >
> > sleep 5
> >
> > ${DRAIN} \
> > --broker 'localhost:5801' \
> > 'q;'
> >
> >
> > echo "done."
> > ---- end script ------------------
> >
> >
> > ---- script: browse --------------
> > #! /bin/bash
> >
> >
> > PORT=5801
> > TRUNK=/home/mick/redhat/trunk
> > QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> > QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> > SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> > DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> > LOG=/tmp/qpidd.log
> >
> > echo "============================================" >> ${LOG}
> > echo "Browsing the messages" >> ${LOG}
> > echo "============================================" >> ${LOG}
> > echo "Browsing the messages"
> >
> > sleep 5
> >
> > ${DRAIN} \
> > --broker 'localhost:5801' \
> > 'q; {mode: browse}'
> >
> >
> > echo "done."
> > ---- end script ------------------
> > Diffs
> > * trunk/qpid/cpp/src/qpid/broker/Queue.cpp (1376905)
> > * trunk/qpid/cpp/src/qpid/broker/QueueSettings.h (1376905)
> > * trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp (1376905)
> >
> > View Diff
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> For additional commands, e-mail: dev-help@qpid.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: Review Request: browse-only queues
Posted by mick <mg...@redhat.com>.
On Thu, 2012-09-06 at 18:53 +0100, Gordon Sim wrote:
> On 09/06/2012 06:24 PM, mick wrote:
> > It seemed reasonable to me to keep it within the queue code, because
> > browse-only-ness is a property of queues, and not of sessions.
>
> While I agree there is some logic to that, I think the interface for
> subscribers to the queue is sufficiently ugly at present that it seems
> less neat to pursue that option. At least to me.
>
> For example having subscribers themselves decide if they acquired the
> message then directly calling dequeue() is ugly and necessitates
> ugliness if trying to keep everything in the queue.
>
> > By moving the enforcement of browse-only-ness out of the queue code, it
> > seems to me that opens a possibility that some other pathway will
> > someday be created -- not through the SessionAdapter code -- that will
> > unknowingly violate the browse-only-ness of the Queue.
>
> Fair point, how about a test on Queue::consume() that throws an error
> for any attempt to pass in an acquiring 'consumer'. (The terminology in
> this part of the code makes me weep!)
>
>
Thanks, that makes sense.
The terminology made me weep too -- but I increased my meds and now it
just doesn't seem to matter.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: Review Request: browse-only queues
Posted by Gordon Sim <gs...@redhat.com>.
On 09/06/2012 06:24 PM, mick wrote:
> It seemed reasonable to me to keep it within the queue code, because
> browse-only-ness is a property of queues, and not of sessions.
While I agree there is some logic to that, I think the interface for
subscribers to the queue is sufficiently ugly at present that it seems
less neat to pursue that option. At least to me.
For example having subscribers themselves decide if they acquired the
message then directly calling dequeue() is ugly and necessitates
ugliness if trying to keep everything in the queue.
> By moving the enforcement of browse-only-ness out of the queue code, it
> seems to me that opens a possibility that some other pathway will
> someday be created -- not through the SessionAdapter code -- that will
> unknowingly violate the browse-only-ness of the Queue.
Fair point, how about a test on Queue::consume() that throws an error
for any attempt to pass in an acquiring 'consumer'. (The terminology in
this part of the code makes me weep!)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: Review Request: browse-only queues
Posted by mick <mg...@redhat.com>.
It seemed reasonable to me to keep it within the queue code, because
browse-only-ness is a property of queues, and not of sessions.
By moving the enforcement of browse-only-ness out of the queue code, it
seems to me that opens a possibility that some other pathway will
someday be created -- not through the SessionAdapter code -- that will
unknowingly violate the browse-only-ness of the Queue.
Innit ?
On Thu, 2012-09-06 at 16:57 +0000, Gordon Sim wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6943/
>
> I'm not hugely keen on this approach, in particular the changes to Queue::getNextMessage() and Queue::dequeue(). I don't understand why you want to keep the changes within the queue. It seems cleaner for me to do this at the earliest point possible, when actually handling the subscribe request. E.g. in conjunction with your change to QueueSettings:
>
> Index: cpp/src/qpid/broker/SessionAdapter.cpp
> ===================================================================
> --- cpp/src/qpid/broker/SessionAdapter.cpp (revision 1378042)
> +++ cpp/src/qpid/broker/SessionAdapter.cpp (working copy)
> @@ -422,6 +422,10 @@
> if(!destination.empty() && state.exists(destination))
> throw NotAllowedException(QPID_MSG("Consumer tags must be unique"));
>
> + if (queue->getSettings().isBrowseOnly && acquireMode == 0) {
> + QPID_LOG(info, "Overriding request to consume from browse only queue " << queue->getName());
> + acquireMode = 1;
> + }
> // We allow browsing (acquireMode == 1) of exclusive queues, this is required by HA.
> if (queue->hasExclusiveOwner() && !queue->isExclusiveOwner(&session) && acquireMode == 0)
> throw ResourceLockedException(QPID_MSG("Cannot subscribe to exclusive queue
>
> To me that seems like a cleaner an more direct implementation of the functionality.
>
>
>
> - Gordon
>
>
> On September 6th, 2012, 1:52 p.m., mick goulish wrote:
>
> Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
> By mick goulish.
> Updated Sept. 6, 2012, 1:52 p.m.
>
>
> Description
> browse-only queues
>
> prevent consumer from acquiring, and prevent queue from dequeing -- no change to any class except Queue, and QueueSettings.
>
>
> yes, questions:
>
> 1. is it OK to *not* change anything else. i.e. i don't see that read-credit should be affected.
>
> 2. also, i did not want to change anything except Queue -- i.e. no change to Consumer cursor. Let it think that it is consuming -- but Queue keeps messages. This might allow queue to change back and forth from browse-only to normal, if we ever care.
> Testing
> passed make check except for 1 HA test, on recent trunk
> Create queue with "browse-only" arg, load 10 messages, and then see that consumer can 'consume' any number of time, but same messages are always there.
>
>
>
> Here are some scripts for testing:
>
> ---- script: start_broker -------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> rm -f ${LOG}
>
>
> echo "Starting broker..."
> ${QPIDD} \
> --auth=no \
> --log-enable info+ \
> --log-to-file ${LOG} \
> --log-hires-timestamp=yes \
> --log-source=yes \
> -p ${PORT} \
> -d
> echo "Broker started."
> ---- end script -----------------
>
>
>
> ---- script: load_messages -------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> echo "============================================" >> ${LOG}
> echo "Loading the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Loading the messages"
>
> sleep 3
>
>
> # address was: 'q; {create: always, node:{ type: queue }}'
>
> COUNT=10
> echo "Sending $COUNT messages..."
> ${SPOUT} \
> -c 10 \
> --broker "localhost:${PORT}" \
> --content "All Work and No Play Make Mick a Dull Boy" \
> 'q; {create: always, node:{type:queue , x-declare:{arguments:{"browse-only":1}}}}'
>
> echo " "
> echo "Done sending messages."
> echo " "
> ---- end script ------------------
>
>
>
>
> ---- script: consume -------------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> echo "============================================" >> ${LOG}
> echo "Consuming the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Consuming the messages"
>
> sleep 5
>
> ${DRAIN} \
> --broker 'localhost:5801' \
> 'q;'
>
>
> echo "done."
> ---- end script ------------------
>
>
> ---- script: browse --------------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
> echo "============================================" >> ${LOG}
> echo "Browsing the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Browsing the messages"
>
> sleep 5
>
> ${DRAIN} \
> --broker 'localhost:5801' \
> 'q; {mode: browse}'
>
>
> echo "done."
> ---- end script ------------------
> Diffs
> * trunk/qpid/cpp/src/qpid/broker/Queue.cpp (1376905)
> * trunk/qpid/cpp/src/qpid/broker/QueueSettings.h (1376905)
> * trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp (1376905)
>
> View Diff
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: Review Request: browse-only queues
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6943/#review11104
-----------------------------------------------------------
I'm not hugely keen on this approach, in particular the changes to Queue::getNextMessage() and Queue::dequeue(). I don't understand why you want to keep the changes within the queue. It seems cleaner for me to do this at the earliest point possible, when actually handling the subscribe request. E.g. in conjunction with your change to QueueSettings:
Index: cpp/src/qpid/broker/SessionAdapter.cpp
===================================================================
--- cpp/src/qpid/broker/SessionAdapter.cpp (revision 1378042)
+++ cpp/src/qpid/broker/SessionAdapter.cpp (working copy)
@@ -422,6 +422,10 @@
if(!destination.empty() && state.exists(destination))
throw NotAllowedException(QPID_MSG("Consumer tags must be unique"));
+ if (queue->getSettings().isBrowseOnly && acquireMode == 0) {
+ QPID_LOG(info, "Overriding request to consume from browse only queue " << queue->getName());
+ acquireMode = 1;
+ }
// We allow browsing (acquireMode == 1) of exclusive queues, this is required by HA.
if (queue->hasExclusiveOwner() && !queue->isExclusiveOwner(&session) && acquireMode == 0)
throw ResourceLockedException(QPID_MSG("Cannot subscribe to exclusive queue
To me that seems like a cleaner an more direct implementation of the functionality.
- Gordon Sim
On Sept. 6, 2012, 1:52 p.m., mick goulish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6943/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2012, 1:52 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
>
>
> Description
> -------
>
> browse-only queues
>
> prevent consumer from acquiring, and prevent queue from dequeing -- no change to any class except Queue, and QueueSettings.
>
>
> yes, questions:
>
> 1. is it OK to *not* change anything else. i.e. i don't see that read-credit should be affected.
>
> 2. also, i did not want to change anything except Queue -- i.e. no change to Consumer cursor. Let it think that it is consuming -- but Queue keeps messages. This might allow queue to change back and forth from browse-only to normal, if we ever care.
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1376905
> trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1376905
> trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1376905
>
> Diff: https://reviews.apache.org/r/6943/diff/
>
>
> Testing
> -------
>
> passed make check except for 1 HA test, on recent trunk
> Create queue with "browse-only" arg, load 10 messages, and then see that consumer can 'consume' any number of time, but same messages are always there.
>
>
> Here are some scripts for testing:
>
> ---- script: start_broker -------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> rm -f ${LOG}
>
>
> echo "Starting broker..."
> ${QPIDD} \
> --auth=no \
> --log-enable info+ \
> --log-to-file ${LOG} \
> --log-hires-timestamp=yes \
> --log-source=yes \
> -p ${PORT} \
> -d
> echo "Broker started."
> ---- end script -----------------
>
>
> ---- script: load_messages -------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> echo "============================================" >> ${LOG}
> echo "Loading the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Loading the messages"
>
> sleep 3
>
>
> # address was: 'q; {create: always, node:{ type: queue }}'
>
> COUNT=10
> echo "Sending $COUNT messages..."
> ${SPOUT} \
> -c 10 \
> --broker "localhost:${PORT}" \
> --content "All Work and No Play Make Mick a Dull Boy" \
> 'q; {create: always, node:{type:queue , x-declare:{arguments:{"browse-only":1}}}}'
>
> echo " "
> echo "Done sending messages."
> echo " "
> ---- end script ------------------
>
>
> ---- script: consume -------------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
>
> echo "============================================" >> ${LOG}
> echo "Consuming the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Consuming the messages"
>
> sleep 5
>
> ${DRAIN} \
> --broker 'localhost:5801' \
> 'q;'
>
>
> echo "done."
> ---- end script ------------------
>
>
> ---- script: browse --------------
> #! /bin/bash
>
>
> PORT=5801
> TRUNK=/home/mick/redhat/trunk
> QPIDD=${TRUNK}/qpid/cpp/src/qpidd
> QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
> SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
> DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
> LOG=/tmp/qpidd.log
>
> echo "============================================" >> ${LOG}
> echo "Browsing the messages" >> ${LOG}
> echo "============================================" >> ${LOG}
> echo "Browsing the messages"
>
> sleep 5
>
> ${DRAIN} \
> --broker 'localhost:5801' \
> 'q; {mode: browse}'
>
>
> echo "done."
> ---- end script ------------------
>
>
> Thanks,
>
> mick goulish
>
>
Re: Review Request: browse-only queues
Posted by mick goulish <mi...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6943/
-----------------------------------------------------------
(Updated Sept. 6, 2012, 1:52 p.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
Description
-------
browse-only queues
prevent consumer from acquiring, and prevent queue from dequeing -- no change to any class except Queue, and QueueSettings.
yes, questions:
1. is it OK to *not* change anything else. i.e. i don't see that read-credit should be affected.
2. also, i did not want to change anything except Queue -- i.e. no change to Consumer cursor. Let it think that it is consuming -- but Queue keeps messages. This might allow queue to change back and forth from browse-only to normal, if we ever care.
Diffs
-----
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1376905
trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1376905
trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1376905
Diff: https://reviews.apache.org/r/6943/diff/
Testing
-------
passed make check except for 1 HA test, on recent trunk
Create queue with "browse-only" arg, load 10 messages, and then see that consumer can 'consume' any number of time, but same messages are always there.
Here are some scripts for testing:
---- script: start_broker -------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
rm -f ${LOG}
echo "Starting broker..."
${QPIDD} \
--auth=no \
--log-enable info+ \
--log-to-file ${LOG} \
--log-hires-timestamp=yes \
--log-source=yes \
-p ${PORT} \
-d
echo "Broker started."
---- end script -----------------
---- script: load_messages -------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
echo "============================================" >> ${LOG}
echo "Loading the messages" >> ${LOG}
echo "============================================" >> ${LOG}
echo "Loading the messages"
sleep 3
# address was: 'q; {create: always, node:{ type: queue }}'
COUNT=10
echo "Sending $COUNT messages..."
${SPOUT} \
-c 10 \
--broker "localhost:${PORT}" \
--content "All Work and No Play Make Mick a Dull Boy" \
'q; {create: always, node:{type:queue , x-declare:{arguments:{"browse-only":1}}}}'
echo " "
echo "Done sending messages."
echo " "
---- end script ------------------
---- script: consume -------------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
echo "============================================" >> ${LOG}
echo "Consuming the messages" >> ${LOG}
echo "============================================" >> ${LOG}
echo "Consuming the messages"
sleep 5
${DRAIN} \
--broker 'localhost:5801' \
'q;'
echo "done."
---- end script ------------------
---- script: browse --------------
#! /bin/bash
PORT=5801
TRUNK=/home/mick/redhat/trunk
QPIDD=${TRUNK}/qpid/cpp/src/qpidd
QPID_CONFIG=${TRUNK}/qpid/tools/src/py/qpid-config
SPOUT=${TRUNK}/qpid/cpp/examples/messaging/spout
DRAIN=${TRUNK}/qpid/cpp/examples/messaging/drain
LOG=/tmp/qpidd.log
echo "============================================" >> ${LOG}
echo "Browsing the messages" >> ${LOG}
echo "============================================" >> ${LOG}
echo "Browsing the messages"
sleep 5
${DRAIN} \
--broker 'localhost:5801' \
'q; {mode: browse}'
echo "done."
---- end script ------------------
Thanks,
mick goulish