Re: Async branch documentation and review

Jan UrbaƄski <[email protected]> Sat, 10 Apr 2010 18:30:56 +0200
Newsgroups gmane.comp.python.db.psycopg.devel
Message-ID <[email protected]>
On 10/04/10 17:32, Daniele Varrazzo wrote:
>>> 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.

On one hand I admit that I see your point. On the other had, would that 
mean you execute() using a cursor, then poll() the connection and then 
fetch() from the cursor?

> 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.

Yeah, that's true. I can see how removing poll() from the cursor would 
help, but that will still be vulnerable to weird behaviour if after 
completing polling someone tries to fetch from a wrong cursor, as you 
demonstrated in a previous mail. I'm afraid that we'll just need to 
pluck the hole in poll() and fetch() by forbidding the usage of a wrong 
cursor.

>> 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.

Good point, consider my statement invalid.

> 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.

OK, I've been confusing two things. It seems to be a good idea to make 
connection.poll() return PSYCO_POLL_WRITE the first time it's called, 
before it calls PQconnectionPoll for the first time and hide that libpq 
detail from the user.

> 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.

Oh! Your post made me go and read the libpq code and I think you can do 
PQsendQuery without waiting for the socket to become writable. So it 
actually is possible to poll in the same way after connect() and after 
execute(), using your proposed loop, because both PQconnectStart and 
PQsendQuery are safe to be called even if the socket write buffer is 
full. For some reason I though you would get an error if you would try 
to call PQsendQuery when the socket write buffer is full.

I'll add a CONN_STATUS_SETUP status and returning PSYCO_POLL_WRITE in 
that state with async connections and that will allow to use the same 
loop for connecting and executing. Thanks for pointing that out.

Cheers,
Jan