subject:"\[jira\] \[Commented\] \(QPID\-3079\) message.accept command should be completed on a per\-dequeue basis"

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2013-07-12 Thread Ken Giusti (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13707036#comment-13707036
 ] 

Ken Giusti commented on QPID-3079:
--

It may no longer apply, given the work done by Alan on HA and the async store 
interface work, this may no longer be an issue.

I'd defer to Alan Conway - I seem to recall this issue was raised in order to 
facilitate new HA.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: Future
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to be called up to N times. The store thread could
> call notifyCompleted for the first queue before the connection thread
> has called enqueueAsync for the second queue, and so on.
> *** No asynchronous completion for message.accept
> We do not currently delay completion of message.accept until the
> message is deleted from the async store. This could cause duplicate
> delivery if the broker crashes after completing the message but 
> before it is removed from store.
> There is code in PersistableMessage to maintain a counter for dequeues
> analogous to to the async enqueue code but this is incorrect. 
> Completion of transfer is triggered when all enqueues for a message are 
> complete.
> Completion of accept is triggered for *each* dequeue from a queue 
> independently.
> Furthermore a single accept can reference many messages, so it can't be 
> associated with a message.
> ** New requirements
> The new cluster design will need to participate in async completion, e.g.
> an accept cannot be comlpeted until the message is 
> - removed from store (if present) AND
> - replicated to the cluster (if present) as dequeued
> The new cluster also needs to asynchronously complete binding commands
> (declare, bind, delete) when they are replicated to the cluster.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2013-07-11 Thread Justin Ross (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13706355#comment-13706355
 ] 

Justin Ross commented on QPID-3079:
---

Ken, what should happen to this issue?

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: Future
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to be called up to N times. The store thread could
> call notifyCompleted for the first queue before the connection thread
> has called enqueueAsync for the second queue, and so on.
> *** No asynchronous completion for message.accept
> We do not currently delay completion of message.accept until the
> message is deleted from the async store. This could cause duplicate
> delivery if the broker crashes after completing the message but 
> before it is removed from store.
> There is code in PersistableMessage to maintain a counter for dequeues
> analogous to to the async enqueue code but this is incorrect. 
> Completion of transfer is triggered when all enqueues for a message are 
> complete.
> Completion of accept is triggered for *each* dequeue from a queue 
> independently.
> Furthermore a single accept can reference many messages, so it can't be 
> associated with a message.
> ** New requirements
> The new cluster design will need to participate in async completion, e.g.
> an accept cannot be comlpeted until the message is 
> - removed from store (if present) AND
> - replicated to the cluster (if present) as dequeued
> The new cluster also needs to asynchronously complete binding commands
> (declare, bind, delete) when they are replicated to the cluster.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-22 Thread jirapos...@reviews.apache.org (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13053322#comment-13053322
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/
---

(Updated 2011-06-22 15:58:11.269092)


Review request for qpid, Alan Conway, Gordon Sim, Kim van der Riet, and Steve 
Huston.


Changes
---

Latest diff of branch against trunk.  Also have added Steve H. to the 
reviewer's list.

I think I've addressed all previous comments - with the exception of Alan's 
request to remove the Queue::pendingDequeueCompletions map (Sorry - I still 
don't see a simpler way of getting back at the pending Message.Transfer 
commands).

I think a change of this magnitude is best left for after 0.12


Summary
---

Modifies the broker's handling of Message.Accept to hold off the completion of 
the command until all messages related to the accept have completed dequeue.  
This particularly applies to persistent messages, as the store::dequeue() 
operation must complete before the message is considered fully dequeued.

Note this bugfix requires some changes to the broker's store module interface:  
previously, the store only identified the message when a dequeue was completed. 
 This is not enough information - the queue from which is was removed must also 
be identified (the message may be in the process of being dequeued on several 
queues at once).


This addresses bug qpid-3079.
https://issues.apache.org/jira/browse/qpid-3079


Diffs (updated)
-

  /trunk/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/Message.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/MessageStore.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/NullMessageStore.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/PersistableQueue.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/Queue.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/broker/SessionContext.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/SessionState.h 1138503 
  /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/store/MessageStorePlugin.h 1138503 
  /trunk/qpid/cpp/src/qpid/store/MessageStorePlugin.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/store/StorageProvider.h 1138503 
  /trunk/qpid/cpp/src/qpid/store/ms-clfs/MSSqlClfsProvider.cpp 1138503 
  /trunk/qpid/cpp/src/qpid/store/ms-sql/MSSqlProvider.cpp 1138503 
  /trunk/qpid/cpp/src/tests/AsyncCompletion.cpp 1138503 
  /trunk/qpid/cpp/src/tests/QueueTest.cpp 1138503 
  /trunk/qpid/cpp/src/tests/TestMessageStore.h 1138503 
  /trunk/qpid/cpp/src/tests/cluster_tests.py 1138503 

Diff: https://reviews.apache.org/r/860/diff


Testing
---

broker unit tests, store unit tests (modified jboss store).   Still needs to be 
vetted on non-linux, and have latest trunk merged in.


Thanks,

Kenneth



> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; }

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-22 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13053305#comment-13053305
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

bq.  On 2011-06-22 13:37:11, Gordon Sim wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h, line 
135
bq.  > 
bq.  >
bq.  > What's the point of this empty method? Can it just be removed?

Good catch - this patch obsoletes that method.  I'll remove it.

- Kenneth

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review865
---

On 2011-06-16 15:25:17, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-16 15:25:17)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to be called up to N times. The store thread could
> call notifyCompleted for the first queue before the connection thread
> has called enqueueAsync for the second queue, and so on.
> *** No asynchronous completion for message.accept
> We do not currently delay completion of message.accept until the
> message is deleted from the async store. This could cause duplicate
> delivery if the broker crashes after completing the message but 
> before it is removed from store.
>

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-22 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13053252#comment-13053252
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review865
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h

What's the point of this empty method? Can it just be removed?

- Gordon

On 2011-06-16 15:25:17, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-16 15:25:17)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to be called up to N times. The store thread could
> call notifyCompleted for the first queue before the connection thread
> has called enqueueAsync for the second queue, and so on.
> *** No asynchronous completion for message.accept
> We do not currently delay completion of message.accept until the
> message is deleted from the async store. This could cause duplicate
> delivery if the broker crashes after completing the message but 
> before it is removed from store.
> There is code in PersistableMessage to maintain a counter for dequeues
> analogous to to the async enqueue code but this is incorrect. 
> Completion of transfer is triggered

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-20 Thread jirapos...@reviews.apache.org (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13052155#comment-13052155
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-



bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > Looking good, just a few suggestions.
bq.  > 
bq.  > Note on testing: need to run store tests as well as qpidd tests.

Yes, both store and C++ broker unit tests are passing.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp, line 116
bq.  > 
bq.  >
bq.  > Static locals not thread safe, use a normal local variable.

Removed.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp, line 125
bq.  > 
bq.  >
bq.  > What happens in the case isRedundant() == true?

the D.R. is removed from the unacked list - this check has been moved to the 
caller.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h, line 300
bq.  > 
bq.  >
bq.  > Functions are too big to inline in .h, move to .cpp.

done.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h, line 310
bq.  > 
bq.  >
bq.  > Suggest a change here to make it more flexible and type safe:
bq.  > 
bq.  > registerCallback(boost::function f);
bq.  > 
bq.  > You can package up all the details in boost::bind at the call site, 
this class doesn't need to know. See comments below

Very nice - done.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp, line 665
bq.  > 
bq.  >
bq.  > static local variables are not thread safe. Use a normal local 
variable constructing an intrusive pointer is trivial.

done.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 804
bq.  > 
bq.  >
bq.  > This doesn't need to be static, and you can avoid the casting. Just 
write
bq.  > 
bq.  > void dequeueDone() { cmd->complete(); }
bq.  > 
bq.  > and see further comment at the call site below

done.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 966
bq.  > 
bq.  >
bq.  > See comments above: this can now be
bq.  > 
bq.  > 
async->registerCallback(boost::bind(&AsyncMessageAcceptCmd::dequeueDone, this))

done.


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h, line 437
bq.  > 
bq.  >
bq.  > Store the DequeueCompletion pointer on the PersistableMessage, avoid 
this map lookup.

Sorry Alan, I'm being dense here - I don't understand.  Wouldn't the 
PersistableMessage still need a container for the DequeueCompletions, as there 
may be more than one dequeue pending for that message?  I'm thinking of fanout 
- same physical PersistableMessage shared among N queues.  There may be several 
different sessions dequeuing/accepting that message at the same time, right?  
How do we 'map' (heh) back from the P.M. to the sessions pending on the 
Message.Accept completion? 


bq.  On 2011-06-17 13:19:13, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h, line 107
bq.  > 
bq.  >
bq.  > Why the change from dequeue to list?

Big performance improvement with qpid-cpp-benchmark by moving from a dequeue to 
a list.  I'm guessing because we delete entries from the middle of the list(?)


- Kenneth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review859
---


On 2011-06-16 15:25:17, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-16 15:25:17)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-17 Thread jirapos...@reviews.apache.org (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13051293#comment-13051293
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/934/
---

Review request for qpid, Gordon Sim, Kim van der Riet, and Steve Huston.


Summary
---

In order to solve QPID-3079, I'd like to make the following changes to the 
MessageStore API.

The problem with the existing api is that, when a dequeue() is completed, only 
the message is notified (via a call to PersistableMessage::dequeueComplete()).  
Since a single instance of a message may be dequeued from multiple queues 
simultaneously, this approach does not provide enough context to allow the 
broker to identify -which- dequeue is being completed.

The proposed solution is to provide a new method to the PersistableQueue class 
that is called by the store when a dequeue completes:

  virtual void dequeueComplete(const boost::intrusive_ptr&) 
= 0;

This allows the broker to identify both the queue that is the source of the 
dequeue, plus the message that was dequeued.  

I've also modified the MessageStore::dequeue() interface to take a shared_ptr 
to the queue, much like the intrusive_ptr that is currently used to pass the 
message.  This allows an asynchronous dequeue to hold a valid pointer to the 
queue until the dequeue completes.

(Gordon has suggested that I make the same changes to the 
MessageStore::enqueue() interface for symmetry - something that I'd like to do 
if there is agreement).

All changes for QPID-3079 are available on the qpid-3079 branch in svn.  The 
changes included in this review are merely the store-related modifications - I 
wanted to focus on these separately.  The full review of QPID-3079 is available 
here: https://reviews.apache.org/r/860/  - and feel free to checkout the branch 
if you'd like :)


This addresses bug qpid-3079.
https://issues.apache.org/jira/browse/qpid-3079


Diffs
-

  /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/store/MessageStorePlugin.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/store/MessageStorePlugin.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/store/StorageProvider.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/store/ms-clfs/MSSqlClfsProvider.cpp 
1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/store/ms-sql/MSSqlProvider.cpp 1124895 

Diff: https://reviews.apache.org/r/934/diff


Testing
---

I've tested these changes against the Linux-based store implementation.  I've 
only verified that the sql stores build under windows thus far - Steve, are 
there any unit tests for these stores?  If so, is this something I can run?  
Otherwise, could you try running them from the qpid-3079 branch and let me know 
if there are failures?  Thx.


Thanks,

Kenneth



> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-17 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13051064#comment-13051064
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review859
---

Looking good, just a few suggestions.

Note on testing: need to run store tests as well as qpidd tests.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h

Why the change from dequeue to list?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp

Static locals not thread safe, use a normal local variable.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp

What happens in the case isRedundant() == true?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

Functions are too big to inline in .h, move to .cpp.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

Suggest a change here to make it more flexible and type safe:

registerCallback(boost::function f);

You can package up all the details in boost::bind at the call site, this 
class doesn't need to know. See comments below

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

Store the DequeueCompletion pointer on the PersistableMessage, avoid this 
map lookup.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

static local variables are not thread safe. Use a normal local variable 
constructing an intrusive pointer is trivial.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

This doesn't need to be static, and you can avoid the casting. Just write

void dequeueDone() { cmd->complete(); }

and see further comment at the call site below

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

See comments above: this can now be

async->registerCallback(boost::bind(&AsyncMessageAcceptCmd::dequeueDone, 
this))

- Alan

On 2011-06-16 15:25:17, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-16 15:25:17)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
>

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-16 Thread jirapos...@reviews.apache.org (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13050494#comment-13050494
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/
---

(Updated 2011-06-16 15:25:17.977221)


Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.


Changes
---

Based on feedback, I've tried to clean up the Queue::dequeue() interface and 
per-dequeue callback handling.

In addition, I've done some performance tuning (results have been uploaded to 
QPID-3079 as a comment).  At this point, the performance of this patch is on 
par or better than the baseline.


Summary
---

Modifies the broker's handling of Message.Accept to hold off the completion of 
the command until all messages related to the accept have completed dequeue.  
This particularly applies to persistent messages, as the store::dequeue() 
operation must complete before the message is considered fully dequeued.

Note this bugfix requires some changes to the broker's store module interface:  
previously, the store only identified the message when a dequeue was completed. 
 This is not enough information - the queue from which is was removed must also 
be identified (the message may be in the process of being dequeued on several 
queues at once).


This addresses bug qpid-3079.
https://issues.apache.org/jira/browse/qpid-3079


Diffs (updated)
-

  /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 

Diff: https://reviews.apache.org/r/860/diff


Testing
---

broker unit tests, store unit tests (modified jboss store).   Still needs to be 
vetted on non-linux, and have latest trunk merged in.


Thanks,

Kenneth



> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to be called up to N times. The store thread could
> call notifyCompleted for the first queue before the connection thread
> has called enqueueAsync for the second queue, and so on.
> *** No asynchronous completion for message.accept
> We do not currently delay completion of message.accept until the
> message is deleted from the async store. This could cause duplicate
> delivery if the broker crashes after completing the message but 
> before it is removed from store.
> There is code in PersistableMessage to maintain a counter for dequeues
> analogous to to the async enqueue code but this is incorrect. 
> Completion of transfer is triggered when all enqueues for a message are 
> complete.
> Completion of accept is triggered for *each* dequeue from a queue 
> independently.
> Furthermore a single accept can reference many messages, so it can't be 
> associated with a message.
> ** New requirements
> The new cluster design will need to participate in async completion, e.g.
> an accept cannot be comlpeted until

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-16 Thread Ken Giusti (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13050485#comment-13050485
 ] 

Ken Giusti commented on QPID-3079:
--

I've done some performance tuning using qpid-cpp-benchmark with various values 
for --ack-frequency.  Both durable and non-durable performance was tested.  
After some tuning, I've got the following results, with baselines to compare 
against:


-- NON-DURABLE -



r1124895

qpid-cpp-benchmark --ack-frequency=100 --repeat=20 --summarize
send-tp recv-tp l-min   l-max   l-avg
31579   30824   0.3379.82   6.02
31574   31543   0.3935.47   6.47
31264   31248   0.4245.53   5.20
30924   30882   0.3647.00   5.91
31341   30941   0.3441.58   7.09
31451   31041   0.3474.01   10.80
31267   30873   0.3447.16   6.12
31405   31376   0.3445.09   5.27
31475   31384   0.4031.62   7.30
31807   31400   0.3841.03   6.04
31591   31567   0.3343.63   7.22
30833   30104   0.3778.80   7.59
30001   29633   0.33129.94  51.55
31465   31435   0.3522.21   5.30
32054   31633   0.4141.82   5.63
30917   30535   0.3649.37   6.00
31657   31607   0.3426.91   5.81
30983   30242   0.3479.37   6.60
31302   31223   0.3964.31   11.39
30897   30496   0.4876.67   9.52
20 runs:  stp=31289.349609 rtp=30999.349609 lmin=0.367000 lmax=55.066998 
lavg=9.141500

qpid-cpp-benchmark --ack-frequency=10 --repeat=20 --summarize
send-tp recv-tp l-min   l-max   l-avg
27617   27599   0.3747.48   6.62
28662   25858   0.33475.21  308.42
28486   28164   0.36199.83  51.70
28910   27407   0.34286.74  115.51
28311   28295   0.39114.41  18.16
28272   27452   0.51138.65  26.97
29066   27801   0.39334.59  130.22
28872   25572   0.34498.66  297.83
29657   26966   0.35427.10  148.45
28235   28225   0.3634.79   7.49
28800   28467   0.4043.37   5.48
29142   28158   0.27157.72  34.48
29466   26616   0.39412.66  174.74
29470   28732   0.3896.42   14.24
29384   28706   0.37101.08  10.33
28221   27580   0.35165.00  58.94
28743   28087   0.3389.09   20.49
28316   28295   0.37113.48  21.90
29420   29342   0.3449.45   7.67
28442   28014   0.3996.83   10.81
20 runs:  stp=28774.599609 rtp=27766.800781 lmin=0.366500 lmax=194.128021 
lavg=73.522491

qpid-cpp-benchmark --ack-frequency=1 --repeat=20 --summarize
send-tp recv-tp l-min   l-max   l-avg
24052   19697   1.791611.97 945.29
24245   19730   0.601622.14 1018.01
22659   19505   0.361541.25 958.42
23262   19665   0.431354.04 728.93
24574   20347   0.411406.91 692.94
25078   20051   0.341624.63 980.73
23622   19594   0.401444.34 828.41
22150   19778   0.351036.81 529.26
23041   19903   0.431319.29 795.41
23851   19780   0.331477.10 848.00
25947   20146   0.451753.31 1122.03
23218   19653   0.401361.62 817.18
22808   19721   0.341226.58 634.30
22528   19809   0.341045.04 425.75
23528   19878   0.361376.19 771.57
26537   20370   0.441891.97 1281.50
23858   19822   0.361571.48 968.33
23362   19867   0.341405.03 775.50
24934   20013   0.411628.51 1029.71
23728   19856   1.381450.68 832.09
20 runs:  stp=23849.099609 rtp=19859.25 lmin=0.513000 lmax=1457.444336 
lavg=849.168091


QPID-3079
-

qpid-cpp-benchmark --ack-frequency=100 --repeat=20 --summarize
send-tp recv-tp l-min   l-max   l-avg
31953   31932   0.4733.19   7.37
32743   32676   0.3922.82   5.72
32115   31313   0.3580.06   5.29
32317   31506   0.3479.88   5.30
32508   32037   0.3345.48   6.12
32021   31604   0.4245.56   5.17
31616   31590   0.4085.72   21.08
32276   31873   0.3747.40   5.23
32177   31637   0.3953.38   5.75
32258   32251   0.3441.31   5.24
32597   32549   0.3939.13   7.92
32712   32703   0.3925.52   5.38
32291   32261   0.4141.70   4.76
32136   32125   0.3241.13   7.37
32194   32144   0.3443.94   7.06
32225   31792   0.3550.12   5.77
31997   31578   0.3346.27   5.43
31576   31184   0.3344.37   5.58
30006   29667

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-13 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13048544#comment-13048544
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

bq.  On 2011-06-09 20:51:15, Alan Conway wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 799
bq.  > 
bq.  >
bq.  > Do we need the uniqueness logic? How often are we likely to see 
duplicate queues in the list? Flushing is an idempotent operation, so flushing 
twice would be an error.

would _not_ be an error I meant to say

- Alan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review795
---

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a clie

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-13 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13048540#comment-13048540
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

bq.  On 2011-06-10 15:28:20, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp, line 1229
bq.  > 
bq.  >
bq.  > "All  you need to know is when they all complete" - not true: we 
need to know when _each_ dequeue completes.  This is not like the enqueue case 
- we need to known when each dequeue is complete as the Message.Accept may be 
only interested in that one particular dequeue event.
bq.  > 
bq.  > Which is what QPID-3079 is specifically trying to solve.

Right but you can still count rather than holding a set/list. If the 
message.accept is interested in N completions it can just count down to 0 then 
complete. I'm wary of adding new dynamic data structures on the critical path.

- Alan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review801
---

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/Q

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-10 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047246#comment-13047246
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

bq.  On 2011-06-10 15:28:20, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h, line 64
bq.  > 
bq.  >
bq.  > Ok, discussed this with Kim - his understanding of this pointer is 
that it is owned by the persistent queue, and the queue should be responsible 
for calling delete on it.   If that is the case, then there is a bug in that 
the store is referencing this context after the queue is destroyed.
bq.  > 
bq.  > I will work with Kim on this to better understand the root cause and 
fix it properly.

The pointer is opaque from the brokers perspective. It is entirely under the 
store's control, including whether to even use it. I think it is an error that 
the PersistableQueue is currently deleting that pointer. The store should do 
any cleanup as a result of the explicit queue deletion.

- Gordon

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review801
---

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-10 Thread jirapos...@reviews.apache.org (JIRA)

[
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047244#comment-13047244
]

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review801
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h

Ok, discussed this with Kim - his understanding of this pointer is that it
is owned by the persistent queue, and the queue should be responsible for
calling delete on it. If that is the case, then there is a bug in that the
store is referencing this context after the queue is destroyed.

I will work with Kim on this to better understand the root cause and fix it
properly.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

"All you need to know is when they all complete" - not true: we need to
know when _each_ dequeue completes. This is not like the enqueue case - we
need to known when each dequeue is complete as the Message.Accept may be only
interested in that one particular dequeue event.

Which is what QPID-3079 is specifically trying to solve.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Because of this: https://bugzilla.redhat.com/show_bug.cgi?id=698721

Store will not complete the dequeue for up to one second *if* it doesn't
have enough work to perform immediately.

Broker needs to force fast completion on Execution.Sync - it does this by
issuing a flush() on the command.

And that's why a simple counter won't work here - we need to track the
queues that are in the process of dequeuing specifically in case a flush is
called.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Which is what I'm trying to do - at least at the SemanticState level (with
the message accept context). Its just that the message.accept context has to
be able to track N outstanding dequeues in order to know when it is complete
(could be a counter), or if the SessionState flushes the command (counter not
sufficient - it needs list of messages or queues to flush).

Let me step back a bit - there are three layers of abstraction introduced
by this patch:

1) SessionState/SessionContext needs to allow for async commands with an
abstract api (i.e. - SessionState doesn't care about the command specifics, it
just needs to know when it is complete, and be able to flush the command if
needed) See changes in SessionContext.

2) SemanticState uses one of these abstract "async command contexts" for
the Message.Accept command. This context must monitor a _set_ of outstanding
dequeues in order to 1) know when the command is complete, and 2) which
queues/messages need flushing should SessionState requires a flush.

3) The Queue needs to provide a callback when a particular message is
dequeued (not when all dequeues complete for a given message). The callback
needs to identify the queue and the particular message that has completed
dequeue.

#3 could be tracked by the message instead, but we still need to know when
-each- dequeue completes, so then the message would have to track each of its
concurrent dequeue's state (and target queue). I think that's exactly what
I've got Queue doing now anyways, and I think representing that in the Queue is
more "natural" - but that's more my artistic opinion :)

- Kenneth

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-10 Thread jirapos...@reviews.apache.org (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047165#comment-13047165
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-



bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp, line 685
bq.  > 
bq.  >
bq.  > The P.M. _could_ be being dequeued simultaneously from multiple 
queues.  Won't that just move the map from the queue to the P.M.?

I'm suggesting "pmsg->setCompletionCallback(callback)". No map. 
setCompletionCallback() may  be called concurrently in multiple deques but that 
does't require a map, just a lock. I think the completion context belongs on 
the message.


bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp, line 1229
bq.  > 
bq.  >
bq.  > Perhaps - I still think we'd need to track multiple outstanding 
callbacks for a single message instance.

Why? All  you need to know is when they all complete. A simple atomic counter 
will do the job. I think we should be aiming to avoid multiple heap allocations 
per message wherever we can. Ideally I'd like to see the completion context 
boiled down to a few counters and allocated in-line as part of the Message. The 
interfaces are good, but the implementation does a lot of dynamic allocation. I 
suspect that will hurt performance.


bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 799
bq.  > 
bq.  >
bq.  > Actually, it's very likely that all the messages that are in a 
Message.accept sequence are on the same queue.  I noticed that during debug - a 
flush of a single message.accept command resulted in multiple flush calls to 
the same queue.  According to Kim, multiple flushes to the same queue are 
ignored, so I could drop this, but that may not be true for store in general.

Why do we need a flush() on the AsyncMessageAcceptCommand?


bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 862
bq.  > 
bq.  >
bq.  > There may be multiple pending dequeues for a given message, so we 
would need some sort of container anyway.

I'd prefer a counter to a container.


bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 876
bq.  > 
bq.  >
bq.  > I'm not really happy with this approach, but I can't think of a way 
to track N outstanding dequeue operations without using some dynamic container.
bq.  > 
bq.  > Each callback is actually a "functor" object that contains state 
needed to map the dequeue back to the pending Message.Accept.  This state impl 
is hidden from the Message object - I'd hate to have to expose command-specific 
state into the messages (but I'm probably not thinking abstractly enough at 
this point :P )

As long as the command-specific state is encapsulated in its own class, I think 
it's fine to attach it to the message. In fact I think we need a general 
solution for "feature x has per-message state"  that avoids keeping a map of 
all the messages. Not for this patch though :)


bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp, line 930
bq.  > 
bq.  >
bq.  > The behaviour of the unacked list should be the same as it is now.  
I'm more worried about the completion of the Message.Accept cmd, which will 
happen after the store has completed dequeue.  That will be a visible change in 
behaviour and I don't think clustering will like it.

Completion of the accept is sent in a control, not a command. So it doesn't 
disturb the command-ids of outgoing transfers. Need to review  if there's any 
potential for trouble with the rest of the Session state. Gordon?


- Alan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review797
---


On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Revie

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-10 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047088#comment-13047088
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

bq.  On 2011-06-10 00:22:59, Kenneth Giusti wrote:
bq.  > /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h, line 64
bq.  > 
bq.  >
bq.  > The pointer is actually _shared_ between the MessageStoreImpl and 
the Queue.  My changes caused a bug where, if the queue was deleted _before_ a 
pending dequeue completed, then the queue was destroyed during the 
MessageStoreImpl destructor, which was using the pointer to do the flush.  I 
think explicitly making it shared is the right thing to do.

That still seems like an issue for the store implementation to handle. The 
broker never touches this external store, its simply a convenience offered to 
the store impl to avoid it having to lookup extra state it wants for the queue.

- Gordon

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review797
---

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/b

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046936#comment-13046936
]

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review797
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h

See reply in SemanticState.cpp comments.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp

You're correct - I'll give it a try.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp

The old code _may_ be in error - it added the queue on asyncEnqueue *and*
asyncDequeue, and nowhere do I see it -remove- it!

I *think* we need to add on enqueue, and remove when dequeueComplete(), but
I could be wrong

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h

The pointer is actually _shared_ between the MessageStoreImpl and the
Queue. My changes caused a bug where, if the queue was deleted _before_ a
pending dequeue completed, then the queue was destroyed during the
MessageStoreImpl destructor, which was using the pointer to do the flush. I
think explicitly making it shared is the right thing to do.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h

That's a good point - let me investigate making the store api a bit more
symmetrical with this change.

Will definitely get input from Steve - thanks for the head's up!

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

I agree it appears overly complex, but it addresses a couple of problems:

1) "lazy" allocation of tracking context. The caller does not know if the
dequeue is sync or async at the point of call. The factory callback allows the
caller to allocate context prior to needing to service the completion (and in
the current thread).

2) Less racy - it guarantees that the completion will be held off until the
factory method returns.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

Will do.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

See the above reply to Alan. I don't think the return code is used
anywhere - it was bool originally. But relying on the return code is racy.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

The P.M. _could_ be being dequeued simultaneously from multiple queues.
Won't that just move the map from the queue to the P.M.?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

Perhaps - I still think we'd need to track multiple outstanding callbacks
for a single message instance.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Actually, it's very likely that all the messages that are in a
Message.accept sequence are on the same queue. I noticed that during debug - a
flush of a single message.accept command resulted in multiple flush calls to
the same queue. According to Kim, multiple flushes to the same queue are
ignored, so I could drop this, but that may not be true for store in general.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

There may be multiple pending dequeues for a given message, so we would
need some sort of container anyway.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

I'm not really happy with this approach, but I can't think of a way to
track N outstanding dequeue operations without using some dynamic container.

Each callback is actually a "functor" object that contains state needed to
map the dequeue back to the pending Message.Accept. This state impl is hidden
from the Message object - I'd hate to have to expose command-specific state
into the messages (but I'm probably not thinking abstractly enough at this
point :P )

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

The behaviour of the unacked list should be the same as it is now. I'm
more worried about the completion of the Message.Accept cmd, which will happen
after the store has completed dequeue. That will

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046797#comment-13046797
]

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review795
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h

Why indirection via a factory? Why not just pass callback pointer?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp

Why can't we do a map lookup here by queue name? Why the search by
PersistenceId?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp

Looks like meaning of synclist has been reversed here - old code added to
list on dequeue, new code removes from list. Is that right?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

why the indirection via a factory?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

Overloading operator () is just confusing. Give it a function name.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

Do we need a map here? Could we store the callback directly on the
PersistentMessage?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

This would be simpler if the callback was stored on the message rather than
in a map

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Do we need the uniqueness logic? How often are we likely to see duplicate
queues in the list? Flushing is an idempotent operation, so flushing twice
would be an error.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Could this callback be attached to the message instead, avoiding heap
allocation?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

I think that using a factory to avoid construction of the callback is more
expensive than always constructing the calback, especially if the callback is
made part of the Message so there's no heap allocation.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Possible cluster issue. The cluster replicates the unacked list. Is this
change predictable for the cluster?

- Alan

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.
bq. ---
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/860/
bq. ---
bq.
bq. (Updated 2011-06-08 20:31:05)
bq.
bq.
bq. Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.
bq.
bq. Summary
bq. ---
bq.
bq. Modifies the broker's handling of Message.Accept to hold off the
completion of the command until all messages related to the accept have
completed dequeue. This particularly applies to persistent messages, as the
store::dequeue() operation must complete before the message is considered fully
dequeued.
bq.
bq. Note this bugfix requires some changes to the broker's store module
interface: previously, the store only identified the message when a dequeue
was completed. This is not enough information - the queue from which is was
removed must also be identified (the message may be in the process of being
dequeued on several queues at once).
bq.
bq.
bq. This addresses bug qpid-3079.
bq. https://issues.apache.org/jira/browse/qpid-3079
bq.
bq.
bq. Diffs
bq. -
bq.
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h
1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp
1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp
1124895
bq.

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046784#comment-13046784
]

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/820/#review747
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h

why virtual inheritance?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h

Why clone?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h

yuck

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp

Why iterate here rather than doing a map lookup?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

Why the indirection via a factory?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

suggest named function rather than overloading operator ()

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

New map, performance?
Safe to have PersistableMessage*?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

Possible cluster issue: map sorted by pointers.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

Instead of a map, can we put a shared_ptr in
PersistableMessge?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp

Need a less obscure error message.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Can we use a count instead of a map?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Or:

assert(unique);
(void)unique; // no op but takes care of unused variable warnings.

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

Replace these 3 lines with: pending.erase(id)

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

trace, not error

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

We don't need to do anything in the TX case?

/branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp

not sure

- Alan

On 2011-06-01 20:19:53, Kenneth Giusti wrote:
bq.
bq. ---
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/820/
bq. ---
bq.
bq. (Updated 2011-06-01 20:19:53)
bq.
bq.
bq. Review request for Alan Conway and Gordon Sim.
bq.
bq.
bq. Summary
bq. ---
bq.
bq. Here's my attempt to implement a solution to qpid-3079 - with an eye
towards allowing async completion of various commands. I've integrated the
existing async Message.Transfer completion code to use this new design.
bq.
bq. The implementation seems too "codey" - I had hoped that this would've
resulted in a simpler implementation, but I've found that C++ is a harsh
mistress.
bq.
bq. I'd like some early feedback - particularly suggestions for reducing the
amount of code/simplifying the design if possible.
bq.
bq. There are two major functional changes involved:
bq.
bq. 1) changes to the Queue:dequeue() method to allow a callback if the
dequeue is asynchronous. Since the caller does not know ahead of time whether
the dequeue is sync or async, Queue::dequeue() now takes a functor parameter
supplied by the caller. The purpose of the functor is to allocate a context to
track the dequeue completion only if necessary. See Queue.h/cpp and the
SemanticState::accepted() changes for detail
bq.
bq. 2) - and this is the one aspect I'd like to improve - changes to the
SessionState/SessionContext/SemanticState interface to allow a command to
complete asynchronously. My understanding of the code is that the
SessionContext defines an abstract interface to the SessionState. The
SemanticState uses this interface for getting session-related information.
bq.
bq. The changes involve creating an abstract class within the SessionContext
class that represents the context for a

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046659#comment-13046659
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review793
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h

This feels a little clumsy to me. Is the return value ever used anywhere? 
Why is there a factory to a callback rather than just a callback?

- Gordon

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> **

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046655#comment-13046655
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review792
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h

It would be nice to make the mechanism by which the store notified 
completion of enqueue more consistent with the dequeue.

So if this dequeueComplete() mechanism is used on dequeue, maybe modify 
enqueue to also use a PersistableQueue::enqueueComplete() method?

The asynchronous store interface is already very adhoc and confusing. 
Anything we can do to avoid further deterioration is worth it.

These changes would affect the windows store included in Qpid. Steve Huston 
should be added to the review list. We should also make some attempt at 
documenting the store interface wrt synchronous v asynchronous behaviour.

- Gordon

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046647#comment-13046647
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review791
---

/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h

What's the motivation for changing from regular to shared pointer here? 
This was originally introduced as opaque store specific context; could the 
changes be hidden behind the interface?

(Not a big issue, just curious).

- Gordon

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046540#comment-13046540
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review789
---

/branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp

change this to us a synchronous session [e.g.  
fix.session.messageFlush(arg::destination=sub.getName())] or sync() on the 
resultof it. That will ensure that all messages have been delivered when the 
call returns and you should not then even need a timeout for the following 
get() call.

/branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp

Might need to increase the timeout here if you are seeing occasional 
errors. You are waiting for the messagAccept() above to be processed but can't 
rely on syncing on the client session. With valgrind on etc I guess under load 
it could take a while to process the range of acks...

- Gordon

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPI

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-09 Thread jirapos...@reviews.apache.org (JIRA)

[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046530#comment-13046530
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/#review788
---

/branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp

Special note:  I'm not very familiar with these client API's - is what I'm 
doing here correct?  Occasionally, I see q.get() timeout when under load (?)

/branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp

Same point here - for some reason, very occasionally I see the pop() 
timeout.  Are my subscription settings and use of these client APIs correct for 
my intent?

- Kenneth

On 2011-06-08 20:31:05, Kenneth Giusti wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/860/
bq.  ---
bq.  
bq.  (Updated 2011-06-08 20:31:05)
bq.  
bq.  
bq.  Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  Modifies the broker's handling of Message.Accept to hold off the 
completion of the command until all messages related to the accept have 
completed dequeue.  This particularly applies to persistent messages, as the 
store::dequeue() operation must complete before the message is considered fully 
dequeued.
bq.  
bq.  Note this bugfix requires some changes to the broker's store module 
interface:  previously, the store only identified the message when a dequeue 
was completed.  This is not enough information - the queue from which is was 
removed must also be identified (the message may be in the process of being 
dequeued on several queues at once).
bq.  
bq.  
bq.  This addresses bug qpid-3079.
bq.  https://issues.apache.org/jira/browse/qpid-3079
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 
1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
bq./branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
bq./branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 
bq.  
bq.  Diff: https://reviews.apache.org/r/860/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  broker unit tests, store unit tests (modified jboss store).   Still needs 
to be vetted on non-linux, and have latest trunk merged in.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Kenneth
bq.  
bq.

> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-08 Thread jirapos...@reviews.apache.org (JIRA)


[ 
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13046195#comment-13046195
 ] 

jirapos...@reviews.apache.org commented on QPID-3079:
-


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/860/
---

Review request for qpid, Alan Conway, Gordon Sim, and Kim van der Riet.


Summary
---

Modifies the broker's handling of Message.Accept to hold off the completion of 
the command until all messages related to the accept have completed dequeue.  
This particularly applies to persistent messages, as the store::dequeue() 
operation must complete before the message is considered fully dequeued.

Note this bugfix requires some changes to the broker's store module interface:  
previously, the store only identified the message when a dequeue was completed. 
 This is not enough information - the queue from which is was removed must also 
be identified (the message may be in the process of being dequeued on several 
queues at once).


This addresses bug qpid-3079.
https://issues.apache.org/jira/browse/qpid-3079


Diffs
-

  /branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoverableQueue.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/RecoveryManagerImpl.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/SemanticState.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionContext.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.h 1124895 
  /branches/qpid-3079/qpid/cpp/src/qpid/broker/SessionState.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/tests/AsyncCompletion.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/tests/QueueTest.cpp 1124895 
  /branches/qpid-3079/qpid/cpp/src/tests/TestMessageStore.h 1124895 

Diff: https://reviews.apache.org/r/860/diff


Testing
---

broker unit tests, store unit tests (modified jboss store).   Still needs to be 
vetted on non-linux, and have latest trunk merged in.


Thanks,

Kenneth



> message.accept command should be completed on a per-dequeue basis
> -
>
> Key: QPID-3079
> URL: https://issues.apache.org/jira/browse/QPID-3079
> Project: Qpid
>  Issue Type: Bug
>  Components: C++ Broker
>Affects Versions: 0.8, 0.9
>Reporter: Ken Giusti
>Assignee: Ken Giusti
> Fix For: 0.11
>
> Attachments: proposal.txt
>
>
> ** Overview
> Asynchronous completion means that command execution is initiated in one 
> thread
> (a client connection thread) and completed in a different thread.
> When the async store is loaded, message.transfer commands are
> completed by a store thread that does the async write.
> ** Issues with asynchronous completion code as of revision r1029686
> *** Not really asynchronous
> IncompleteMessageList::process blocks the connection thread till all
> outstanding async commands (messages) for the session are complete.
> With the new cluster, this could deadlock since it is blocking a Poller 
> thread.
> *** Race condition for message.transfer
> 
> Sketch of the current code:
> // Called in connection thread 
> PersistableMessage::enqueueAsync { ++counter; } 
> // Called in store thread once message is written.
> PersistableMessage::enqueueComplete { if (--counter == 0) notifyCompleted(); }
> The intent is that notify be called once per message, after the
> message has been written to each queue it was routed to.
> However of a message is routed to N queues, it's possible for
> notifyCompleted to be called up to N times. The store thread could
> call notifyCompleted for

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

2011-06-01 Thread jirapos...@reviews.apache.org (JIRA)

[
https://issues.apache.org/jira/browse/QPID-3079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13042400#comment-13042400
]

jirapos...@reviews.apache.org commented on QPID-3079:
-

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/820/
---

Review request for Alan Conway and Gordon Sim.

Summary
---

Here's my attempt to implement a solution to qpid-3079 - with an eye towards
allowing async completion of various commands. I've integrated the existing
async Message.Transfer completion code to use this new design.

The implementation seems too "codey" - I had hoped that this would've resulted
in a simpler implementation, but I've found that C++ is a harsh mistress.

I'd like some early feedback - particularly suggestions for reducing the amount
of code/simplifying the design if possible.

There are two major functional changes involved:

1) changes to the Queue:dequeue() method to allow a callback if the dequeue is
asynchronous. Since the caller does not know ahead of time whether the dequeue
is sync or async, Queue::dequeue() now takes a functor parameter supplied by
the caller. The purpose of the functor is to allocate a context to track the
dequeue completion only if necessary. See Queue.h/cpp and the
SemanticState::accepted() changes for detail

2) - and this is the one aspect I'd like to improve - changes to the
SessionState/SessionContext/SemanticState interface to allow a command to
complete asynchronously. My understanding of the code is that the
SessionContext defines an abstract interface to the SessionState. The
SemanticState uses this interface for getting session-related information.

The changes involve creating an abstract class within the SessionContext class
that represents the context for a single outstanding asynchronous command.
This class is abstract - the intent is for SemanticState to subclass it and add
any state it needs in order to asynchronously process a particular command (see
the class AsyncMessageAcceptCmd in SemanticState for an example of a derivation
used for asynchronous Message.Accept). My intent was to have SemanticState
allocate one of these contexts should a command require async completion - and
use this context to retain state while the command executes.

Then the SemanticState would register the context with the SessionState via the
SessionContext's registerAsyncCommand() method. The SessionContext would store
information that it needs when the command completes (right now: sequence #,
isSync bit set, and acceptRequired in the case of message.transfer).

What I particularly do not like is the exposure of the AsyncCommandManager
context into the SessionContext interface. This command manager was created
when I added support for async Message.Transfer - it provided a context that
would track all outstanding message.transfer commands that were in flight
should the SessionState be destroyed. The command manager context is
reference-counted, and each outstanding command references it - guaranteeing
that some context is present when the command completes, even if the session
has been destroyed.

It would be great if I could hide this manager context from being exported by
SessionContext. And I'd like a better way of tracking the command seq #,
isSync, and requiresAccept rather than just jamming them into the command
context when it is registered...

Regardless, please give me your impressions before I go too far down the
testing path, if possible. Thanks.

This addresses bug qpid-3079.
https://issues.apache.org/jira/browse/qpid-3079

Diffs
-

/branches/qpid-3079/qpid/cpp/src/qpid/broker/AsyncCompletion.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/Message.cpp 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStore.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/MessageStoreModule.cpp 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/NullMessageStore.cpp 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableMessage.cpp 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/PersistableQueue.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.h 1124901
/branches/qpid-3079/qpid/cpp/src/qpid/broker/Queue.cpp 1124901
/branches/qpid-3079/qpid/cpp/src/qpid

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

[jira] [Commented] (QPID-3079) message.accept command should be completed on a per-dequeue basis

26 matches

Site Navigation

Mail list logo

Footer information