-
Notifications
You must be signed in to change notification settings - Fork 245
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
core.Stream uses select() and it is prone to its limitations #182
Comments
…o its limitations
Fixed bug #182: Stream uses select() and it is prone to its limitations
…g bug tomerfiliba-org#182 and observing spinning processes.
…g bug tomerfiliba-org#182 and observing spinning processes.
The bug in compat.py was unveiled after fixing bug tomerfiliba-org#182 and observing spinning processes. After that, the poll() call in server.py became too slow (1s instead of 1ms), leading to timeouts. The new value allows for not spinning too fast yet reacting quickly to new incoming connections.
The bug in compat.py was unveiled after fixing bug tomerfiliba-org#182 and observing spinning processes. After that, the poll() call in server.py became too slow (1s instead of 1ms), leading to timeouts. The new value allows for not spinning too fast yet reacting quickly to new incoming connections.
The bug in compat.py was unveiled after fixing bug tomerfiliba-org#182 and observing spinning processes. After that, the poll() call in server.py became too slow (1s instead of 1ms), leading to timeouts. The new value allows for not spinning too fast yet reacting quickly to new incoming connections.
Further fixes unveiled after fix for bug #182
Hi, can't test this since I get
already while opening the files locally before doing any server related stuff. According to the commit messages, the issue is supposed to be fixed, so closing this. Reopen if needed. |
Hi, it's true that we have this fix in production for over one year and we did not see any issue, but nevertheless can you send me please the test you're executing and the full stack trace? |
ops, sorry - you mean you executed my test case I guess. After >1 year I forgot I posted it. Now the 'Too many open files' is precisely the error you get before the patch, in principle you should not get it after it. |
:)
No, I mean the error I get is not due to rpyc. I get an error not during select but already when opening the files, here:
Therefore, I can't really verify whether the error in rpyc itself still exists if it were possible on my platform to open so many files. (archlinux, tried py2.7 and 3.6) |
OK, I see. This is most likely because recent platforms limit the number of open files to 1024 (I could reproduce your behavior on my desktop, while on our production platforms, py2.7.5 on CentOS 7, I can still run the full test). You can try and run |
Great, thanks. Seems to work! |
Cool, thanks! |
The Stream class is supposed to use the lib.compat layer, in order to make use of poll() or select() depending on the OS. But the Stream.poll() method actually uses the builtin select() function straight, thus being prone to the select limitation of not handling file descriptors higher than 1024.
To prove this, you can try the following code (on Linux):
The stack trace includes the following error:
The following is a simple patch to fix this bug, can you please review it? Thanks!
The text was updated successfully, but these errors were encountered: