undefined | Better HN

0 pointsnbadg9y ago0 comments

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

0 comments

dante99999y ago

> Does that all make sense?

yeah it does make sense.

> in a real-world deployment you couldn't limit connections with client-side code

yeah that's a very good point. But in a real world scenario handling this would not be that easy. Limiting number of available connections on the server side is not a trivial task to implement. Setting your server to avoid failures and simply return either 503 service unavailable to some clients or 429 (too many conns) to others would probably require quite a lot of coding. It's also not very clear to me how this would be implemented, how do people implement things like this? Just putting some check for number of open files before line that opens file and setting response code to 500 and 429 before opening file? This would only stop server from opening to many "html" files, but would not stop server from getting flooded with connections. Is my aiohttp app even the right place to add checks like this? Wouldn't it be better to use haproxy or nginx or some other load balancing service in front of aiohttp app and let it handle too much traffic?

Other thing that comes to my mind (need to check this later) is that perhaps some partial "handling" of cases like this could/should be implemented in aiohttp library. I'm not sure how it behaves now, but maybe it should simply fail to open file, return 500 to the client, and print noisy traceback about open files to my logs? I didnt see this behavior when doing my tests, so either it didn't occur, is not implemented in aiohttp, or it occurrred and I somehow missed that. From my experience with Twisted I know that this is how Twisted resources behave, if you have some unhandled exceptions twisted just returns 500 to client and show traceback in logs.

nbadgOP9y ago

Keep in mind that 5XX error codes are for server errors and 4XX codes are for client errors. Returning 429 would imply "too many connections (from your computer)", not total for the service. Choosing to return a 503 for over-taxed servers is, as far as I can tell, done maybe half the time. Depending on the kind of service you're running, you might want to enforce a server timeout that says "after a certain number of milliseconds of local response time, return a 5XX error code and abandon the connection". That would be a particular component in an overall strategy for handling high load, which would heavily bias towards serving the easiest responses first. That may or may not be a good idea: what if the "expensive" requests are from paying customers accessing account pages, and the "inexpensive" ones are from a sudden spike in traffic to your homepage due to some good press somewhere? Of course eventually, you'd want to separate these two kinds of traffic entirely, such that customers are only affected by outages that they create. You can then focus on expanding your capacity to handle customers directly, instead of trying to lump that in with the much more unpredictable behavior of general web traffic.

> Just putting some check for number of open files before line that opens file and setting response code to 500 and 429 before opening file?

So actually this is one of the big benefits of putting the semaphore limiting file access within its own dedicated coroutine (except on the server side instead of the client). It allows you to handle the connection without having to deal with immediate responses. What that means in practice is that your server will be slower to respond under high load, but until it hits the client's (browser) timeout limit, you'll still be able to respond. It actually doesn't require any extra code to do that. Note that this isn't the only way to achieve this result, but it's probably the most direct, and simplest, especially given the approach you've taken with the code thus far.

A load balancer sits on top of that, ideally monitoring metrics like server CPU usage, memory load, or (most directly) request response time, and then shifts around requests between servers accordingly, to minimize the delay incurred in the aforementioned "wait for semaphore (or other synchronization primitive)" part.

At the end of the day, until you start hitting the limit of concurrent connections that others have mentioned, you don't really actually need to worry very much about how many connections you have open at once. You just want to focus on handling every connection you have as quickly as possible.

j / k navigate · click thread line to collapse