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

[David Goetz]

Changing ratelimiting so that it only limits PUTs/DELETEs.

[gholt (gholt)]

I should've thought of this before, but a/c/o POST and COPY should also be in the same pool as PUT and DELETE.

[lp:~david-goetz/swift/ratelimit_changes]

324. By David Goetz on 2011-07-11

limit posts and copies

[gholt (gholt)]

That won't work exactly. The COPY is sourced from the url and writes to the Destination header.

I don't think it's worth the extra crap to pull the destination header, parse it, and then rate limit, but if you want... Otherwise, just a quick note there stating that COPYs go unlimited should be fine.

review: Needs Fixing

[lp:~david-goetz/swift/ratelimit_changes]

325. By David Goetz on 2011-07-12

limit PUT, DELETE, POSTs to /a/c and /a/c/o

326. By David Goetz on 2011-07-13

fixing what is limited

[David Goetz (david-goetz)]

fixing:
limit PUT/DELETE to /a/c
limit PUT/DELETE/POST to /a/c/o

review: Needs Resubmitting

[lp:~david-goetz/swift/ratelimit_changes]

327. By David Goetz on 2011-07-13

fixing unit test

328. By David Goetz on 2011-07-18

fix docs

[gholt (gholt)]

review: Approve

[John Dickinson (notmyname)]

review: Approve

=== modified file 'doc/source/ratelimit.rst'
--- doc/source/ratelimit.rst	2011-01-21 01:05:44 +0000
+++ doc/source/ratelimit.rst	2011-07-18 22:21:24 +0000
@@ -35,20 +35,17 @@
                                     faster than listed rate). A larger number
                                     will result in larger spikes in rate but
                                     better average accuracy.
-account_ratelimit        0          If set, will limit all requests to
-                                    /account_name and PUTs to
-                                    /account_name/container_name. Number is in
-                                    requests per second
+account_ratelimit        0          If set, will limit PUT and DELETE requests
+                                    to /account_name/container_name.
+                                    Number is in requests per second.
 account_whitelist        ''         Comma separated lists of account names that
                                     will not be rate limited.
 account_blacklist        ''         Comma separated lists of account names that
                                     will not be allowed. Returns a 497 response.
 container_ratelimit_size ''         When set with container_limit_x = r:
                                     for containers of size x, limit requests
-                                    per second to r.  Will limit GET and HEAD
-                                    requests to /account_name/container_name
-				    and PUTs and DELETEs to
-                                    /account_name/container_name/object_name
+                                    per second to r.  Will limit PUT, DELETE,
+                                    and POST requests to /a/c/o.
 ======================== =========  ===========================================
 
 The container rate limits are linearly interpolated from the values given.  A

=== modified file 'swift/common/middleware/ratelimit.py'
--- swift/common/middleware/ratelimit.py	2011-02-23 19:44:36 +0000
+++ swift/common/middleware/ratelimit.py	2011-07-18 22:21:24 +0000
@@ -105,16 +105,15 @@
         :param obj_name: object name from path
         """
         keys = []
-        if self.account_ratelimit and account_name and (
-                not (container_name or obj_name) or
-                (container_name and not obj_name and
-                 req_method in ('PUT', 'DELETE'))):
+        # COPYs are not limited
+        if self.account_ratelimit and \
+                account_name and container_name and not obj_name and \
+                req_method in ('PUT', 'DELETE'):
             keys.append(("ratelimit/%s" % account_name,
                          self.account_ratelimit))
 
-        if account_name and container_name and (
-                (not obj_name and req_method in ('GET', 'HEAD')) or
-                (obj_name and req_method in ('PUT', 'DELETE'))):
+        if account_name and container_name and obj_name and \
+                req_method in ('PUT', 'DELETE', 'POST'):
             container_size = None
             memcache_key = get_container_memcache_key(account_name,
                                                       container_name)

=== modified file 'test/unit/common/middleware/test_ratelimit.py'
--- test/unit/common/middleware/test_ratelimit.py	2011-06-16 18:09:15 +0000
+++ test/unit/common/middleware/test_ratelimit.py	2011-07-18 22:21:24 +0000
@@ -139,13 +139,16 @@
 
 class TestRateLimit(unittest.TestCase):
 
+    def _reset_time(self):
+        global time_ticker
+        time_ticker = 0
+
     def setUp(self):
-        global time_ticker
-        time_ticker = 0
         self.was_sleep = eventlet.sleep
         eventlet.sleep = mock_sleep
         self.was_time = time.time
         time.time = mock_time
+        self._reset_time()
 
     def tearDown(self):
         eventlet.sleep = self.was_sleep
@@ -186,31 +189,34 @@
                                                 logger=FakeLogger())
         the_app.memcache_client = fake_memcache
         self.assertEquals(len(the_app.get_ratelimitable_key_tuples(
-                    'GET', 'a', None, None)), 1)
-        self.assertEquals(len(the_app.get_ratelimitable_key_tuples(
-                    'POST', 'a', 'c', None)), 0)
+                    'DELETE', 'a', None, None)), 0)
         self.assertEquals(len(the_app.get_ratelimitable_key_tuples(
                     'PUT', 'a', 'c', None)), 1)
         self.assertEquals(len(the_app.get_ratelimitable_key_tuples(
-                    'GET', 'a', 'c', None)), 1)
+                    'DELETE', 'a', 'c', None)), 1)
         self.assertEquals(len(the_app.get_ratelimitable_key_tuples(
                     'GET', 'a', 'c', 'o')), 0)
         self.assertEquals(len(the_app.get_ratelimitable_key_tuples(
                     'PUT', 'a', 'c', 'o')), 1)
 
-    def test_ratelimit(self):
+    def test_account_ratelimit(self):
         current_rate = 5
         num_calls = 50
         conf_dict = {'account_ratelimit': current_rate}
         self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp())
         ratelimit.http_connect = mock_http_connect(204)
-        req = Request.blank('/v/a')
-        req.environ['swift.cache'] = FakeMemcache()
-        make_app_call = lambda: self.test_ratelimit(req.environ,
-                                                    start_response)
-        begin = time.time()
-        self._run(make_app_call, num_calls, current_rate)
-        self.assertEquals(round(time.time() - begin, 1), 9.8)
+        for meth, exp_time in [('DELETE', 9.8), ('GET', 0),
+                           ('POST', 0), ('PUT', 9.8)]:
+            req = Request.blank('/v/a%s/c' % meth)
+            req.method = meth
+            req.environ['swift.cache'] = FakeMemcache()
+            make_app_call = lambda: self.test_ratelimit(req.environ,
+                                                        start_response)
+            begin = time.time()
+            self._run(make_app_call, num_calls, current_rate,
+                      check_time=bool(exp_time))
+            self.assertEquals(round(time.time() - begin, 1), exp_time)
+            self._reset_time()
 
     def test_ratelimit_set_incr(self):
         current_rate = 5
@@ -218,7 +224,8 @@
         conf_dict = {'account_ratelimit': current_rate}
         self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp())
         ratelimit.http_connect = mock_http_connect(204)
-        req = Request.blank('/v/a')
+        req = Request.blank('/v/a/c')
+        req.method = 'PUT'
         req.environ['swift.cache'] = FakeMemcache()
         req.environ['swift.cache'].init_incr_return_neg = True
         make_app_call = lambda: self.test_ratelimit(req.environ,
@@ -306,7 +313,8 @@
         self.test_ratelimit = dummy_filter_factory(conf_dict)(FakeApp())
         ratelimit.http_connect = mock_http_connect(204)
         self.test_ratelimit.log_sleep_time_seconds = .00001
-        req = Request.blank('/v/a')
+        req = Request.blank('/v/a/c')
+        req.method = 'PUT'
         req.environ['swift.cache'] = FakeMemcache()
 
         time_override = [0, 0, 0, 0, None]
@@ -335,7 +343,7 @@
                                                 logger=FakeLogger())
         the_app.memcache_client = fake_memcache
         req = lambda: None
-        req.method = 'GET'
+        req.method = 'PUT'
 
         class rate_caller(Thread):
 
@@ -346,7 +354,7 @@
             def run(self):
                 for j in range(num_calls):
                     self.result = the_app.handle_ratelimit(req, self.myname,
-                                                           None, None)
+                                                           'c', None)
 
         nt = 15
         begin = time.time()
@@ -361,45 +369,6 @@
         time_took = time.time() - begin
         self.assertEquals(1.5, round(time_took, 1))
 
-    def test_ratelimit_acc_vrs_container(self):
-        conf_dict = {'clock_accuracy': 1000,
-                     'account_ratelimit': 10,
-                     'max_sleep_time_seconds': 4,
-                     'container_ratelimit_10': 6,
-                     'container_ratelimit_50': 2,
-                     'container_ratelimit_75': 1}
-        self.test_ratelimit = dummy_filter_factory(conf_dict)(FakeApp())
-        ratelimit.http_connect = mock_http_connect(204)
-        req = Request.blank('/v/a/c')
-        req.environ['swift.cache'] = FakeMemcache()
-        cont_key = get_container_memcache_key('a', 'c')
-
-        class rate_caller(Thread):
-
-            def __init__(self, parent, name):
-                Thread.__init__(self)
-                self.parent = parent
-                self.name = name
-
-            def run(self):
-                self.result = self.parent.test_ratelimit(req.environ,
-                                                         start_response)
-
-        def runthreads(threads, nt):
-            for i in range(nt):
-                rc = rate_caller(self, "thread %s" % i)
-                rc.start()
-                threads.append(rc)
-            for thread in threads:
-                thread.join()
-        begin = time.time()
-        req.environ['swift.cache'].set(cont_key, {'container_size': 20})
-        begin = time.time()
-        threads = []
-        runthreads(threads, 3)
-        time_took = time.time() - begin
-        self.assertEquals(round(time_took, 1), .4)
-
     def test_call_invalid_path(self):
         env = {'REQUEST_METHOD': 'GET',
                'SCRIPT_NAME': '',
@@ -441,7 +410,8 @@
         conf_dict = {'account_ratelimit': current_rate}
         self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp())
         ratelimit.http_connect = mock_http_connect(204)
-        req = Request.blank('/v/a')
+        req = Request.blank('/v/a/c')
+        req.method = 'PUT'
         req.environ['swift.cache'] = FakeMemcache()
         req.environ['swift.cache'].error_on_incr = True
         make_app_call = lambda: self.test_ratelimit(req.environ,