Logging in BaseHTTPServer.BaseHTTPRequestHandler causes lag

[aerodonkey]

BPO	6085
Nosy	@orsenthil, @pitrou, @merwok
Files	server_resolve.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/orsenthil'
closed_at = <Date 2012-04-29.05:45:26.064>
created_at = <Date 2009-05-22.10:12:39.461>
labels = ['library', 'performance']
title = 'Logging in BaseHTTPServer.BaseHTTPRequestHandler causes lag'
updated_at = <Date 2012-04-29.05:45:26.062>
user = 'https://bugs.python.org/aerodonkey'

bugs.python.org fields:

activity = <Date 2012-04-29.05:45:26.062>
actor = 'orsenthil'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2012-04-29.05:45:26.064>
closer = 'orsenthil'
components = ['Library (Lib)']
creation = <Date 2009-05-22.10:12:39.461>
creator = 'aerodonkey'
dependencies = []
files = ['24637']
hgrepos = []
issue_num = 6085
keywords = ['patch', 'needs review']
message_count = 19.0
messages = ['88188', '103928', '103940', '104154', '104159', '104164', '104207', '104209', '104213', '104216', '104267', '104278', '154195', '154244', '158740', '159573', '159578', '159579', '159580']
nosy_count = 11.0
nosy_names = ['orsenthil', 'pitrou', 'schmir', 'eric.araujo', 'aerodonkey', 'ubershmekel', 'neologix', 'santoso.wijaya', 'python-dev', 'knicker.kicker', 'wmgaca']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue6085'
versions = ['Python 3.3']

[aerodonkey]

The invocation of socket.getfqdn to get FQDN of the host may delay for
several seconds on some platforms when no appropriate host name is found.
This causes lag when using SimpleXMLRPCServer and other class derived from
BaseHTTPServer for every request.

[santosowijaya]

There is a simple workaround to bypass the hostname resolution mechanism, so that the server can respond faster.

# This is a hack to patch slow socket.getfqdn calls that
# BaseHTTPServer (and its subclasses) make.
# See: http://bugs.python.org/issue6085
# See: http://www.answermysearches.com/xmlrpc-server-slow-in-python-how-to-fix/2140/

import BaseHTTPServer

def _bare_address_string(self):
    host, port = self.client_address[:2]
    return '%s' % host

BaseHTTPServer.BaseHTTPRequestHandler.address_string = \
_bare_address_string

# End hack.

[neologix]

Yes, performing a DNS resolution every time we log something is definitely sub-optimal. We should perform it only once, and cache the result.

[neologix]

The attached patch caches the result of FQDN lookup.

[santosowijaya]

Doesn't that only cache the first remote client it encounters, though? Maybe a dictionary of caches?

[neologix]

Doesn't that only cache the first remote client it encounters, though? Maybe a dictionary of caches?

A BaseHTTPRequestHandler is instantiated every time a client connects, so there should be only one client per handler, no (the cache is an attribute of the handler, not the server) ?

[pitrou]

Actually, DNS resolution should be disabled by default, as in most HTTP servers probably.

[orsenthil]

And for this specific request, it fdqn is looked up only for "logging" to sys.stderr. Either removing the fqdn call or just caching per connection it as the patch does is both fine. I doubt if someone is relying this logging anywhere in the code. This is useful only when BaseHTTPServer is started in the command like as a standalone HTTP server.

What shall we do for this? remove the fqdn or cache it once and use it. Either is fine. And there is not testing code present (or required too - as this is a cli invocation scenario).

[pitrou]

And for this specific request, it fdqn is looked up only for "logging"
to sys.stderr. Either removing the fqdn call or just caching per
connection it as the patch does is both fine. I doubt if someone is
relying this logging anywhere in the code. This is useful only when
BaseHTTPServer is started in the command like as a standalone HTTP
server.

I think we should totally remove the call to fqdn. The common practice
with HTTP servers is to log numeric IPs and, if desired, let a batch log
analysis process (such as awstats) deal with DNS resolution and caching.

And there is not testing code present (or required too - as this is a
cli invocation scenario).

What do you mean? BaseHTTPRequestHandler is tested in test_httpservers.

[orsenthil]

On Mon, Apr 26, 2010 at 10:45:56AM +0000, Antoine Pitrou wrote:

What do you mean? BaseHTTPRequestHandler is tested in test_httpservers.

I meant specifically for that function which is logging to sys.stderr.
No return values, state changes and no tests present in test_httpservers.
Seems to me the cli invokation scenario of the BaseHTTPServer module.

Only other place where fqdn look up happens is HTTPServer.server_bind
and in the setting of server_name, if any code is relying on it could
break.

[santosowijaya]

Please correct my understanding if I am mistaken, but from skimming through the source code, it seems that a new BaseHTTPRequestHandler is instantiated per request. If a caching mechanism is to be adopted as per the patch, wouldn't it be forgotten when a new request from the same client arrives in the future?

[neologix]

@santa4nt: You're correct, the cache is retained only from within the same handler: the other solution would be to keep the cache at the server level, but then you'd have to deal with the cache size (you don't want it to grow forever), and it seems a bit overkill. I just kept the FQDN resolution because the code seemed to do it on purpose, but I agree that if nothing relies on this behaviour, it might be as simple to remove the resolution altogether.