3
\$\begingroup\$

I have a simple webserver written in Python using aiohttp. My goal is a server that can receive many file uploads simultaneously and stream them all to disk. This code works, but I'm not sure if it is efficiently handling simultaneous connections:

import pathlib
import click
from aiohttp import web
from src import settings
from src import logger

log = None


async def upload_handler(request):
    """
    POST handler that accepts uploads to any path.
    The files are accepted and saved under the root path
    of the server.
    """
    # You cannot rely on Content-Length if transfer is chunked.
    size = 0
    local_path = settings.WWW_ROOT + request.path
    path_without_file = local_path.rsplit('/', 1)[0]
    pathlib.Path(path_without_file).mkdir(parents=True, exist_ok=True)

    with open(local_path, 'wb') as f:
        while True:
            chunk, is_end_of_http_chunk = await request.content.readchunk()
            if not chunk:
                break
            size += len(chunk)
            f.write(chunk)

    return web.Response(text='%d bytes uploaded' % size, status=201)


@click.command()
@click.option('--port', type=int, default=8000)
@click.option('--log-level', type=click.Choice(['debug', 'info', 'warning', 'error', 'critical']), default='info')
def cli(port, log_level):
    global log
    log = logger.get_default_logger(name='', log_level=log_level)
    app = web.Application(client_max_size=2e9, debug=settings.DEBUG)
    app.router.add_post('/{tail:.*}', upload_handler)
    web.run_app(app, port=port)


if __name__ == '__main__':
    cli()

The aiohttp stuff feels a little magic to me. Is this running on parallel threads? I'm seeing some high CPU usage at times.

Are there performance issues here? I see performance issues on the server, but I'm not sure if they occur here or elsewhere in the stack.

\$\endgroup\$
  • \$\begingroup\$ Wait, you're allowing anybody to upload to a server that's accessible to the general Internet? That sounds like an imminent web server compromise or malware distribution is about to happen. \$\endgroup\$ – Snowbody Mar 22 '18 at 19:53
  • 1
    \$\begingroup\$ No this is being proxied by nginx with authentication set up \$\endgroup\$ – bcattle Mar 22 '18 at 19:53
  • \$\begingroup\$ aiohttp uses threads for DNS lookups (resolver.py), if aiodns isn't available. Try running with aiodns installed. \$\endgroup\$ – Daniel Mar 22 '18 at 21:22
2
\$\begingroup\$

Interesting application.

Is this running on parallel threads?

Ummm, kind of. But in the kernel, not in python threads. Suppose you have four cores. Then what is good about the await ... readchunk() is that if in steady state it consumes 10% of a core, then python could get nearly 40 webserver threads to do useful work for that many clients before the onset of thrashing. (Webservers are open loop, rather than a closed loop queueing system, so you pretty much always need to have a little headroom available, a few idle cycles, to stay out of pathological performance regimes.)

    path_without_file = local_path.rsplit('/', 1)[0]

I'm sure this is right, but it's not very idiomatic. You don't like os.path.dirname() ?

    pathlib.Path(path_without_file).mkdir(parents=True, exist_ok=True)

Ok, that's a little crazy. Yes, I understand that nginx only hands you authenticated requests. But still. You just accepted arbitrary bytes under control of the web client and used it to mutate your local filesystem. I recommend at least verifying that folder or path_without_file conforms to some sane regex (no unicode combining marks). Same remarks, but more so, for the open(local_path, 'wb') fragment.

            f.write(chunk)

You didn't post any timing figures, so it's not clear if network input or disk output is the bottleneck. But if you want to go wild you might consider tracking current offset and doing an aio write at that offset, to obtain greater queue depth against your local RAID or other storage. When benching, be sure to try lots of slow clients and also try a couple of heavy hitters. You may find that the output of tcpdump -r trace -ttt is instructive, for revealing stalls.

Security remarks aside, kudos on some nice simple, solid code. Looks like it accomplishes what you set out to do.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.