[
https://issues.apache.org/jira/browse/QPID-3076?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13025814#comment-13025814
]
[email protected] commented on QPID-3076:
-----------------------------------------------------
bq. On 2011-04-27 09:51:57, Gordon Sim wrote:
bq. > /trunk/qpid/cpp/docs/man/qpidd.1, line 44
bq. > <https://reviews.apache.org/r/512/diff/4/?file=17336#file17336line44>
bq. >
bq. > This is probably unintnetional? I suspect it may just be a result of
the build process changing since you created the diff?
bq.
bq. Alan Conway wrote:
bq. You probably need to re-bootstrap and configure to get all the
makefiles update.
Totally unintentional - apparently "make" regenerated it. I updated to the
latest trunk, re-ran ./bootstrap && ./configure, and I still end up getting a
modified qpidd.1.
Definitely not part of the patch - I will remove it.
bq. On 2011-04-27 09:51:57, Gordon Sim wrote:
bq. > /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp, line 165
bq. > <https://reviews.apache.org/r/512/diff/4/?file=17341#file17341line165>
bq. >
bq. > Just curious about removing this check... was it required? Is there
ever a case when the payload is not set? If so are we sure it is safe?
I honestly don't know where that check came from. I removed it because all the
other implementations derived from QueueObserver do not check payload, neither
does the Queue code itself. I'd rather leave it out to be consistent with the
other QueueObserver impls, and avoid a needless check in the fastpath.
bq. On 2011-04-27 09:51:57, Gordon Sim wrote:
bq. > /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h, line 59
bq. > <https://reviews.apache.org/r/512/diff/4/?file=17342#file17342line59>
bq. >
bq. > I'm not hugely keen on these changes to QueueObserver. I fully
anticipate that the interface will need to be adapted and expanded. However I'd
like - if possible - to keep cluster specific concerns out of it.
bq. >
bq. > That is a big 'if' however. One suggestion would be to create a
sub-class of QueueObserver - e.g. StatefulQueueObserver - that included these
changes. You could then create an instance of that if desired/required. The
UpdateClient could also then cast to that interface to determine if there was
relevant state.
bq. >
bq. > I'd like at some point to move to a queue factory and have a
specific specialisation of that for clustered brokers that could encapsulate
any cluster specific shenanigans.
Agreed - those new methods are only for cluster support, which makes them
specialized in a manner that won't be applicable to all QueueObservers. I
like your suggestion of a sub-class specifically for cluster support. How
about I refactor this patch to take that approach?
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/512/#review573
-----------------------------------------------------------
On 2011-04-26 17:04:03, Kenneth Giusti wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/512/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-04-26 17:04:03)
bq.
bq.
bq. Review request for Alan Conway and Gordon Sim.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Hi Alan,
bq.
bq. Here's a patch that should give you an idea of the direction I'm going in
- could be completely wrong here so I don't want to go too far without checking
with you.
bq.
bq. From your notes, I've modified a bit those items I *think* we need to
replicate based on the changes I've made to increase performance (some items
have been removed). Here's what I think we need to cover:
bq.
bq. Replicate:
bq.
bq. For each Queue:
bq. If Queue has a QueueFlowLimit configured:
bq. QueueFlowLimit::flowStoppedCount
bq. QueueFlowLimit::index set of msg sequence numbers that are
pending.
bq.
bq. For each msg in the QueueFlowLimit::index set:
bq. Allocate a SessionState::IncompleteIngressMsgXfer context
bq. Attach a SessionState::IncompleteIngressMsgXfer context to
messsage's AsyncCompletion callback
bq. Force AsyncCompletion::completion count to 1 (??? or more
???)
bq.
bq. For each SessionState:
bq. Replicate the vector of pendingExecutionSyncs SequenceNumbers
bq. Add an AsyncCommandCompleter context
bq. Replicate the vector of completedMsgs
bq. If vector of completedMsgs > 0:
bq. Scheduled requestIOProcessing for callback ????
bq.
bq. Let me know what you think, thanks.
bq.
bq.
bq. This addresses bug QPID-3076.
bq. https://issues.apache.org/jira/browse/QPID-3076
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/docs/man/qpidd.1 1096739
bq. /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1096739
bq. /trunk/qpid/cpp/src/qpid/broker/Queue.h 1096739
bq. /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1096739
bq. /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1096739
bq. /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1096739
bq. /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1096739
bq. /trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp 1096739
bq. /trunk/qpid/cpp/src/qpid/cluster/Connection.h 1096739
bq. /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1096739
bq. /trunk/qpid/cpp/src/qpid/cluster/UpdateClient.h 1096739
bq. /trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1096739
bq. /trunk/qpid/cpp/src/tests/QueueFlowLimitTest.cpp 1096739
bq. /trunk/qpid/cpp/src/tests/cluster_tests.py 1096739
bq. /trunk/qpid/cpp/src/tests/queue_flow_limit_tests.py 1096739
bq. /trunk/qpid/cpp/xml/cluster.xml 1096739
bq.
bq. Diff: https://reviews.apache.org/r/512/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Barely compiles.
bq.
bq.
bq. Thanks,
bq.
bq. Kenneth
bq.
bq.
> Enable producer flow control (QPID-2935) in a cluster
> -----------------------------------------------------
>
> Key: QPID-3076
> URL: https://issues.apache.org/jira/browse/QPID-3076
> Project: Qpid
> Issue Type: New Feature
> Components: C++ Clustering
> Affects Versions: 0.9, 0.10
> Reporter: Alan Conway
> Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: QPID-3076-2.patch
>
>
> QPID-2935 adds producer flow control to the broker, but it is disabled in a
> cluster. Flow control adds additional broker state that must be replicated in
> a cluster in order to maintain cluster consistency. Add this replication and
> enable flow control in a cluster.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
