U1DB

Merge lp:~jameinel/u1db/faster-shutdown into lp:u1db

  1. faster-shutdown
  2. Merge into trunk
Proposed by John A Meinel
Status: Rejected
Rejected by: John A Meinel
Proposed branch: lp:~jameinel/u1db/faster-shutdown
Merge into: lp:u1db
Diff against target: 109 lines (+45/-6)
2 files modified
u1db/remote/sync_server.py (+24/-0)
u1db/tests/test_remote_sync_server.py (+21/-6)
To merge this branch: bzr merge lp:~jameinel/u1db/faster-shutdown
Reviewer Review Type Date Requested Status
Ubuntu One hackers Pending
Review via email: mp+81281@code.launchpad.net

Description of the change

The test suite has started slowing down a bit. We're now over 2s on my machine (2.3s for my last test).

I did some profiling, and a fair amount of time was spent waiting for the socket server to shut down. This adds a few changes, some of which are a bit ugly.

0) This changes 'make check' to finish in 1.4s down from 2.3s. That's about 40% of the total runtime just cut out.
1) Fix bug #884427, suppress EBADF when the server gets it, rather than printing to stderr/stdout.
2) When we get a shutdown request, do a simple connect and close to the server. The default serve_forever loop does have a customizable timeout, but it is set to 10ms, which seems to add up quite a bit with lots of tests. (currently we have 500 tests, which could be 5s total time.)
The connect-and-close can let us make that faster without turning the server into a busy-wait loop.
It does cause bug #884427 to trigger all the time, though, because now the server code tries to read on a socket that is closed. (sometimes that causes recv() to return the empty string, sometimes it raises EBADF)
3) Hack shutdown() to do this. You have to request the server start shutting down before you do the connection, unfortunately BaseServer uses a private variable, so we have to poke to get access to it.
Its either that or re-implement all the stuff that BaseServer implements to avoid a private attribute.

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) wrote :

On my notebook (i7, intel ssd) this takes it from ~2.8s to ~2.5s.

Revision history for this message
John A Meinel (jameinel) wrote :

code was deleted anyway.

Unmerged revisions

106. By John A Meinel

Get faster test shutdown by doing a fake connect.

This requires fixing bug #884427, otherwise we get too much stipple in tests.
Otherwise the SocketServer defaults to the 0.01s timeout time, which adds up when
you have lots of tests.

Overall, this patch changes 'make check' from 2.172s down to 1.423s.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'u1db/remote/sync_server.py'
2--- u1db/remote/sync_server.py 2011-11-01 19:07:07 +0000
3+++ u1db/remote/sync_server.py 2011-11-04 14:21:39 +0000
4@@ -14,7 +14,10 @@
5
6 """A Server that listens for synchronization requests"""
7
8+import errno
9+import socket
10 import SocketServer
11+import sys
12 import threading
13
14 from u1db import (
15@@ -58,6 +61,13 @@
16 self.handle_error(request, client_address)
17 self.close_request(request)
18
19+ def handle_error(self, request, client_address):
20+ exc_cls, exc_obj, exc_tb = sys.exc_info()
21+ if exc_cls is socket.error and exc_obj.errno in (errno.EBADF,):
22+ # Ignore EBADF, it just means client closed early.
23+ return
24+ SocketServer.TCPServer.handle_error(self, request, client_address)
25+
26 def _add_request(self, request, t):
27 with self._request_threads_lock:
28 self._request_threads[request] = t
29@@ -95,6 +105,19 @@
30 t.join()
31 t = self._get_a_request_thread()
32
33+ def shutdown(self):
34+ self._BaseServer__shutdown_request = True
35+ if self._BaseServer__is_shut_down.isSet():
36+ return
37+ s = socket.socket()
38+ try:
39+ s.connect(self.server_address)
40+ except socket.error:
41+ pass
42+ else:
43+ s.close()
44+ SocketServer.TCPServer.shutdown(self)
45+
46 def force_shutdown(self):
47 self.shutdown()
48 waiting_threads = []
49@@ -129,6 +152,7 @@
50 if extra_bytes:
51 decoder.accept_bytes(extra_bytes)
52 while not decoder.request_finished:
53+ # TODO: We could trap EBADF here, and treat it as content == ''
54 content = self.request.recv(READ_CHUNK_SIZE)
55 if content == '':
56 # We have been disconnected
57
58=== modified file 'u1db/tests/test_remote_sync_server.py'
59--- u1db/tests/test_remote_sync_server.py 2011-11-01 19:07:07 +0000
60+++ u1db/tests/test_remote_sync_server.py 2011-11-04 14:21:39 +0000
61@@ -14,6 +14,7 @@
62
63 """Tests for the remote synchronization server"""
64
65+import errno
66 import threading
67 import socket
68 import SocketServer
69@@ -32,21 +33,35 @@
70 )
71
72
73-class MyHelloHandler(SocketServer.BaseRequestHandler):
74+class ReadMoreHandler(SocketServer.BaseRequestHandler):
75+
76+ def _read_more(self):
77+ try:
78+ return self.request.recv(1024)
79+ except socket.error, e:
80+ if e.errno in (errno.EBADF,):
81+ # Socket was closed
82+ return
83+ raise
84+
85+
86+class MyHelloHandler(ReadMoreHandler):
87
88 def handle(self):
89- value = self.request.recv(1024)
90+ value = self._read_more()
91 if value == 'hello\n':
92 self.request.sendall('hello to you, too\n')
93
94
95-class TwoMessageHandler(SocketServer.BaseRequestHandler):
96+class TwoMessageHandler(ReadMoreHandler):
97
98 def handle(self):
99- value = self.request.recv(1024)
100+ value = self._read_more()
101+ if value is None:
102+ return
103 self.request.sendall('same to you\n')
104- value = self.request.recv(1024)
105- if value == '':
106+ value = self._read_more()
107+ if value == '' or value is None:
108 return
109 self.request.sendall('goodbye\n')
110

Subscribers

People subscribed via source and target branches