Re: Async branch documentation and review

On Fri, Apr 9, 2010 at 8:16 PM, Jan Urbański <[email protected]> wrote:

> In short, I wouldn't personally go for removing poll() from the cursor
...
> Same as above, no problem with having only connection.fileno() (and no harm
> in keeping it).

Yes, to reply Federico too, I think the important thing is that
fileno() and poll() are either both available on the cursor or both
missing: the two method together make what we may call a "pollable
interface", so having only poll() and not fileno() on the cursor would
be the only poor choice we risk to do :)

>> I just discovered another quirk in having a cursor.poll() while the
>> responsibility of the communication state really belongs to the
>> connection: cursors can steal results each other:
>
> :( Yeah, that kind of sucks. Could this fall in the category of "so don't do
> it"? Or should we add an exception when you try to poll() a cursor that's
> not the one currently executing?

What I mean, from this and the above observation is that, if there are
a bunch of methods on the cursor that a) only delegate or thinly wrap
a call to self->conn and b) are guarded by something like "if self !=
self->conn->async_cursor { complain("wrong cursor") }", then I feel
like these methods have been put on the wrong object.

Having many cursor per connection is mostly useful to enable different
clients (functions, threads...) to have their results in an object of
their own. But I think the responsibility to carry ahead the
communication belongs to the connection and, correct me if I'm wrong,
the libpq only allows a single execution at time, until the results
have been fetched or canceled. The latter problem (the result theft)
for me smells as a glitch arising from having put on the "many" side
something actually belonging to the "one" side.

> Another thing is that if/when we start supporting async named
> cursors, we might need to still call something on the cursor
> (haven't thought about the interface yet).

I think whatever status you may need in the cursor object, it is
always reachable from the connection via the conn->async_cursor
pointer.

>> Jan, looking at your example in the test_async, the procedure to
>> obtain a nonblocking connection seems being:
>
>>
>>  [complicated code]
>>
>> I understand why you start with a POLL_WRITE, have a write wait and
>> then poll, but only because I've read the libpq documentation [1] and
>> dealt with this quirk myself in the green branch. I think it would be
>
>> Wouldn't it be better to have connection.poll() return a consistent
>> result and so suggest the user a single wait loop for both the phases?
>
> This has one problem, namely that if you don't wait for the socket to become
> writable you can't reliably even start executing any query, because if the
> write buffer happens to be full you will get an EWOULDBLOCK. The same goes
> for a connection attempt, calling PQconnectionPoll will return an error if
> the write socket is full (which will almost never happen, but for execute()
> it's not that obvious).
>
> So you always need that kind of logic:
>
>  * wait for fd to be writable
>  * execute query or start connecting
>  * poll and wait on the read or write event until it's done
>
> You could make poll() return POLL_WRITE by default, but then the user would
> have to first do select once, then execute a query and then enter the select
> loop. I can't see a good way to prevent that.

I don't think I am following you here. I know that a wait for write is
required before start looping for PQconnectionPoll(): this is why I
think that poll() should return WRITE right after PQconnectionStart(),
while from your example it seems the user is expected to know this. I
may be wrong on this point, in which case the best thing would be if
you could fix the example I've written in the documentation.

To say in code what I am clumsily trying to say in English, here
(http://tinyurl.com/y6c4s4x) is the implementation of poll() for the
connection phase in the green branch. As soon as the connection
begins, it is put in status=CONN_STATUS_SETUP and PQconnectStart() is
called. The first time poll() is called, it puts
status=CONN_STATUS_ASYNC and unconditionally returns POLL_WRITE, so
that the caller will write for the fd writable. Successive poll()
calls will invoke PQconnectionPoll() and return according to it. In
this implementation then, poll() is called once more than
PQconnectPoll(), but this way it is the only method in charge to say
"what to do now".

execute() is a different beast, but I have followed the same guideline
of only considering connection.poll() responsible to carry on the
communication. Internally this is is done with a different set of
libpq calls (PQflush, PqconsumeInput, PQisBusy...) and state changes,
but externally, from the PoV of the poll() caller, both connect() and
execute() loop are identical: poll(), exit if OK, else wait for read
or write and loop. With another entirely different implementation it
will be possible to support COPY FROM/TO both in async and in green
modes (hopefully by then we may have a single implementation) with the
same client wait loop.

If we don't do this, we will have to explain the users that polling
after connect() and polling after execute() have to be done in two
different ways: I don't like this very much because we would end up
tying the clients' connect() loop to an implementation detail of the
libpq async connection sequence.

-- Daniele
_______________________________________________
Psycopg mailing list
[email protected]
http://lists.initd.org/mailman/listinfo/psycopg