[ovs-dev] [PATCH v3] [python] Don't mix system poll/monkeypatched select

[Ilya Maximets]

On 7/1/21 8:27 AM, Terry Wilson wrote:
> Sounds good to me. As you said before, the call to poll(0) itself doesn't appear to block eventlet. Further inspection showed that the issue that we were seeing was actually caused by cpu-bound code in python-ovs that was blocking eventlet on connection in __do_parse_update() and, if not using the c json extension, Parser.feed(). The short story is that eventlet recommends putting time.sleep(0) calls in cpu-heavy code without I/O to cooperatively yield to eventlet greenthreads. I'll post something with a little  more analysis and a patch tomorrow when I'm not typing on my phone.
> 
> With that said, I do still like the SO_ERROR code better than hacking around select/poll for eventlet.

Thanks for getting to the bottom of the issue!
Since it's not essential to revert these patches right now,
I'll keep them.  In case we'll find another issue with this
code in the future, we'll know what to do. :)

> 
> 
> 
> On Tue, Jun 22, 2021, 12:13 PM Ilya Maximets <i.maximets at ovn.org <mailto:i.maximets at ovn.org>> wrote:
> 
>     On 6/22/21 6:51 PM, Ben Pfaff wrote:
>     > On Tue, Jun 15, 2021 at 10:06:53PM +0200, Ilya Maximets wrote:
>     >> 3. Use select() + getsockopt(SO_ERRROR), effectively revert d6cedfd9d29d
>     >>    --> give up on systems that doesn't support SO_ERRROR. (ESX?)
>     >
>     > d6cedfd9d29d is the commit titled "socket-util: Avoid using SO_ERROR."
>     > from 2012.  This was right after the Nicira acquisition when we were
>     > trying to port OVS to ESX.  We dropped that effort later and took a
>     > different strategy on ESX.  You can kind of see that since there are no
>     > references to ESX in the log from 2014 to 2019 and the one in 2019 is
>     > commit e6fc4e2ce97e ("Remove ESX references.").
>     >
>     > So, in short, ESX doesn't matter, go ahead and use SO_ERROR.
>     >
> 
>     Good to know.  Thanks!
> 
>     So, IIUC, if we'll revert following 3 commits, the issue should be fixed:
> 
>     1. c1aa16d191d2 ("ovs python: ovs.stream.open_block() returns success even if the remote is unreachable")
>     2. 24f974c481bc ("socket-util: Remove get_socket_error().")
>     3. d6cedfd9d29d ("socket-util: Avoid using SO_ERROR.")
> 
>     We should keep test cases from the commit #1, though.
> 
>     This can be done as a single patch that reverts 3 above with an
>     explanation, why we're doing that.
> 
>     Terry, what do you think?
> 
>     Best regards, Ilya Maximets.
>