StaticFileHandler block the whole application when downloading a large file
Env: Tornado 6.1 When I download a 1GB file, the whole web application is blocked.
The code shows this await won't switch to other coroutines.
https://github.com/tornadoweb/tornado/blob/6cdf82e927d962290165ba7c4cccb3e974b541c3/tornado/web.py#L2648
after add
await asyncio.sleep(0)
the blocking-method dismissed.
I think this could happen with a very fast network, is that right? If you're able to write the data out to the network as quickly as you can read it from the disk, this coroutine always stays busy so it will never be forced to yield.
I'm not sure if we want to await asyncio.sleep(0) after every chunk (for efficiency), but it wouldn't be a bad idea to add a yield point at least occasionally. Or more elaborately, we could consider adding some sort of rate-limiting mechanism so that you could set an upper bound on how fast a file could be sent to keep fast clients from monopolizing your bandwidth.
In my use,the method flush do not work well. Someone tell me, it should be write header as stream reponse,method flush would work?
What do you mean that flush does not work well? What did you expect, and what happened instead? I don't understand your second sentence at all.
Sorry,it work well.The above I meet,may be: when the network is good and not good,the flush will work not the same?
I got it from document.
Flush adapts to the speed of the network. If the network is slow, flush() will make the application wait to help ensure that memory usage is stable. If the network is fast, flush() may not do anything at all.
Honglei's problem appears to be because the network is very fast. In this case flush does not need to make the application wait, but this also means that it does not help multiple tasks share the CPU fairly. This is something that may be helped by calling asyncio.sleep(0) occasionally.
I think a deeper issue here, to be honest, is the fact that def get_content() is not (or cannot, to the best of my knowledge,) be a coroutine. It is certainly helpful that the file is chunked, so that a large GB file download can be split in places and possibly yield up the IOLoop for other clients, but the initial act of fetching the content is still problematic.
Maybe I'm missing something, but it might be useful to define an asynchronous call to invoke (and supplant in some cases) calls to get_content():
async def get_content_iter(self, start: Optional[int]=None, end: Optional[int]=None):
# Default implementation falls back to get_content()
content = self.get_content(start, end)
if isinstance(content, bytes):
yield content
return
# Yield each chunk, with optional "rate-limiting".
for i, chunk in enumerate(file_chunks):
yield chunk
if i % 10 == 9:
await asyncio.sleep(0)
I suppose this doesn't support older versions of python that do not have asynchronous generators, but something similar could possibly be envisioned if older versions of python need to be supported. In any case, some asynchronous version of the get_content() call may be useful to actually support flows that make DB calls or S3 calls to serve some content, where typical "static file pattern" approaches that could use nginx might not be as effective.
True, although I'll point out that as the docs say, this is not intended to be a high-performance file server. When it's backed by a filesystem the files will often be in the page cache so the reads will be fast, and async filesystem reads are poorly supported anyway.
It would be nice for all the overridable methods to be async; that's certainly what we'd have done if we were starting this design after async methods existed. But now if we're going to change get_content we should also change get_content_version and all the rest, and pulling on that thread means changing a lot of other public interfaces.
I suppose this doesn't support older versions of python that do not have asynchronous generators
Fortunately that's not a problem; async generators were introduced in python 3.6, and the oldest supported version of python is now 3.7.
True, although I'll point out that as the docs say, this is not intended to be a high-performance file server.
Maybe not for localized static files, but this may be more relevant for files that are static, but served from a third-party source. You could imagine a variant of this handler serving file content from a database or from S3 or similar; ironically, the docs for web.StaticFileHandler suggest such possibilities (see here: https://www.tornadoweb.org/en/stable/web.html#tornado.web.StaticFileHandler ). Even with small files, the overhead for fetching file content from a database (as the docs suggest) would still potentially block the IOLoop.
Obviously, users could homebrew their own handlers to do this properly without blocking the IOLoop, but then they'd need to reinvent the Etag and caching logic, which isn't exactly obvious. Alas.
I'm playing around with my own repo and pip packaging for a different interface for now instead.