Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(33)

Issue 73041: Performance improvements to XMLRPC

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by krisvale
Modified:
10 years, 2 months ago
Base URL:
http://svn.python.org/view/*checkout*/python/trunk/
Visibility:
Public.

Description

This patch is a combination of various improvements to SocketServer, BaseHTTPServer, SimpleXMLRPCServer and xmlrcplib. They are the result of deploying XMLRPC in a production environment at CCP. There are three parts: 1) Make the XMLRPC client and server handle HTTP/1.1 with keep-alive 2) make the xmlrpc client and server capable of using the gzip content encoding for the xml 3) Implement socket timeout for servers. I have been tying to submit these patches separately but it is an ardous process. Perhaps it is better to get this accepted as a single patch?

Patch Set 1 #

Total comments: 11

Patch Set 2 : Updated the patch. Added error handling for gzip decode error #

Total comments: 1

Patch Set 3 : Fix unittest. Move 'disable_nagle_algorithm' to StreamRequestHandler, where it is more appropriate. #

Total comments: 9

Patch Set 4 : reinstate send_host(), minor changes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+406 lines, -70 lines) Patch
Doc/library/simplexmlrpcserver.rst View 1 1 chunk +9 lines, -0 lines 0 comments Download
Lib/BaseHTTPServer.py View 1 1 chunk +18 lines, -10 lines 0 comments Download
Lib/DocXMLRPCServer.py View 1 1 chunk +0 lines, -4 lines 0 comments Download
Lib/SimpleXMLRPCServer.py View 1 2 3 5 chunks +51 lines, -6 lines 0 comments Download
Lib/SocketServer.py View 1 2 3 3 chunks +8 lines, -3 lines 0 comments Download
Lib/test/test_xmlrpc.py View 1 2 3 7 chunks +128 lines, -14 lines 0 comments Download
Lib/xmlrpclib.py View 1 2 3 11 chunks +192 lines, -33 lines 0 comments Download

Messages

Total messages: 14
krisvale
http://codereview.appspot.com/73041/diff/1/8 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/1/8#newcode1413 Line 1413: connection.putheader("Content-Encoding", "gzipp") typo, should be "gzip" http://codereview.appspot.com/73041/diff/1/8#newcode1429 Line ...
10 years, 2 months ago (2009-06-11 15:25:51 UTC) #1
Benjamin
Do you have tests for the gzip encoding? http://codereview.appspot.com/73041/diff/1/3 File Lib/BaseHTTPServer.py (right): http://codereview.appspot.com/73041/diff/1/3#newcode317 Line 317: ...
10 years, 2 months ago (2009-06-11 16:24:58 UTC) #2
krisvale
Also, I will add gzip tests http://codereview.appspot.com/73041/diff/1/3 File Lib/BaseHTTPServer.py (right): http://codereview.appspot.com/73041/diff/1/3#newcode317 Line 317: if not ...
10 years, 2 months ago (2009-06-11 21:02:02 UTC) #3
josiah.carlson
http://codereview.appspot.com/73041/diff/1/5 File Lib/SimpleXMLRPCServer.py (right): http://codereview.appspot.com/73041/diff/1/5#newcode435 Line 435: encode_threshold = 1400 #a common MTU I notice ...
10 years, 2 months ago (2009-06-14 17:47:42 UTC) #4
krisvale
Updated the patch. Added error handling for gzip decode error
10 years, 2 months ago (2009-06-14 21:04:19 UTC) #5
krisvale
Fix unittest. Move 'disable_nagle_algorithm' to StreamRequestHandler, where it is more appropriate.
10 years, 2 months ago (2009-06-14 21:40:44 UTC) #6
krisvale
This is a message http://codereview.appspot.com/73041/diff/5003/6005 File Lib/SocketServer.py (right): http://codereview.appspot.com/73041/diff/5003/6005#newcode377 Line 377: - disable_nagle_algorithm This was ...
10 years, 2 months ago (2009-06-14 21:43:45 UTC) #7
Benjamin
It looks good now. :) http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1154 Line 1154: f.close() I agree ...
10 years, 2 months ago (2009-06-15 03:08:44 UTC) #8
Martin v. Löwis
I'm not sure whether the change can go into 2.7, because it causes incompatibilities. http://codereview.appspot.com/73041/diff/6010/5015 ...
10 years, 2 months ago (2009-06-20 16:48:45 UTC) #9
krisvale
http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (left): http://codereview.appspot.com/73041/diff/6010/5015#oldcode1264 Line 1264: def send_host(self, connection, host): On 2009/06/20 16:48:46, Martin ...
10 years, 2 months ago (2009-06-21 17:21:56 UTC) #10
effbot
LGTM. http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1466 Line 1466: return self._connection Why remove the nice error ...
10 years, 2 months ago (2009-06-21 21:26:05 UTC) #11
krisvale
http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1466 Line 1466: return self._connection On 2009/06/21 21:26:06, effbot wrote: > ...
10 years, 2 months ago (2009-06-24 21:13:28 UTC) #12
effbot
http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1466 Line 1466: return self._connection > Because this is now part ...
10 years, 2 months ago (2009-06-24 21:24:02 UTC) #13
krisvale
10 years, 2 months ago (2009-06-24 21:24:44 UTC) #14
I've made two changes as per your comments:
1) send_host has been reinstated
2) I've re-added the HTTPS attribute check, for completeness.

I also fixed:
1) Transport needs to remember the "host" parameter in order to cache the
"connection"
2) changed the ServerProxy.__call__() to be aan attribute getter.  one can thus
close it with:  p('close')() and access the transport with p('transport').  The
latter can be  particularly useful, for example if one subclasses Transport to
add functionality, such as keepalive pings and other stuff.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b