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