3 ms·
Thanks for feedback. > You would probably handle more requests if you changed that -- I would do the file access in a run_in_executor with a max executor work
by dante9999 10y ago
Thanks for feedback.
> You would probably handle more requests if you changed that -- I would do the file access in a run_in_executor with a max executor workers of 1000.
This is really good point. I'm going to check this and edit post adding this information there.
> Also, the placement of your semaphore acquisition doesn't make any sense to me. I would create a dedicated coroutine like this:
looking into my semaphore code next day after writing it I do wonder if I'm using it correctly. I assumed it works correctly because it fixed my "too many open files" exception, so it seems to mean that I'm no longer exceeding 1024 open files limits. Can you clarify why you think my use of semaphore does not make sense and why your suggestion is better? What is the benefit of dedicated coroutine?
> That being said, it also doesn't make any sense to me to have the semaphore in the client code, since the error is in the server code.
I admit that I focused more on my client than server. One thing that worries me about my test server is that it does not print any exceptions. Either it does not fail at all, which seems unlikely, or it fails silently, which is more likely and is bad. So I need to check my server code to see what exactly happens there.
> it also doesn't make any sense to me to have the semaphore in the client code, since the error is in the server code.
main reason for semaphore in client code is that it should stop client from making over 1k connections at a time. My logic here is that if client wont make 1k connections at a time - server wont receive 1k connections at a time and thus there will be no problem of too many open files on server (it won't have to send more than 1k responses). However I see that this logic may not be totally correct, other comment points out that it's possible for sockets to "hang around" after closing: https://news.ycombinator.com/item?id=11557672 https://news.ycombinator.com/item?id=11557672 so I need to review that and edit post.
> https://docs.python.org/3/library/asyncio-dev.html#debug-mode-of-asyncio https://docs.python.org/3/library/asyncio-dev.html#debug-mod...
this looks really great, will look into this thanks.
- e12e 10y agoAs per my comment further up, it might be interesting to spin up a handful of listening processes (eg: 127.0.0.1 through 127.0.0.10), and a handful of clients; and have the clients pick one at random, or something like that. Not so much for "real world testing", but just as an exercise to see if one can press the system to other limits than open connections/address pairs?
- nbadg 10y agoNo problem, it's especially hard to find external feedback for side projects and experiments so I try to give it when I can. > I assumed it works correctly because it fixed my "too many open files" exception It works, so at the end of the day that's what matters. The client vs server question, from my perspective, ultimately comes down to a question of test realism; in a real-world deployment you couldn't limit connections with client-side code because there are multiple clients. That's what I mean by "it doesn't make sense given that the error is server-side". > Can you clarify why you think my use of semaphore does not make sense and why your suggestion is better? What is the benefit of dedicated coroutine? I'm saying that mostly, but not exclusively, from a division of concerns standpoint. You're acquiring the semaphore in a completely different context than you're releasing it. On the one hand, that's partly a programming style issue. On the other hand, it can also have some really important consequences: for example, it's actually the event loop itself that is releasing the semaphore for you when the task is done. Because of the way the event loop works, it's hard to say exactly when the semaphore will be released. You want to hold it for the absolute minimum time possible, since it's holding up execution of other connections in the loop. Putting it into a dedicated coroutine makes it clearer what's going on, makes it such that the acquirer and releaser of the semaphore are the same, and means you are definitely holding the semaphore for the minimum amount of time possible (since, again, execution flow will not leave any particular coroutine until you yield/await another). In general I would say that releasing the semaphore in a callback is significantly more fragile, and mildly to moderately less performant, than creating a dedicated coroutine to hold the semaphore and handle the request. Does that all make sense? > Either it does not fail at all, which seems unlikely, or it fails silently, which is more likely and is bad. That's a fair statement, I think. As an aside, the print statement is slow, so keep that in mind. It might actually be faster to have a single memory-mapped file for the whole thing, and then just append the error and traceback to the file. The built-in traceback library can be very useful for that. That's also a bit more realistic, since obviously IRL you wouldn't be using a print statement to keep track of errors. On a similar note, because file access is so slow, you'd be best off figuring out some way to remove the part where the server accesses the disk once per connection entirely. On a real-world system you'd possibly use some kind of memory caching system to do that, especially if you're just reading files and not writing them. That allows you to use a little more memory (potentially as little as enough to have a single copy of the file in memory) to drastically improve performance.