)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7047586bac0c72473b62e1514959ed0f713a453c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a9e2d5ca_31cca6a4","updated":"2022-03-22 18:10:43.000000000","message":"I like the idea of starting with something relatively simple that we could try in production sooner.\n\nI\u0027m going to see if we can share any code to bucketize this. What would ideally hash into buckets? a/c/o? device?","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"74fae58c7a0bb228fd4cc504e21211c0c5d59f5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3b83387d_3a8c8ce5","updated":"2022-03-31 17:08:11.000000000","message":"I put up an alternative based on refactored and shared rate-limiting https://review.opendev.org/c/openstack/swift/+/836046","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a193b09c87967e045797c88dda63a36b45923c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"40ec07b8_b7c3e9f7","updated":"2022-01-14 00:49:10.000000000","message":"Looks interesting, I wonder if we should consolidate all our ratelimit ideas.\n\nSeems this is just a token ratelimiter with a high water mark of reqs_per_second. So feels like something we can turn into a RateLimter class that could be used for more then one application.\n\nAs this might solve some replication thundering herd issues, happy to start with a very specific version like this. I wonder if with things like tsync we might have some way of changing the ratelimit when we have something that knows more about what\u0027s happening in the cluster. Just  thinking out loud 😊","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bb06ddde58a92fb5d49c71d92068e1d767f2e50d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d99542f5_3be97bc1","updated":"2022-03-17 18:34:35.000000000","message":"Tim \u0026 Matt - thank you for thinking about this and making comments\n\nI wonder if we could define in real terms the minimum amount of work we\u0027d need to do to create a change we could release?\n\nI imagine it would be cheaper to make storage server ratelimiting \"smarter\" AFTER we have some basic storage server ratelimiting actaully deployed in prod and actively making SREs life better.  Or maybe at that point we\u0027d just be done?\n\nPlease feel free to push over.","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"85dc2d23ef0c0ad51c0276d2852d6aac2070c51c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"444a7811_574fbc28","updated":"2022-01-13 22:30:42.000000000","message":"if folks are keen on something like this I\u0027ll probably respin to cleanup a few things\n","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"}],"etc/container-server.conf-sample":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":128,"context_line":"# will be used if the value ends with a \u0027%\u0027."},{"line_number":129,"context_line":"# fallocate_reserve \u003d 1%"},{"line_number":130,"context_line":"#"},{"line_number":131,"context_line":"# set the maximum rate limit per worker, beyond this the server will return 503"},{"line_number":132,"context_line":"# and emit a ratelimit statsd metric without logging to shed load, the default"},{"line_number":133,"context_line":"# value of zero will not ever apply ratelimiting"},{"line_number":134,"context_line":"# req_per_second \u003d 0.0"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"12402a28_1bbde38b","line":131,"range":{"start_line":131,"start_character":29,"end_line":131,"end_character":39},"updated":"2022-03-16 17:36:39.000000000","message":"Right; makes sense given the implementation. I wonder if it makes sense to call out explicitly how servers_per_port can be used to get you per-disk limits.","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"}],"swift/common/base_storage_server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        self.log_format \u003d conf.get(\u0027log_format\u0027, LOG_LINE_DEFAULT_FORMAT)"},{"line_number":34,"context_line":"        self.anonymization_method \u003d conf.get(\u0027log_anonymization_method\u0027, \u0027md5\u0027)"},{"line_number":35,"context_line":"        self.anonymization_salt \u003d conf.get(\u0027log_anonymization_salt\u0027, \u0027\u0027)"},{"line_number":36,"context_line":"        self.req_per_second \u003d float(conf.get(\u0027req_per_second\u0027, 0.0))"},{"line_number":37,"context_line":"        self.tokens \u003d max(1, self.req_per_second)"},{"line_number":38,"context_line":"        self.last_token_update \u003d time.time()"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"edbbfc23_3bd9da7d","line":36,"range":{"start_line":36,"start_character":46,"end_line":36,"end_character":49},"updated":"2022-03-16 17:36:39.000000000","message":"Probably ought to expand the config option to \"requests_per_second\".","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":34,"context_line":"        self.anonymization_method \u003d conf.get(\u0027log_anonymization_method\u0027, \u0027md5\u0027)"},{"line_number":35,"context_line":"        self.anonymization_salt \u003d conf.get(\u0027log_anonymization_salt\u0027, \u0027\u0027)"},{"line_number":36,"context_line":"        self.req_per_second \u003d float(conf.get(\u0027req_per_second\u0027, 0.0))"},{"line_number":37,"context_line":"        self.tokens \u003d max(1, self.req_per_second)"},{"line_number":38,"context_line":"        self.last_token_update \u003d time.time()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"eb115b9f_b626b469","line":37,"updated":"2022-03-16 17:36:39.000000000","message":"I think I can justify this starting out at whatever the token cap down at L81 is... I could also justify starting at zero, though.","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"85dc2d23ef0c0ad51c0276d2852d6aac2070c51c","unresolved":true,"context_lines":[{"line_number":35,"context_line":"        self.anonymization_salt \u003d conf.get(\u0027log_anonymization_salt\u0027, \u0027\u0027)"},{"line_number":36,"context_line":"        self.req_per_second \u003d float(conf.get(\u0027req_per_second\u0027, 0.0))"},{"line_number":37,"context_line":"        self.tokens \u003d max(1, self.req_per_second)"},{"line_number":38,"context_line":"        self.last_token_update \u003d time.time()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    @property"},{"line_number":41,"context_line":"    def server_type(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9e2e853c_489ba798","line":38,"updated":"2022-01-13 22:30:42.000000000","message":"i moved this hear as an after thought - if we\u0027re keen on the idea i should update the other storage server __call__, example configs and tests","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":76,"context_line":"    def ratelimit_req(self, now):"},{"line_number":77,"context_line":"        if self.req_per_second \u003e 0.0:"},{"line_number":78,"context_line":"            delta \u003d now - self.last_token_update"},{"line_number":79,"context_line":"            # even if we\u0027ve been asleep awhile we only give it a second"},{"line_number":80,"context_line":"            self.tokens \u003d min(self.tokens + delta * self.req_per_second,"},{"line_number":81,"context_line":"                              self.req_per_second)"},{"line_number":82,"context_line":"            self.last_token_update \u003d now"}],"source_content_type":"text/x-python","patch_set":1,"id":"95a9c7ec_a5be9062","line":79,"updated":"2022-03-16 17:36:39.000000000","message":"This feels like it should maybe be another config option; ratelimit_buffer_time or something. I could see one second\u0027s worth being too much if we\u0027re hoping to allow thousands of requests per second...","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a193b09c87967e045797c88dda63a36b45923c6","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            delta \u003d now - self.last_token_update"},{"line_number":79,"context_line":"            # even if we\u0027ve been asleep awhile we only give it a second"},{"line_number":80,"context_line":"            self.tokens \u003d min(self.tokens + delta * self.req_per_second,"},{"line_number":81,"context_line":"                              self.req_per_second)"},{"line_number":82,"context_line":"            self.last_token_update \u003d now"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            self.tokens \u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"5e826690_13b0e99d","line":81,"updated":"2022-01-14 00:49:10.000000000","message":"OK so the high water mark is a second or rather req_per_second. So there wont be any build up and bursting.\n\nWhen trying to deal with a thundering herd issue this makes sesne. Otherwise we build up and allow thundering herds to actaully happen.\n\nI like the idea, might have a play with it.\n\nWould love to have it start to ratelimit once the server started seeing issues.. but that would mean some kind of system monitoring and a nice pipedream 😊","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            self.tokens \u003d 1"},{"line_number":85,"context_line":"        if self.tokens \u003c\u003d 0:"},{"line_number":86,"context_line":"            self.logger.increment(\u0027ratelimit\u0027)"},{"line_number":87,"context_line":"            return HTTPServiceUnavailable()"},{"line_number":88,"context_line":"        self.tokens -\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"7d4384d7_50044bda","line":86,"updated":"2022-03-16 17:36:39.000000000","message":"Right; so we don\u0027t need any locks because this is the only place eventlet can trampoline, and by now we\u0027ve already made our decision.","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":84,"context_line":"            self.tokens \u003d 1"},{"line_number":85,"context_line":"        if self.tokens \u003c\u003d 0:"},{"line_number":86,"context_line":"            self.logger.increment(\u0027ratelimit\u0027)"},{"line_number":87,"context_line":"            return HTTPServiceUnavailable()"},{"line_number":88,"context_line":"        self.tokens -\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"16fcc7c7_f1964471","line":87,"updated":"2022-03-16 17:36:39.000000000","message":"I\u0027m torn between 503 and 429. *shrug*","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bb06ddde58a92fb5d49c71d92068e1d767f2e50d","unresolved":true,"context_lines":[{"line_number":84,"context_line":"            self.tokens \u003d 1"},{"line_number":85,"context_line":"        if self.tokens \u003c\u003d 0:"},{"line_number":86,"context_line":"            self.logger.increment(\u0027ratelimit\u0027)"},{"line_number":87,"context_line":"            return HTTPServiceUnavailable()"},{"line_number":88,"context_line":"        self.tokens -\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"15bc521e_e008f935","line":87,"in_reply_to":"16fcc7c7_f1964471","updated":"2022-03-17 18:34:35.000000000","message":"i am confident the proxy and consistency engine will \"correctly\" handle 503 from any storage server response (probably with tests) - I think 503 is the best place to start\n\nupdating all callers and tests to verify 429 can make it all the way to the client would be a nice next step","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"}],"swift/container/server.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a193b09c87967e045797c88dda63a36b45923c6","unresolved":true,"context_lines":[{"line_number":226,"context_line":"        if self.tokens \u003c\u003d 0:"},{"line_number":227,"context_line":"            self.logger.increment(\u0027ratelimit\u0027)"},{"line_number":228,"context_line":"            return HTTPServiceUnavailable()"},{"line_number":229,"context_line":"        self.tokens -\u003d 1"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def account_update(self, req, account, container, broker):"},{"line_number":232,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"27585d07_bda6a633","line":229,"updated":"2022-01-14 00:49:10.000000000","message":"Why is this defined again here, isn\u0027t it already defined the same in the base class?","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"}],"test/unit/container/test_server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                break"},{"line_number":228,"context_line":"            success +\u003d 1"},{"line_number":229,"context_line":"        self.assertTrue(resp.status.startswith(\u0027503\u0027), resp.status)"},{"line_number":230,"context_line":"        self.assertGreaterEqual(success, 1)"},{"line_number":231,"context_line":"        # more requests"},{"line_number":232,"context_line":"        self.conf[\u0027req_per_second\u0027] \u003d 10.0"},{"line_number":233,"context_line":"        self.controller \u003d container_server.ContainerController("}],"source_content_type":"text/x-python","patch_set":1,"id":"e62b334c_e4e63df0","line":230,"updated":"2022-03-16 17:36:39.000000000","message":"This will almost certainly be equal, yeah?","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":235,"context_line":"        start \u003d time.time()"},{"line_number":236,"context_line":"        success \u003d 0"},{"line_number":237,"context_line":"        # we should be able to hit the ratelimit pretty quick"},{"line_number":238,"context_line":"        while time.time() \u003c start + 1.0:"},{"line_number":239,"context_line":"            resp \u003d req.get_response(self.controller)"},{"line_number":240,"context_line":"            if not resp.status.startswith(\u0027204\u0027):"},{"line_number":241,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":1,"id":"231bb16f_4f951632","line":238,"updated":"2022-03-16 17:36:39.000000000","message":"IDK about requiring the test take a full second...","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7711baa5d4944eadfa81ec2fc7b0b7a451d0effb","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                break"},{"line_number":242,"context_line":"            success +\u003d 1"},{"line_number":243,"context_line":"        self.assertTrue(resp.status.startswith(\u0027503\u0027), resp.status)"},{"line_number":244,"context_line":"        self.assertGreaterEqual(success, 10)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def test_get_and_validate_policy_index(self):"},{"line_number":247,"context_line":"        # no policy is OK"}],"source_content_type":"text/x-python","patch_set":1,"id":"f9481cf9_657877f8","line":244,"updated":"2022-03-16 17:36:39.000000000","message":"This one might go higher, especially in the gate.","commit_id":"852f16c8adc1df178a72f51862af9d227ed2fc52"}]}