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

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


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


Comments on the diff.


/trunk/qpid/cpp/include/qpid/messaging/Tracker.h
<https://reviews.apache.org/r/1687/#comment3905>

    Alternative name for Tracker: SessionPoller



/trunk/qpid/cpp/include/qpid/messaging/Tracker.h
<https://reviews.apache.org/r/1687/#comment3907>

    Remove track(Sender). Send and receive should be symmetric and we may add 
more event types later.
    
    Applications using this API will themselves be server-like and getting 
their own async events that trigger the need to send messages. If any session 
gets capacity they'll want to check if they have an event for that session. 
Apps that know in advance about a set of messages to send on a session won't 
use this API.
    
    If there's an easy way for the user to tell which sessions are interesting, 
they can implement that in their own loop, no need to push that responsibility 
onto the Tracker.



/trunk/qpid/cpp/include/qpid/messaging/Tracker.h
<https://reviews.apache.org/r/1687/#comment3906>

    I would suggest something more like  sys::Poller with a single wait() 
function that returns an Event. That's a pretty common pattern. (seelect, poll, 
epoll etc) 
    
    I'd also suggest replacing track/remove 
    with: 
    
    void monitor(Session, incoming, outgoing)
    
    So you can control monitoring per-session. remove becomes: monitor(session, 
false, false)
    
    I think the 2 booleans probably want to be replaced by an enum or set of 
flags to be
    more extensible.
    



/trunk/qpid/cpp/src/qpid/client/SessionImpl.cpp
<https://reviews.apache.org/r/1687/#comment3908>

    Why doesn't session pass this to listener?



/trunk/qpid/cpp/src/qpid/client/amqp0_10/SenderImpl.cpp
<https://reviews.apache.org/r/1687/#comment3909>

    Should pass this to avoid need for map lookup by parent.



/trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.cpp
<https://reviews.apache.org/r/1687/#comment3924>

    why inline session.flush?


- Alan


On 2011-08-31 18:01:43, Gordon Sim wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1687/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-31 18:01:43)
bq.  
bq.  
bq.  Review request for Alan Conway and Darryl Pierce.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  This allows multiple sessions (from multiple connections) to be managed 
with a single thread. It also allows non-blocking control over senders that 
reach their capacity (e.g. due to flow control).
bq.  
bq.  It does not yet cover tracking of message settlement and reconnection is 
still blocking.
bq.  
bq.  The API changes consist of a new class, Tracker - not at all keen on that 
name but have yet to come up with something I like - through which a set of 
sessions and/or senders can be tracked for 'readability' or 'writability' 
respectively. This is somewhat asymmetric. The reason for this choice was that 
in general you only care about the writability (i.e. available capacity) of 
certain senders (those you have data to send out on at present), whereas 
generally you care about any incoming messages. I could see this being extended 
to support both tracking specific receivers and to track all senders on a 
session but those seem like more special cases and not as critical at first.
bq.  
bq.  The internal implementation on the 0-10 codebase leaves a lot to be 
desired. At present it is certainly not the most optimal, but it has virtually 
no effect on users who don't use the new class. The threading of the 0-10 
client is driven by its use of the old client API underneath it. That and the 
locking in the messaging API layer means that a lot of checking actually needs 
to occur on application threads rather than the IO thread. Its this reason that 
I haven't tried at this stage to e.g. make a file handle readable when the 
tracker has a next() session available. Since it would in any case require a 
separate thread, little is gained at this stage. I envisage the 1.0 
implementation being able to handle that case much better, being architected 
from the start with a more ideal threading.
bq.  
bq.  So while this patch is still fairly rudimentary I thought it was worth 
sharing for some wider comment from interested parties.
bq.  
bq.  
bq.  This addresses bug QPID-3460.
bq.      https://issues.apache.org/jira/browse/QPID-3460
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    /trunk/qpid/cpp/src/qpid/messaging/SessionImpl.h 1163236 
bq.    /trunk/qpid/cpp/src/qpid/sys/BlockingQueue.h 1163236 
bq.    /trunk/qpid/cpp/src/qpid/messaging/Session.cpp 1163236 
bq.    /trunk/qpid/cpp/examples/messaging/Makefile.am 1163236 
bq.    /trunk/qpid/cpp/examples/messaging/extra_dist/Makefile 1163236 
bq.    /trunk/qpid/cpp/examples/messaging/non_blocking.cpp PRE-CREATION 
bq.    /trunk/qpid/cpp/include/qpid/messaging/Session.h 1163236 
bq.    /trunk/qpid/cpp/include/qpid/messaging/Tracker.h PRE-CREATION 
bq.    /trunk/qpid/cpp/src/CMakeLists.txt 1163236 
bq.    /trunk/qpid/cpp/src/Makefile.am 1163236 
bq.    /trunk/qpid/cpp/src/qpid/client/SessionImpl.h 1163236 
bq.    /trunk/qpid/cpp/src/qpid/client/SessionImpl.cpp 1163236 
bq.    /trunk/qpid/cpp/src/qpid/client/amqp0_10/SenderImpl.cpp 1163236 
bq.    /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.h 1163236 
bq.    /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.cpp 1163236 
bq.    /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionTracker.h PRE-CREATION 
bq.    /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionTracker.cpp PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/1687/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Minimal ad hoc testing with the new example contained in this patch.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Gordon
bq.  
bq.



> Better support for non-blocking usage in messaging API
> ------------------------------------------------------
>
>                 Key: QPID-3460
>                 URL: https://issues.apache.org/jira/browse/QPID-3460
>             Project: Qpid
>          Issue Type: Improvement
>          Components: C++ Client
>    Affects Versions: 0.12
>            Reporter: Gordon Sim
>            Assignee: Gordon Sim
>
> In particular:
> * remove the requirement for a thread-per-session when processing incoming 
> messages
> * remove the need to either block or poll when a sender reaches its capacity
> * remove the need to poll for changes in message settlement (i.e. completion 
> of sends and acknowledgements)
> * allow non-blocking reconnection
> * allow integration with polling loops etc

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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

Reply via email to