[jira] Commented: (QPID-2921) c++ broker: Improvements to asynchronos completion

["Ken Giusti (JIRA)" <ji...@apache.org>]

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

Ken Giusti commented on QPID-2921:
----------------------------------

Here's a summary of the improvements that Alan has recommended as I understand them:

i1) Remove the blocking of the polling thread by SessionState when pending for completion of outstanding async commands.
i2) Fix the enqueueAsync/enqueueComplete race condition that could result in multiple completions per single message
i3) do not allow an outbound message to satisfy a message.accept until the message is deleted from the queue's async store
i4) a message becomes eligible to satisfy a message.accept on a per-dequeue basis, not the last of all dequeues as it is currently done
i5) a message.accept references multiple messages - all must enforce rules i3 & i4 in order for the message.accept to complete.

And some new requirements:

r1) allow clustering to hold off the completions of (various) commands
r2) provide a completion model that will allow performing completions from a thread that may not be the same as the initiating thread.

Alan - is this correct?

As a starting point, I'm trying to implement asynchronous completion of message.transfer commands.  In doing this, I'm attempting to
address points i1, i2, and r2 above.  Here's what I've discovered so far:

First, I need to associate the transfer Completion with the inbound message, not with the transfer command itself.  This is because the
transfer command stays within the domain of the SessionState, whilst the message is handed off to various potential asynchronous services,
such as store, queuing, etc.  In other words, SessionState isn't aware of all the potential async operations that may apply to a given
message as it is transfered, but the message itself will.

Instead, I consider SessionState a "completer" - using Alan's definition.  It adds iself to the message, passes the message off to
semantic state, then completes the message on return.  This prevents the i2 race condition.

In addition, I'm planning to:

0) Augment Alan's proposed Completion class to enable it to identify the desired completion thread (via storing Thread::current()).   When
the Completion "completes", the action callback can check its current thread against this value to determine how to process the completion.
1) replace the asyncEnqueue functionality in the broker::Message with a more-generic transferComplete Completer object (based on Alan's
proposed class).  This allows the message to be considered complete against a variety of operations beyond queueing (e.g. credit supply).
2) remove the incomplete message list from SessionState - it is no longer needed.
3) Augment the SessionState enqueue complete callback to be thread aware.

I'll attach a set of code changes that roughly implements the above description for comment(won't compile yet - just as a reference).


> c++ broker: Improvements to asynchronos completion
> --------------------------------------------------
>
>                 Key: QPID-2921
>                 URL: https://issues.apache.org/jira/browse/QPID-2921
>             Project: Qpid
>          Issue Type: Improvement
>          Components: C++ Broker
>    Affects Versions: 0.8
>            Reporter: Alan Conway
>            Assignee: Ken Giusti
>
> ** 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.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org