Skip to content
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

Server closes connection before sending data #370

Closed
oicnysa opened this issue May 18, 2015 · 9 comments
Closed

Server closes connection before sending data #370

oicnysa opened this issue May 18, 2015 · 9 comments
Labels

Comments

@oicnysa
Copy link

oicnysa commented May 18, 2015

I have a very simple file server using Aiohttp:

import os.path
from os import listdir
import asyncio
from aiohttp import web
import aiohttp_jinja2
import jinja2

@aiohttp_jinja2.template('template2.html')
@asyncio.coroutine
def get(request):
  root = os.path.abspath(os.path.dirname(__file__))
  files = [file_name for file_name in listdir(os.path.join(root,'files'))
                    if os.path.isfile(os.path.join(root, 'files', file_name))]
  return {'files': files}

if __name__ == "__main__":

   app = web.Application()
   aiohttp_jinja2.setup(app, loader=jinja2.FileSystemLoader('/root/async'))

   app.router.add_route('GET', '/', get)
   app.router.add_static('/files/' , '/root/async/files')

   loop = asyncio.get_event_loop()
   f = loop.create_server(app.make_handler(), '0.0.0.0', 8080)
   srv = loop.run_until_complete(f)
   print(' serving on ', srv.sockets[0].getsockname())
   try:
    loop.run_forever()
   except KeyboardInterrupt:
    pass

When I test it using siege tool with, for example, such command:

siege -c 1000 -b -t 60 http://127.0.0.1:8080/files/megabytes15.test

It raises a lot of exceptions like this:

socket.send() raised exception

And the amount of data transferred is very small. I assume that the socket closes before the file is fully sent to the user. I would be very appreciated for some suggestion. Thank you in advance! UPD:

template2.html

<!DOCTYPE html>
<html>
<head><title>Aiohttp Test Server</title></head>
</body>
<h3>List of files:</h3>
<ul>
{% for item in files %}
<li><a href="/files/{{ item }}">{{ item }}</a></li>
{% endfor %}
</ul>
</body>
</html>

/root/async directory contains template2.html and server script file(code mentioned at the start). /root/async/files directory contains some dummy files of 15b, 15kb and 15mb sizes.

UPD: the exception is not risen for small files.
UPD: This exception is raised from connetion_lost function of web.py:

def connection_lost(self, exc):
        self._manager.connection_lost(self, exc)
        super().connection_lost(exc)
@oicnysa
Copy link
Author

oicnysa commented May 21, 2015

I've made my own investigation. Appeared that siege sends a get request with "Connection: close" header. This works fine for the same functionality, written on Tornado framework.
I've changed the "Connection: close" header to "Connection: Keep-Alive". Now there is no problem with "socket.send() raised exception", but siege throws Segmentation fault.
I assume that aiohttp, on most connection executes directive "Connection: close" first and than trying to send data to already closed socket. Need to figure out where relevant code is located.

@oicnysa oicnysa changed the title socket.send() raised exception Server closes connection before sending data May 21, 2015
@asvetlov
Copy link
Member

@oicnysa thanks for investigation. I filed an issue in asyncio: python/asyncio#248

@oicnysa
Copy link
Author

oicnysa commented May 21, 2015

Thanks. I've read your post in asyncio. Will try to patch by myself, but I'm not so familiar with aiohttp code. So maybe if you will have some time, you could describe more detailed, where "code to patch" situated. I mean, of course, not "/usr/lib/pithon/site-packages/aiohttp", but classes and methods. :)

@asvetlov
Copy link
Member

@oicnysa good fix requires changes in asyncio: we need for new API.
If people will agree with me I'll fix asyncio code. Or maybe somebody will guess better solution than I see now.
Dirty fix should add yield from asyncio.sleep(0) into static file handler, but I dislike it. The way slows down downloading of all files, that's why I don't want to fix aiohttp only.

To get rid of annoying prints (socket.send() raised exception is not exception actually but log warning record) you may do follow trick:

from asyncio import constants
constants.LOG_THRESHOLD_FOR_CONNLOST_WRITES = 1000000  # some big number

It will disable logging.

I still did not investigated why siege tool don't want to work with aiohttp server though. Looks like It gets answer unsupported by siege and terminates connection instantly. As result you see a lot of logging warnings.

@oicnysa
Copy link
Author

oicnysa commented May 21, 2015

I think it is because I've tried it on virtualbox(maybe it couldn't handle so many sockets). Now I've managed to test aiohttp with siege and Connection: Keep-Alive on more powerful machine(Dell PowerEdge R200). But the results were bad comparing to Tornado and siege with "Connection: Close".
Tornado had 2143 successful transactions and 96.88% availability, but aiohttp had only 267 successful transactions and 78.99% availability. But when running this test with smaller files like 15 kb and 10 seconds for test, aiohttp is faster.

@asvetlov
Copy link
Member

Tornado has more complex static files handling: it supports RANGE and content caching like LAST-MODIFIED and so on.
More important, Tornado has 64Kb chunks size but aiohttp used to have 8Kb.
In fresh aiohttp masher I've increased chunk size to 64Kb (but for modern machines it's maybe still too small value, 256Kb or even 1Mb would be better).

Anyway, for static files the best way is to use proxy server like nginx.

@oicnysa
Copy link
Author

oicnysa commented May 22, 2015

Thanks for information Andrew!

@oicnysa
Copy link
Author

oicnysa commented May 22, 2015

I've tried master version and it is performing very well.

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants