[jira] [Commented] (PROTON-668) Document Proton-c IO restrictions for 0.8 release

["Cliff Jansen (JIRA)" <ji...@apache.org>]

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

Cliff Jansen commented on PROTON-668:
-------------------------------------

Additional notes based on comments to the Proton mailing list:

> >   a socket may only ever be used with a single pn_io_t in its lifetime
> >     exception: a socket from pn_accept() is not yet associated with any pn_io_t
> >     and its first use can be with any pn_io_t (or never with a pn_io_t at all)

> I'm not sure if it's implied, but pn_connect() and pn_listen() also need
> to support 'third party event loop'.
> Specifically, pn_connect() has to remain non-blocking (we get to know
> about the connect error later in the external event loop)

Yes third party loop support is implied for pn_listen and pn_connect.
The exception comment is about "moving" a socket between two pn_io_t
instances (or determining "first use").  The third party loop is merely
not allowed to pass any given socket to more that one pn_io_t.

> >  threads may do concurrent pn_io_xx) calls as long as no two are simultaneous 
> >  on the same socket (where xxx is send/recv/read/write)

> This will break pn_io_error() and pn_io_wouldblock() as they are defined now.

There is certainly no error or blocking state kept per socket, or per
thread.  I am describing the existing multi-threaded use in driver.c
(heavily used by Dispatch) and assuming others may wish to do the same
on Windows.  pn_io_error and pn_wouldblock are conspicuously absent in
driver.c (old and new).

> I have a feeling you don't really want/need to expose the pn_pipe(),
> but add a pn_selector_interrupt() and a mechanism of querying that for
> the caller of pn_selector_select() especially as you want to implement
> it completely differently on windows.

Yes, the pipe is an awkward difference between Windows and Posix
thinking, where the latter sees pipes and sockets as close kin but
Windows sees them as very distant relatives.

Ditching the pn_pipe() call altogether in favour of a
pn_selector_interrupt() is cleanest and most performant from the
Windows IOCP perspective.  However, as long as a pn_pipe is only
accessed via pn_read and pn_write the scope for optimization reduces
any performance penalty to a minimum.  So I am OK keeping things as
they are but would not resist change.


> Document Proton-c IO restrictions for 0.8 release
> -------------------------------------------------
>
>                 Key: PROTON-668
>                 URL: https://issues.apache.org/jira/browse/PROTON-668
>             Project: Qpid Proton
>          Issue Type: Improvement
>          Components: proton-c
>    Affects Versions: 0.8
>            Reporter: Cliff Jansen
>            Assignee: Cliff Jansen
>            Priority: Blocker
>             Fix For: 0.8
>
>
> Proton is designed to provide an efficient IO layer that functions without imposing a threading model on the application.  Applications may (1) roll their own IO and just use the Proton engine, (2) use all Proton primitives, (3) use some Proton primitives augmented by an external event loop.
> Case (1) is unrelated to this JIRA.  The others may be restated:
> Scenario 2: Proton event loop: a proton selector manages socket events for all sockets placed in the selector, all associated sockets use pn_io_xxx() calls.  Sockets outside the selector are "unmanaged" and passed through to the OS socket function unchanged.
> Scenario 3: Third party event loop (no proton selector involved), all sockets are treated as for "unmanaged" in scenario 2.
> Scenario 4, 5...: Others to support?
> The problem:
> The Proton Posix pattern for efficient IO is:
>   "tell me when your (OS) buffer is ready for io transfer (in or out)"
> Whereas the normal Windows pattern is somewhat reversed (IO completion ports):
>   "tell me when you are done transferring data (to or from) my (user space) buffer"
> The current Windows IOCP implementation (PROTON-640) tries to make the latter look like the former with some constraints.   There should be documentation specifying reasonable limits on Proton usage that may be falsely implied by the API but do not translate efficiently to Windows.  Assuming that future Windows implementations may adopt more aggressive performance strategies (especially on the read side), I would propose something along the lines of:
>   a socket may only ever be used with a single pn_io_t in its lifetime
>     exception: a socket from pn_accept() is not yet associated with any pn_io_t and its first use can be with any pn_io_t (or never with a pn_io_t at all)
>   send/recv/close may not be intermixed with similar non-Proton OS calls (otherwise: out of order or lost data)
>   a socket can move once from an external loop to a proton loop, but never the other way
>   pn_pipe() values can only be used with pn_read and pn_write and pn_selector_select, they cannot participate in an external event loop.
>   Furthermore, there is no thread safety except:
>     threads may do concurrent pn_io_xxx() calls as long as no two are simultaneous on the same socket (where xxx is send/recv/read/write)
>     pn_selector_select() is thread safe against pn_read/pn_write/pn_send/pn_recv, but the outcome of the select is indeterminate.  pn_selector_select() must be interrupted and restarted at any time when other simultaneous IO may affect the outcome.
>     calls on different pn_io_t objects do not interact and are thread safe.
> If it is desirable for a socket to be used in an external loop after being used in a Proton loop, we would need some sort of blocking calls along the lines of:
>   pn_io_flush()
>   pn_io_drain()
> which would be no-ops on Posix but would unwind outstanding completions on Windows.
> Early criticism of any of the above assumptions would be greatly appreciated.  I will try to reword the above, or its evolution into the existing documentation for 0.8.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: [jira] [Commented] (PROTON-668) Document Proton-c IO restrictions for 0.8 release

[Bozo Dragojevic <bo...@digiverse.si>]

Hi Cliff,

I hope you don't mind if I reply on list, and not in Jira...

On 12. 09. 14 09:18, Cliff Jansen (JIRA) wrote:
>     [ https://issues.apache.org/jira/browse/PROTON-668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14131213#comment-14131213 ] 
>
> Cliff Jansen commented on PROTON-668:
> -------------------------------------
>
> Additional notes based on comments to the Proton mailing list:
>
>>>   a socket may only ever be used with a single pn_io_t in its lifetime
>>>     exception: a socket from pn_accept() is not yet associated with any pn_io_t
>>>     and its first use can be with any pn_io_t (or never with a pn_io_t at all)
>> I'm not sure if it's implied, but pn_connect() and pn_listen() also need
>> to support 'third party event loop'.
>> Specifically, pn_connect() has to remain non-blocking (we get to know
>> about the connect error later in the external event loop)
> Yes third party loop support is implied for pn_listen and pn_connect.
> The exception comment is about "moving" a socket between two pn_io_t
> instances (or determining "first use").  The third party loop is merely
> not allowed to pass any given socket to more that one pn_io_t.
>

OK.

>>>  threads may do concurrent pn_io_xx) calls as long as no two are simultaneous 
>>>  on the same socket (where xxx is send/recv/read/write)
>> This will break pn_io_error() and pn_io_wouldblock() as they are defined now.
> There is certainly no error or blocking state kept per socket, or per
> thread.  I am describing the existing multi-threaded use in driver.c
> (heavily used by Dispatch) and assuming others may wish to do the same
> on Windows.  pn_io_error and pn_wouldblock are conspicuously absent in
> driver.c (old and new).
>

How the new driver propagates errors? I saw it's looking at errno, but,
does errno interact with iocp?
(didn't look at that, sorry)

Here is one example why sharing an pn_io_t across threads seems unsafe,
even if those threads do not
use pn_io_error() call:

  ssize_t pn_send(pn_io_t *io, pn_socket_t sockfd, const void *buf,
size_t len) {
    ssize_t count;
    iocpdesc_t *iocpd = pni_iocpdesc_map_get(io->iocp, sockfd);
    if (iocpd) {
      count = pni_iocp_begin_write(iocpd, buf, len, &io->wouldblock,
io->error);


So if two threads do pn_send() on same pn_io_t but on two different
pn_socket_t that are both in
some error state (that is captured in their respective iocpdesc_t->error),
both threads will access the same io->error, and that cannot end well.

>> I have a feeling you don't really want/need to expose the pn_pipe(),
>> but add a pn_selector_interrupt() and a mechanism of querying that for
>> the caller of pn_selector_select() especially as you want to implement
>> it completely differently on windows.
> Yes, the pipe is an awkward difference between Windows and Posix
> thinking, where the latter sees pipes and sockets as close kin but
> Windows sees them as very distant relatives.
>
> Ditching the pn_pipe() call altogether in favour of a
> pn_selector_interrupt() is cleanest and most performant from the
> Windows IOCP perspective.  However, as long as a pn_pipe is only
> accessed via pn_read and pn_write the scope for optimization reduces
> any performance penalty to a minimum.  So I am OK keeping things as
> they are but would not resist change.
>
>

agree that starting with the smallest API change is better.

Bozzo