)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Alistair Coles \u003calistairncoles@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-04-22 18:28:31 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add backend rate limiting middleware"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a fairly blunt tool: ratelimiting is per device and"},{"line_number":10,"context_line":"applied independently in each worker, but this at least provides"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"2d966f70_bc7a47de","line":7,"updated":"2022-04-28 20:30:57.000000000","message":"I\u0027d prefer it was called load shedding; but maybe it won\u0027t be difficult to keep front-end and back-end ratelimiting separate.","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Alistair Coles \u003calistairncoles@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-04-22 18:28:31 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add backend rate limiting middleware"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a fairly blunt tool: ratelimiting is per device and"},{"line_number":10,"context_line":"applied independently in each worker, but this at least provides"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"d03503de_9d9dc5ce","line":7,"in_reply_to":"2d966f70_bc7a47de","updated":"2022-05-04 13:48:10.000000000","message":"Ack","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"OPTIONS, REPLICATE, SSYNC methods are not rate-limited. Note that"},{"line_number":17,"context_line":"SSYNC sub-requests are passed directly to the object server app"},{"line_number":18,"context_line":"and will not pass though this middleware."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I78b59a081698a6bff0d74cbac7525e28f7b5d7c1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"57b32bb2_21b4b0d3","line":18,"updated":"2022-04-28 20:30:57.000000000","message":"i very much WOULD like to ratelimit REPLICATE (re-hash) requests tho","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"OPTIONS, REPLICATE, SSYNC methods are not rate-limited. Note that"},{"line_number":17,"context_line":"SSYNC sub-requests are passed directly to the object server app"},{"line_number":18,"context_line":"and will not pass though this middleware."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I78b59a081698a6bff0d74cbac7525e28f7b5d7c1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9cca3971_a29874f2","line":18,"in_reply_to":"57b32bb2_21b4b0d3","updated":"2022-05-04 13:48:10.000000000","message":"Done","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fa1b83bcd80a7d366c524e3d048d69549b9e14a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"af4af750_4968168f","updated":"2022-03-31 17:08:46.000000000","message":"this is an alternative to https://review.opendev.org/c/openstack/swift/+/824653","commit_id":"9b19704d03513b73ca765d06aa0a74916ff2ccb8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fccbc7b5_28ef1631","updated":"2022-04-28 20:30:57.000000000","message":"this looks great!  I did a little functional validation with vsio [1].  I wonder if you\u0027re happy with the tests?\n\n1. https://github.com/NVIDIA/vagrant-swift-all-in-one/pull/125","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"96489f5d_59f06974","in_reply_to":"fccbc7b5_28ef1631","updated":"2022-05-04 13:48:10.000000000","message":"I wasn\u0027t happy with the tests, I\u0027m glad you concur :)","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bed73433bdca4dbc1224d6ef40ac60e02b941d2c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"682bfd91_41fce875","updated":"2022-05-18 06:53:33.000000000","message":"This is looking great. We probably should also add it to the SAIO pipelines too.\n\nHolding back on a +2 until I can give it a whirl in a SAIO. Got it checked out, now just need some time to run it. Tomorrow after meetings is the plan, so we can get this awesome work landed.","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b3f544d9d9ce164cf33918f179799d96c902454f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"98e2a292_58a9ae30","updated":"2022-05-17 22:27:17.000000000","message":"looks pretty good!","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7296fe81a51c5e59987f2a3a2e1063a2e8b6ae6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"eb57e62c_348c126c","updated":"2022-05-20 06:05:33.000000000","message":"Hmm, so i ran this on a VSAIO. Set the backend ratelimit to 10. Then simutated a bit of traffic:\n\n  for x in $(seq 50); do for y in $(seq 50); do curl -i http://localhost:8090/v1/AUTH_test/c/o$x$y -X PUT --data-binary 1234; done; done\n\nwhich went fine for a short while, I saw 529s in the logs. Then all of a sudden, a few error limited, then more, then all were meaning all I\u0027d get back were 503s. \n\n  May 20 05:49:52 saio2 proxy-server[44772]: STDERR: (44772) accepted (\u0027127.0.0.1\u0027, 35078)\n  May 20 05:49:52 saio2 object-6010[44761]: STDERR: (44761) accepted (\u0027127.0.0.1\u0027, 55228)\n  May 20 05:49:52 saio2 object-6030[44771]: STDERR: (44771) accepted (\u0027127.0.0.1\u0027, 48630)\n  May 20 05:49:52 saio2 object-6020[44760]: STDERR: (44760) accepted (\u0027127.0.0.1\u0027, 38768)\n  May 20 05:49:52 saio2 object-6010[44761]: STDERR: 127.0.0.1 - - [20/May/2022 05:49:52] \"PUT \n /sdb1/706/AUTH_test/c/o56 HTTP/1.1\" 529 311 0.000743\n  May 20 05:49:52 saio2 proxy-server[44772]: ERROR 529 Expect: 100-continue From Object Server 127.0.0.1:6010/sdb1 (txn: tx7ba628dfe8924733a837c-0062872c00)\n  May 20 05:49:52 saio2 proxy-server[44772]: Node error limited 127.0.0.4:6040 (sdb4) (txn: tx7ba628dfe8924733a837c-0062872c00) (client_ip: 127.0.0.1)\n  May 20 05:49:52 saio2 container-6011[44768]: STDERR: (44768) accepted (\u0027127.0.0.1\u0027, 36952)\n  May 20 05:49:52 saio2 container-6011[44768]: STDERR: 127.0.0.1 - - [20/May/2022 05:49:52] \"PUT /sdb1/61/AUTH_test/c/o56 HTTP/1.1\" 529 292 0.000292\n  May 20 05:49:52 saio2 object-6030[44771]: ERROR Container update failed (saving for async update later): 529 response from 127.0.0.1:6011/sdb1 (txn: tx7ba628dfe8924733a837c-0062872c00)\n  May 20 05:49:52 saio2 container-6021[44766]: STDERR: (44766) accepted (\u0027127.0.0.1\u0027, 43730)\n  May 20 05:49:52 saio2 container-6021[44766]: 127.0.0.1 - - [20/May/2022:05:49:52 +0000] \"PUT /sdb2/61/AUTH_test/c/o56\" 201 - \"PUT http://localhost:8090/sdb2/706/AUTH_test/c/o56\" \"tx7ba628dfe8924733a837c-0062872c00\" \"object-server 44760\" 0.0008 \"-\" 44766 0\n  May 20 05:49:52 saio2 container-6021[44766]: STDERR: 127.0.0.1 - - [20/May/2022 05:49:52] \"PUT /sdb2/61/AUTH_test/c/o56 HTTP/1.1\" 201 120 0.001479 (txn: tx7ba628dfe8924733a837c-0062872c00)\n  May 20 05:49:52 saio2 object-6030[44771]: 127.0.0.1 - - [20/May/2022:05:49:52 +0000] \"PUT /sdb3/706/AUTH_test/c/o56\" 201 - \"PUT http://localhost:8090/v1/AUTH_test/c/o56\" \"tx7ba628dfe8924733a837c-0062872c00\" \"proxy-server 44772\" 0.0220 \"-\" 44771 0\n  May 20 05:49:52 saio2 object-6030[44771]: STDERR: 127.0.0.1 - - [20/May/2022 05:49:52] \"PUT /sdb3/706/AUTH_test/c/o56 HTTP/1.1\" 201 181 0.023450 (txn: tx7ba628dfe8924733a837c-0062872c00)\n  May 20 05:49:52 saio2 object-6020[44760]: 127.0.0.1 - - [20/May/2022:05:49:52 +0000] \"PUT /sdb2/706/AUTH_test/c/o56\" 201 - \"PUT http://localhost:8090/v1/AUTH_test/c/o56\" \"tx7ba628dfe8924733a837c-0062872c00\" \"proxy-server 44772\" 0.0282 \"-\" 44760 0\n  May 20 05:49:52 saio2 object-6020[44760]: STDERR: 127.0.0.1 - - [20/May/2022 05:49:52] \"PUT /sdb2/706/AUTH_test/c/o56 HTTP/1.1\" 201 181 0.038873 (txn: tx7ba628dfe8924733a837c-0062872c00)\n  May 20 05:49:52 saio2 proxy-server[44772]: 127.0.0.1 127.0.0.1 20/May/2022/05/49/52 PUT /v1/AUTH_test/c/o56 HTTP/1.0 201 - curl/7.81.0 - 4 - - tx7ba628dfe8924733a837c-0062872c00 - 0.0462 - - 1653025792.590838194 1653025792.637016296 0\n  May 20 05:49:52 saio2 proxy-server[44772]: STDERR: 127.0.0.1 - - [20/May/2022 05:49:52] \"PUT /v1/AUTH_test/c/o56 HTTP/1.1\" 201 314 0.047483 (txn: tx7ba628dfe8924733a837c-0062872c00)\n  May 20 05:49:52 saio2 proxy-server[44772]: STDERR: (44772) accepted (\u0027127.0.0.1\u0027, 35080)\n  May 20 05:49:52 saio2 proxy-server[44772]: Node error limited 127.0.0.4:6040 (sdb4) (txn: tx2b51a22c740e486587eda-0062872c00) (client_ip: 127.0.0.1)\n  May 20 05:49:52 saio2 object-6030[44771]: STDERR: (44771) accepted (\u0027127.0.0.1\u0027, 48632)\n  May 20 05:49:52 saio2 object-6020[44760]: STDERR: (44760) accepted (\u0027127.0.0.1\u0027, 38770)\n  May 20 05:49:52 saio2 proxy-server[44772]: Node error limited 127.0.0.1:6010 (sdb1) (txn: tx2b51a22c740e486587eda-0062872c00)\n\nIt does make we worried aobut error limiting. I know you do have a patch to not error limit 529s. But does make we wonder if its something we do want before we ever use this. Especially when the `Node error limited` logs are logged at debug, so we wont see them in prod. So long as error limiting is invisible to the logs, I don\u0027t like adding something that would increase it. Worse this happened on my VSAIO where I only have 4 nodes, in prod what would happen... they\u0027d all get palmed off further and further down the handoff stack.","commit_id":"f84b8475ab63e85a74f2cefcb51cc47e88f0a8ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4ced2d6a546c76d1671162306af5e7eb563fe568","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a4554f2d_9c755562","in_reply_to":"eb57e62c_348c126c","updated":"2022-05-20 14:01:39.000000000","message":"I think more *transparency* in proxy error limiting would be super useful to invest in: maybe some statsd in addition to debug logging?\n\nIt\u0027s orthogonal tho, how could having NO control over a node getting error limited because of Timeouts possibly be BETTER than ops having the option to push back against a flooded storage service with 529s.  So what if a few 503s make it out to the client - we are doing that ANYWAY.\n\nIs the name \"backend rate limiting\" vs. \"backend load shedding\" making people think this is supposed to somehow be a tool for the services to communitcate to the proxy \"slow down\" instead of \"go away\"?\n\nMaybe instead of limiting to a \"rate\" it would be easier to understand if the limit was on \"in-flight\" - part of the reason we\u0027ve seen servers get bogged down, fall over, and crap on everything is because the responses start getting SLOW and the backlog grows without bound.","commit_id":"f84b8475ab63e85a74f2cefcb51cc47e88f0a8ac"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fce7104beed15f1be650e8addf3283e65a6fa2a5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"08840539_8f59195c","in_reply_to":"eb57e62c_348c126c","updated":"2022-05-20 13:49:53.000000000","message":"There are mixed opinions on whether the proxy should error limit or not. I initially felt strongly it should not, because the time scales of error limiting vs rate limiting are very different. But I also recognise that not error limiting in the proxy results in increased backend requests as proxy keeps trying \u0027too busy\u0027 nodes.\n\nThere is some mitigation in that there are N to 1 proxies to backend ratelimiter, so on average each proxy will receive requests_per_second/N 529s from a \u0027too busy\u0027 node.\n\nAnother mitigation is to increase the rate_buffer closer to the error limit interval. In extreme, if rate_buffer \u003d\u003d error limit interval then the proxy would be appropriately backing off (i.e. backend has stacked up 60s worth of requests and proxy error limits for 60s). But this does mean the backend stacking up 60s worth of requests.\n\nA combination of the 2 mitigations may be the answer, but we\u0027ll need to carefully tune and learn.\n\nFWIW, long term I\u0027d like the proxy to grow a more sensitive error/backoff algorithm. For example, order backend nodes based on rate of 529s, with thresholds to fully error limit nodes.","commit_id":"f84b8475ab63e85a74f2cefcb51cc47e88f0a8ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4ced2d6a546c76d1671162306af5e7eb563fe568","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"56bcbab2_6dca3a4b","updated":"2022-05-20 14:01:39.000000000","message":"\u003e A combination of the 2 mitigations may be the answer, but we\u0027ll need to carefully tune and learn.\n\nI think it\u0027s just as hard for me to believe \"a proxy should NEVER error limit a server returning 529s\"  as it is \"we can\u0027t possibly USE backned ratelimits as long as the proxy might error limit\"\n\nI think this is a step we could take and improve, and the code is ready - let\u0027s ALSO improve proxy error limiting; which could in the future include deliniating not just how we handle a 507 from a 503 or a Timeout but now... a 529!","commit_id":"ccaf49a00cea1990027eb699bff8f10c664b2c9a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"61f121612308b80ce7112c78db54d436fa77a2d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7f1a7cdf_c8b831ef","updated":"2022-08-30 04:57:06.000000000","message":"This has a +2 from clay and I and was months ago. So going to +A this one.","commit_id":"ccaf49a00cea1990027eb699bff8f10c664b2c9a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7d54d8c600333b5bd06fe57e0cd47e8199d2d8f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4020e689_8c63f57c","updated":"2022-05-23 05:09:12.000000000","message":"kk, fair enough. It\u0027s also hard to know what to tune until we have something. So I\u0027m on board. Still would like the confirugation in the SAIO configuration, but I\u0027ve gone and done that in a follow up (after reloadable config) as we might as well configure it that way in SAIO: https://review.opendev.org/c/openstack/swift/+/842909","commit_id":"ccaf49a00cea1990027eb699bff8f10c664b2c9a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fe06dfc079da1f30dfb5fc0668e628b78d2d6e25","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"f4814233_11e55a3a","updated":"2022-05-23 04:53:58.000000000","message":"recheck","commit_id":"ccaf49a00cea1990027eb699bff8f10c664b2c9a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"36c0b2cc874c89cfd344f0e5036d3c9cfeb3ba85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7a43310f_68d88c78","updated":"2022-08-30 04:26:53.000000000","message":"reheck","commit_id":"ccaf49a00cea1990027eb699bff8f10c664b2c9a"}],"etc/object-server.conf-sample":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":111,"context_line":"# ionice_priority \u003d"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"[pipeline:main]"},{"line_number":114,"context_line":"pipeline \u003d healthcheck recon object-server"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"[app:object-server]"},{"line_number":117,"context_line":"use \u003d egg:swift#object"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"9c31e97d_b4871070","line":114,"updated":"2022-04-28 20:30:57.000000000","message":"should we update the example pipeline?","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":111,"context_line":"# ionice_priority \u003d"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"[pipeline:main]"},{"line_number":114,"context_line":"pipeline \u003d healthcheck recon object-server"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"[app:object-server]"},{"line_number":117,"context_line":"use \u003d egg:swift#object"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"0012825a_81a4feb0","line":114,"in_reply_to":"9c31e97d_b4871070","updated":"2022-05-04 13:48:10.000000000","message":"Done","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"}],"swift/common/middleware/backend_ratelimit.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"# Used by get_swift_info and register_swift_info to store information about"},{"line_number":17,"context_line":"# the swift cluster."},{"line_number":18,"context_line":"import time"},{"line_number":19,"context_line":"from collections import defaultdict"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"dd6b7b88_71836ef7","line":17,"updated":"2022-04-28 20:30:57.000000000","message":"meta question: should we pull recon and this new guy out of commmon/middleware or make a new proxy sub-folder or something?","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"# Used by get_swift_info and register_swift_info to store information about"},{"line_number":17,"context_line":"# the swift cluster."},{"line_number":18,"context_line":"import time"},{"line_number":19,"context_line":"from collections import defaultdict"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"10fec8f8_08f34125","line":17,"in_reply_to":"dd6b7b88_71836ef7","updated":"2022-05-04 13:48:10.000000000","message":"Good question. recon and backend_ratelimit are \"common\" to all backends. We also have things like base_storage_server db_replicator in common which are not common to *all* services. I don\u0027t feel the need for more directory depth, but maybe middleware doesn\u0027t need to be under common?? and could then be split into backend vs proxy??","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":77,"context_line":"    def backend_ratelimit_filter(app):"},{"line_number":78,"context_line":"        return BackendRateLimitMiddleware(app, conf)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    return backend_ratelimit_filter"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa9ca849_dfa8497d","line":80,"updated":"2022-04-28 20:30:57.000000000","message":"this module is so short and sweet - I love it\n\ncan we make it have auto-reloading with configurable per device per method rate limits and dynamic tuning from iostat?  And a pony!?","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    def backend_ratelimit_filter(app):"},{"line_number":78,"context_line":"        return BackendRateLimitMiddleware(app, conf)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    return backend_ratelimit_filter"}],"source_content_type":"text/x-python","patch_set":7,"id":"1b7a8d19_aeeb4e5f","line":80,"in_reply_to":"fa9ca849_dfa8497d","updated":"2022-05-04 13:48:10.000000000","message":"don\u0027t nerd-snipe! ;-)","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"}],"swift/common/utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b3f544d9d9ce164cf33918f179799d96c902454f","unresolved":true,"context_lines":[{"line_number":373,"context_line":"            raise ValueError"},{"line_number":374,"context_line":"    except ValueError:"},{"line_number":375,"context_line":"        raise ValueError(\u0027Value must be a non-negative float number, not \"%s\".\u0027"},{"line_number":376,"context_line":"                         % value)"},{"line_number":377,"context_line":"    return value"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9078c0be_24887a01","line":376,"updated":"2022-05-17 22:27:17.000000000","message":"except ValueError: raise ValueError\n\n... fine","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4a108d64a0bd9c2acbb47771b8479a68cf1741a8","unresolved":true,"context_lines":[{"line_number":373,"context_line":"            raise ValueError"},{"line_number":374,"context_line":"    except ValueError:"},{"line_number":375,"context_line":"        raise ValueError(\u0027Value must be a non-negative float number, not \"%s\".\u0027"},{"line_number":376,"context_line":"                         % value)"},{"line_number":377,"context_line":"    return value"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"a9c11b08_4a23f387","line":376,"in_reply_to":"3ab49c91_b2e52bbf","updated":"2022-05-18 10:36:55.000000000","message":"see lines 413, 427 ... same pattern (I\u0027m not sure how a TypeError would get raised in those functions)","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fce7104beed15f1be650e8addf3283e65a6fa2a5","unresolved":true,"context_lines":[{"line_number":373,"context_line":"            raise ValueError"},{"line_number":374,"context_line":"    except ValueError:"},{"line_number":375,"context_line":"        raise ValueError(\u0027Value must be a non-negative float number, not \"%s\".\u0027"},{"line_number":376,"context_line":"                         % value)"},{"line_number":377,"context_line":"    return value"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5668f34b_99c3f355","line":376,"in_reply_to":"5cc06b7e_1f84fadb","updated":"2022-05-20 13:49:53.000000000","message":"thanks for pointing that out. Fixed.","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bed73433bdca4dbc1224d6ef40ac60e02b941d2c","unresolved":true,"context_lines":[{"line_number":373,"context_line":"            raise ValueError"},{"line_number":374,"context_line":"    except ValueError:"},{"line_number":375,"context_line":"        raise ValueError(\u0027Value must be a non-negative float number, not \"%s\".\u0027"},{"line_number":376,"context_line":"                         % value)"},{"line_number":377,"context_line":"    return value"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3ab49c91_b2e52bbf","line":376,"in_reply_to":"9078c0be_24887a01","updated":"2022-05-18 06:53:33.000000000","message":"It is an interesting try/except usage but does get the error message the way we need it :)","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d9ca9555c8064caf8c014e36e63fd3822a05df70","unresolved":true,"context_lines":[{"line_number":373,"context_line":"            raise ValueError"},{"line_number":374,"context_line":"    except ValueError:"},{"line_number":375,"context_line":"        raise ValueError(\u0027Value must be a non-negative float number, not \"%s\".\u0027"},{"line_number":376,"context_line":"                         % value)"},{"line_number":377,"context_line":"    return value"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5cc06b7e_1f84fadb","line":376,"in_reply_to":"a9c11b08_4a23f387","updated":"2022-05-18 23:14:44.000000000","message":"It\u0027s for the sake of idioms like\n\n config_positive_int_value(conf.get(\u0027key that may not exist\u0027))\n\nI think. At any rate:\n\n \u003e\u003e\u003e float(None)\n Traceback (most recent call last):\n   File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n TypeError: float() argument must be a string or a number, not \u0027NoneType\u0027\n \u003e\u003e\u003e int(None)\n Traceback (most recent call last):\n   File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n TypeError: int() argument must be a string, a bytes-like object or a number, not \u0027NoneType\u0027","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"}],"test/unit/common/middleware/test_backend_ratelimit.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        conf \u003d {\u0027requests_per_device_rate_buffer\u0027: -1}"},{"line_number":65,"context_line":"        factory \u003d backend_ratelimit.filter_factory(conf)"},{"line_number":66,"context_line":"        self.assertRaises(ValueError, factory, self.swift)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _do_test_ratelimit(self, method, req_per_sec, rate_buffer):"},{"line_number":69,"context_line":"        # apply a ratelimit"}],"source_content_type":"text/x-python","patch_set":7,"id":"57947896_23324fee","line":66,"updated":"2022-04-28 20:30:57.000000000","message":"I prefer using assertRaises as a context mgr and checking the ctx.err string to make sure the ValueError is the one I expect","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        conf \u003d {\u0027requests_per_device_rate_buffer\u0027: -1}"},{"line_number":65,"context_line":"        factory \u003d backend_ratelimit.filter_factory(conf)"},{"line_number":66,"context_line":"        self.assertRaises(ValueError, factory, self.swift)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _do_test_ratelimit(self, method, req_per_sec, rate_buffer):"},{"line_number":69,"context_line":"        # apply a ratelimit"}],"source_content_type":"text/x-python","patch_set":7,"id":"aedc5277_bd82dac7","line":66,"in_reply_to":"57947896_23324fee","updated":"2022-05-04 13:48:10.000000000","message":"me too. In this case the ValueError has no message :/ will fix that","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        start \u003d time.time()"},{"line_number":76,"context_line":"        success \u003d defaultdict(int)"},{"line_number":77,"context_line":"        ratelimited \u003d 0"},{"line_number":78,"context_line":"        while time.time() \u003c start + 0.09:"},{"line_number":79,"context_line":"            for dev in [\u0027sda1\u0027, \u0027sda2\u0027, \u0027sda3\u0027]:"},{"line_number":80,"context_line":"                req \u003d Request.blank(\u0027/%s/99/a/c/o\u0027 % dev,"},{"line_number":81,"context_line":"                                    environ\u003d{\u0027REQUEST_METHOD\u0027: method})"}],"source_content_type":"text/x-python","patch_set":7,"id":"27d3c6ba_d3cc935e","line":78,"updated":"2022-04-28 20:30:57.000000000","message":"given how well we\u0027ve tested the RateLimiter class would this look better with mocked time?","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        start \u003d time.time()"},{"line_number":76,"context_line":"        success \u003d defaultdict(int)"},{"line_number":77,"context_line":"        ratelimited \u003d 0"},{"line_number":78,"context_line":"        while time.time() \u003c start + 0.09:"},{"line_number":79,"context_line":"            for dev in [\u0027sda1\u0027, \u0027sda2\u0027, \u0027sda3\u0027]:"},{"line_number":80,"context_line":"                req \u003d Request.blank(\u0027/%s/99/a/c/o\u0027 % dev,"},{"line_number":81,"context_line":"                                    environ\u003d{\u0027REQUEST_METHOD\u0027: method})"}],"source_content_type":"text/x-python","patch_set":7,"id":"e7de9f88_ffba0929","line":78,"in_reply_to":"27d3c6ba_d3cc935e","updated":"2022-05-04 13:48:10.000000000","message":"agree, these tests are flakey","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                self.assertEqual(2, success, success_per_dev)"},{"line_number":116,"context_line":"            success_per_dev \u003d self._do_test_ratelimit(method, 10, 1)"},{"line_number":117,"context_line":"            for dev, success in success_per_dev.items():"},{"line_number":118,"context_line":"                self.assertEqual(11, success)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        do_test_ratelimit(\u0027GET\u0027)"},{"line_number":121,"context_line":"        do_test_ratelimit(\u0027HEAD\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"241b1d80_191e3435","line":118,"updated":"2022-04-28 20:30:57.000000000","message":"would success_per_dev ever be empty?","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                self.assertEqual(2, success, success_per_dev)"},{"line_number":116,"context_line":"            success_per_dev \u003d self._do_test_ratelimit(method, 10, 1)"},{"line_number":117,"context_line":"            for dev, success in success_per_dev.items():"},{"line_number":118,"context_line":"                self.assertEqual(11, success)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        do_test_ratelimit(\u0027GET\u0027)"},{"line_number":121,"context_line":"        do_test_ratelimit(\u0027HEAD\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"27375347_a9df6fb7","line":118,"in_reply_to":"241b1d80_191e3435","updated":"2022-05-04 13:48:10.000000000","message":"Done","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7e26d2eb499a9685e7368f375c4f7c06ee986085","unresolved":true,"context_lines":[{"line_number":129,"context_line":"            # verify no rate-limiting"},{"line_number":130,"context_line":"            success_per_dev \u003d self._do_test_ratelimit(method, 10, 10)"},{"line_number":131,"context_line":"            for dev, success in success_per_dev.items():"},{"line_number":132,"context_line":"                self.assertGreater(success, 20)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        do_test_no_ratelimit(\u0027OPTIONS\u0027)"},{"line_number":135,"context_line":"        do_test_no_ratelimit(\u0027REPLICATE\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"f0c40783_b4ca97cb","line":132,"updated":"2022-04-28 20:30:57.000000000","message":"the meta assertion is that our test machine is \"fast enough\" to do 20 OPTIONS queries in 0.9s?","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9204ff4b560dbf86ad8315e21a005f54da50bf0c","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            # verify no rate-limiting"},{"line_number":130,"context_line":"            success_per_dev \u003d self._do_test_ratelimit(method, 10, 10)"},{"line_number":131,"context_line":"            for dev, success in success_per_dev.items():"},{"line_number":132,"context_line":"                self.assertGreater(success, 20)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        do_test_no_ratelimit(\u0027OPTIONS\u0027)"},{"line_number":135,"context_line":"        do_test_no_ratelimit(\u0027REPLICATE\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"0f3eac5b_7d81970e","line":132,"in_reply_to":"f0c40783_b4ca97cb","updated":"2022-05-04 13:48:10.000000000","message":"Done","commit_id":"64f7d5dddc6a7d5499857b170eec49f02f3903a7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"bed73433bdca4dbc1224d6ef40ac60e02b941d2c","unresolved":true,"context_lines":[{"line_number":29,"context_line":"from test.unit.common.middleware.helpers import FakeSwift"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class FakeApp(object):"},{"line_number":33,"context_line":"    def __init__(self):"},{"line_number":34,"context_line":"        self.calls \u003d []"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"e510a296_50253067","line":32,"range":{"start_line":32,"start_character":6,"end_line":32,"end_character":13},"updated":"2022-05-18 06:53:33.000000000","message":"Another FakeApp :(\n\nNot a blocker, but we do seem to have a truckload of these now. According to another repo I have checked out:\n\n $ grep -ri \"class FakeApp\" test  |wc -l\n 45\n\nSurely we can have a base one back in the test namespace and the add to it, or even extend it into other versions as required. Anyway, maybe a low hanging fruit task.","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b3f544d9d9ce164cf33918f179799d96c902454f","unresolved":true,"context_lines":[{"line_number":104,"context_line":"                        self.assertTrue(resp.status.startswith("},{"line_number":105,"context_line":"                            \u0027529 Too Many Backend Requests\u0027))"},{"line_number":106,"context_line":"                        ratelimited +\u003d 1"},{"line_number":107,"context_line":"                fake_time[0] +\u003d 0.01"},{"line_number":108,"context_line":"        self.assertEqual("},{"line_number":109,"context_line":"            ratelimited,"},{"line_number":110,"context_line":"            logger.get_increment_counts().get(\u0027backend.ratelimit\u0027, 0))"}],"source_content_type":"text/x-python","patch_set":8,"id":"9c2ddfa9_e7f33baf","line":107,"updated":"2022-05-17 22:27:17.000000000","message":"so each device gets 20 req in 0.2 seconds","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b3f544d9d9ce164cf33918f179799d96c902454f","unresolved":true,"context_lines":[{"line_number":130,"context_line":"            success_per_dev \u003d self._do_test_ratelimit(method, 1, 1)"},{"line_number":131,"context_line":"            self.assertEqual([2] * 3, list(success_per_dev.values()))"},{"line_number":132,"context_line":"            success_per_dev \u003d self._do_test_ratelimit(method, 10, 1)"},{"line_number":133,"context_line":"            self.assertEqual([12] * 3, list(success_per_dev.values()))"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        do_test_ratelimit(\u0027GET\u0027)"},{"line_number":136,"context_line":"        do_test_ratelimit(\u0027HEAD\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"b179470e_7effcccb","line":133,"updated":"2022-05-17 22:27:17.000000000","message":"10, 0 \u003d\u003e [2] (10 req/s ~ 2 req in 0.2s)\n10, 1 \u003d\u003e [12] (10 req/s rate buffer for 1s is 10 requests + baseline)","commit_id":"35829a4a2cfa849536ecc832efc663a28ce4e118"}]}