Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

waiting: futures, Ready.RW #107

Closed
wants to merge 2 commits into from
Closed

waiting: futures, Ready.RW #107

wants to merge 2 commits into from

Conversation

dlax
Copy link
Contributor

@dlax dlax commented Oct 13, 2021

The second change appears to be needed for the pipeline mode #74 in order to have a generator waiting on RW-read event in async case. I tried to explain things in commit messages. For more context, the generator I'm working on is visible at dlax@98f741a.

@dvarrazzo
Copy link
Member

I wasn't aware of a thing like ready for RW. I thought the kernel tells you either one or the other.

The fact that python 3.6 and 3.10 have failed makes me think that it's not really deterministic...

However if there's a better way to do the async loop, that'd be great 😄

@dlax
Copy link
Contributor Author

dlax commented Oct 13, 2021 via email

@dlax
Copy link
Contributor Author

dlax commented Oct 14, 2021

I wasn't aware of a thing like ready for RW. I thought the kernel tells you either one or the other.

On that topic, dlax@0fedfc0 adds a test to demonstrate that this is possible with various select() strategies. (See the CI job at https://github.com/dlax/psycopg3/runs/3892034101.)

Also, simpler:

>>> import select
>>> import tempfile
>>> fd, fpath = tempfile.mkstemp()
>>> fd
3
>>> select.select([fd], [fd], [])
([3], [3], [])

(Not an expert in this field, so my reasoning might be wrong...)

In some generators, we might be interested in receiving both read-ready
and write-ready event at the same time.

The previous implementation of wait_epoll() and wait_async() did not
allow that: wait_epoll() would return Ready.R, even if the event was
'EPOLLIN | EPOLLOUT', and wait_async() would just send the first ready
value (because of the previous Event-based strategy).

On the other hande, wait_selector() already worked fine, by accident
since it did not return a true Ready value, but an integer; this is
fixed now.

In tests, this is demonstrated by using a connection in pipeline mode,
with a "large" query and a little sleep.
@dlax
Copy link
Contributor Author

dlax commented Oct 14, 2021

Amended second commit to use the simpler test mentioned above and apply to all wait() implementations.

@dlax dlax mentioned this pull request Oct 15, 2021
@dlax
Copy link
Contributor Author

dlax commented Oct 17, 2021

I'm going to rework this and submit another PR.

@dlax dlax closed this Oct 17, 2021
@dlax dlax deleted the futures branch November 12, 2021 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants