Merge lp:~david-goetz/swift/ratelimit_no_memcache_bug into lp:~hudson-openstack/swift/trunk

Proposed by David Goetz
Status: Merged
Approved by: Mike Barton
Approved revision: 225
Merged at revision: 225
Proposed branch: lp:~david-goetz/swift/ratelimit_no_memcache_bug
Merge into: lp:~hudson-openstack/swift/trunk
Diff against target: 196 lines (+62/-22)
4 files modified
swift/common/memcached.py (+7/-0)
swift/common/middleware/ratelimit.py (+26/-22)
test/unit/common/middleware/test_ratelimit.py (+19/-0)
test/unit/common/test_memcached.py (+10/-0)
To merge this branch: bzr merge lp:~david-goetz/swift/ratelimit_no_memcache_bug
Reviewer Review Type Date Requested Status
Swift Core security contacts Pending
Review via email: mp+51205@code.launchpad.net

Description of the change

ratelimiting does not handle memcache restart

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'swift/common/memcached.py'
--- swift/common/memcached.py 2011-01-04 23:34:43 +0000
+++ swift/common/memcached.py 2011-02-24 20:42:51 +0000
@@ -45,6 +45,10 @@
45 return md5(key).hexdigest()45 return md5(key).hexdigest()
4646
4747
48class MemcacheConnectionError(Exception):
49 pass
50
51
48class MemcacheRing(object):52class MemcacheRing(object):
49 """53 """
50 Simple, consistent-hashed memcache client.54 Simple, consistent-hashed memcache client.
@@ -180,6 +184,7 @@
180 :param delta: amount to add to the value of key (or set as the value184 :param delta: amount to add to the value of key (or set as the value
181 if the key is not found) will be cast to an int185 if the key is not found) will be cast to an int
182 :param timeout: ttl in memcache186 :param timeout: ttl in memcache
187 :raises MemcacheConnectionError:
183 """188 """
184 key = md5hash(key)189 key = md5hash(key)
185 command = 'incr'190 command = 'incr'
@@ -209,6 +214,7 @@
209 return ret214 return ret
210 except Exception, e:215 except Exception, e:
211 self._exception_occurred(server, e)216 self._exception_occurred(server, e)
217 raise MemcacheConnectionError("No Memcached connections succeeded.")
212218
213 def decr(self, key, delta=1, timeout=0):219 def decr(self, key, delta=1, timeout=0):
214 """220 """
@@ -220,6 +226,7 @@
220 value to 0 if the key is not found) will be cast to226 value to 0 if the key is not found) will be cast to
221 an int227 an int
222 :param timeout: ttl in memcache228 :param timeout: ttl in memcache
229 :raises MemcacheConnectionError:
223 """230 """
224 self.incr(key, delta=-delta, timeout=timeout)231 self.incr(key, delta=-delta, timeout=timeout)
225232
226233
=== modified file 'swift/common/middleware/ratelimit.py'
--- swift/common/middleware/ratelimit.py 2011-02-02 21:39:08 +0000
+++ swift/common/middleware/ratelimit.py 2011-02-24 20:42:51 +0000
@@ -18,6 +18,7 @@
1818
19from swift.common.utils import split_path, cache_from_env, get_logger19from swift.common.utils import split_path, cache_from_env, get_logger
20from swift.proxy.server import get_container_memcache_key20from swift.proxy.server import get_container_memcache_key
21from swift.common.memcached import MemcacheConnectionError
2122
2223
23class MaxSleepTimeHitError(Exception):24class MaxSleepTimeHitError(Exception):
@@ -136,28 +137,31 @@
136 :param max_rate: maximum rate allowed in requests per second137 :param max_rate: maximum rate allowed in requests per second
137 :raises: MaxSleepTimeHitError if max sleep time is exceeded.138 :raises: MaxSleepTimeHitError if max sleep time is exceeded.
138 '''139 '''
139 now_m = int(round(time.time() * self.clock_accuracy))140 try:
140 time_per_request_m = int(round(self.clock_accuracy / max_rate))141 now_m = int(round(time.time() * self.clock_accuracy))
141 running_time_m = self.memcache_client.incr(key,142 time_per_request_m = int(round(self.clock_accuracy / max_rate))
142 delta=time_per_request_m)143 running_time_m = self.memcache_client.incr(key,
143 need_to_sleep_m = 0144 delta=time_per_request_m)
144 if (now_m - running_time_m >145 need_to_sleep_m = 0
145 self.rate_buffer_seconds * self.clock_accuracy):146 if (now_m - running_time_m >
146 next_avail_time = int(now_m + time_per_request_m)147 self.rate_buffer_seconds * self.clock_accuracy):
147 self.memcache_client.set(key, str(next_avail_time),148 next_avail_time = int(now_m + time_per_request_m)
148 serialize=False)149 self.memcache_client.set(key, str(next_avail_time),
149 else:150 serialize=False)
150 need_to_sleep_m = \151 else:
151 max(running_time_m - now_m - time_per_request_m, 0)152 need_to_sleep_m = \
152153 max(running_time_m - now_m - time_per_request_m, 0)
153 max_sleep_m = self.max_sleep_time_seconds * self.clock_accuracy154
154 if max_sleep_m - need_to_sleep_m <= self.clock_accuracy * 0.01:155 max_sleep_m = self.max_sleep_time_seconds * self.clock_accuracy
155 # treat as no-op decrement time156 if max_sleep_m - need_to_sleep_m <= self.clock_accuracy * 0.01:
156 self.memcache_client.decr(key, delta=time_per_request_m)157 # treat as no-op decrement time
157 raise MaxSleepTimeHitError("Max Sleep Time Exceeded: %s" %158 self.memcache_client.decr(key, delta=time_per_request_m)
158 need_to_sleep_m)159 raise MaxSleepTimeHitError("Max Sleep Time Exceeded: %s" %
159160 need_to_sleep_m)
160 return float(need_to_sleep_m) / self.clock_accuracy161
162 return float(need_to_sleep_m) / self.clock_accuracy
163 except MemcacheConnectionError:
164 return 0
161165
162 def handle_ratelimit(self, req, account_name, container_name, obj_name):166 def handle_ratelimit(self, req, account_name, container_name, obj_name):
163 '''167 '''
164168
=== modified file 'test/unit/common/middleware/test_ratelimit.py'
--- test/unit/common/middleware/test_ratelimit.py 2011-01-26 22:38:13 +0000
+++ test/unit/common/middleware/test_ratelimit.py 2011-02-24 20:42:51 +0000
@@ -21,12 +21,14 @@
2121
22from swift.common.middleware import ratelimit22from swift.common.middleware import ratelimit
23from swift.proxy.server import get_container_memcache_key23from swift.proxy.server import get_container_memcache_key
24from swift.common.memcached import MemcacheConnectionError
2425
2526
26class FakeMemcache(object):27class FakeMemcache(object):
2728
28 def __init__(self):29 def __init__(self):
29 self.store = {}30 self.store = {}
31 self.error_on_incr = False
3032
31 def get(self, key):33 def get(self, key):
32 return self.store.get(key)34 return self.store.get(key)
@@ -36,6 +38,8 @@
36 return True38 return True
3739
38 def incr(self, key, delta=1, timeout=0):40 def incr(self, key, delta=1, timeout=0):
41 if self.error_on_incr:
42 raise MemcacheConnectionError('Memcache restarting')
39 self.store[key] = int(self.store.setdefault(key, 0)) + int(delta)43 self.store[key] = int(self.store.setdefault(key, 0)) + int(delta)
40 if self.store[key] < 0:44 if self.store[key] < 0:
41 self.store[key] = 045 self.store[key] = 0
@@ -403,6 +407,21 @@
403 start_response)407 start_response)
404 self._run(make_app_call, num_calls, current_rate)408 self._run(make_app_call, num_calls, current_rate)
405409
410 def test_restarting_memcache(self):
411 current_rate = 2
412 num_calls = 5
413 conf_dict = {'account_ratelimit': current_rate}
414 self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp())
415 ratelimit.http_connect = mock_http_connect(204)
416 req = Request.blank('/v/a')
417 req.environ['swift.cache'] = FakeMemcache()
418 req.environ['swift.cache'].error_on_incr = True
419 make_app_call = lambda: self.test_ratelimit(req.environ,
420 start_response)
421 begin = time.time()
422 self._run(make_app_call, num_calls, current_rate, check_time=False)
423 time_took = time.time() - begin
424 self.assert_(round(time_took, 1) == 0) # no memcache, no limiting
406425
407if __name__ == '__main__':426if __name__ == '__main__':
408 unittest.main()427 unittest.main()
409428
=== modified file 'test/unit/common/test_memcached.py'
--- test/unit/common/test_memcached.py 2011-01-04 23:34:43 +0000
+++ test/unit/common/test_memcached.py 2011-02-24 20:42:51 +0000
@@ -50,6 +50,7 @@
50 self.cache = {}50 self.cache = {}
51 self.down = False51 self.down = False
52 self.exc_on_delete = False52 self.exc_on_delete = False
53 self.read_return_none = False
5354
54 def sendall(self, string):55 def sendall(self, string):
55 if self.down:56 if self.down:
@@ -110,6 +111,8 @@
110 else:111 else:
111 self.outbuf += 'NOT_FOUND\r\n'112 self.outbuf += 'NOT_FOUND\r\n'
112 def readline(self):113 def readline(self):
114 if self.read_return_none:
115 return None
113 if self.down:116 if self.down:
114 raise Exception('mock is down')117 raise Exception('mock is down')
115 if '\n' in self.outbuf:118 if '\n' in self.outbuf:
@@ -166,6 +169,9 @@
166 self.assertEquals(memcache_client.get('some_key'), '6')169 self.assertEquals(memcache_client.get('some_key'), '6')
167 memcache_client.incr('some_key', delta=-15)170 memcache_client.incr('some_key', delta=-15)
168 self.assertEquals(memcache_client.get('some_key'), '0')171 self.assertEquals(memcache_client.get('some_key'), '0')
172 mock.read_return_none = True
173 self.assertRaises(memcached.MemcacheConnectionError,
174 memcache_client.incr, 'some_key', delta=-15)
169175
170 def test_decr(self):176 def test_decr(self):
171 memcache_client = memcached.MemcacheRing(['1.2.3.4:11211'])177 memcache_client = memcached.MemcacheRing(['1.2.3.4:11211'])
@@ -179,6 +185,10 @@
179 self.assertEquals(memcache_client.get('some_key'), '11')185 self.assertEquals(memcache_client.get('some_key'), '11')
180 memcache_client.decr('some_key', delta=15)186 memcache_client.decr('some_key', delta=15)
181 self.assertEquals(memcache_client.get('some_key'), '0')187 self.assertEquals(memcache_client.get('some_key'), '0')
188 mock.read_return_none = True
189 self.assertRaises(memcached.MemcacheConnectionError,
190 memcache_client.decr, 'some_key', delta=15)
191
182192
183 def test_retry(self):193 def test_retry(self):
184 logging.getLogger().addHandler(NullLoggingHandler())194 logging.getLogger().addHandler(NullLoggingHandler())