)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"11405401d6c352aab6707bb46acdfb31203b5b0d","unresolved":true,"context_lines":[{"line_number":11,"context_line":"    enough errors to trigger error suppression."},{"line_number":12,"context_line":" 2. \u0027error_limiter.forced_limit\u0027, when one node runs into serious"},{"line_number":13,"context_line":"    error like Insufficient Storage."},{"line_number":14,"context_line":" 3. \u0027error_limiter.node_limited\u0027, when one node is error limited"},{"line_number":15,"context_line":"    during node selection process."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Id9bec9b46dad82163517fd71cfdda5b751464588"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"edc4a830_364bb7a8","line":14,"updated":"2022-11-08 15:46:35.000000000","message":"see comment in code: \u0027is-limited\u0027 might be clearer. Unfortunately the phrase \u0027is error limited\u0027 is ambiguous: it could mean \u0027the act of limiting\u0027 of \u0027the state of having been limited\u0027.","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"57c43b94f068fd56b948685d6937ccf1e5442fef","unresolved":false,"context_lines":[{"line_number":11,"context_line":"    enough errors to trigger error suppression."},{"line_number":12,"context_line":" 2. \u0027error_limiter.forced_limit\u0027, when one node runs into serious"},{"line_number":13,"context_line":"    error like Insufficient Storage."},{"line_number":14,"context_line":" 3. \u0027error_limiter.node_limited\u0027, when one node is error limited"},{"line_number":15,"context_line":"    during node selection process."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Id9bec9b46dad82163517fd71cfdda5b751464588"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"4197812f_4bcb1504","line":14,"in_reply_to":"edc4a830_364bb7a8","updated":"2022-11-09 04:40:10.000000000","message":"Done","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ce09f91a91bf522a0d399c1c09d48a7dbd57b6f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"17b710d6_c46d3e44","updated":"2022-11-08 06:18:31.000000000","message":"I modified all production code per today\u0027s discussion, but haven\u0027t added more tests yet. Just want to get feedback first, thanks.","commit_id":"cccb876053f9c8323bfb3fa70ed97ee929f53168"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f1c516c89d5b02cc36191aa6490376e9feacf01c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2520b385_d8dd1c62","updated":"2022-11-08 06:08:07.000000000","message":"thanks for the review.","commit_id":"cccb876053f9c8323bfb3fa70ed97ee929f53168"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"11405401d6c352aab6707bb46acdfb31203b5b0d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"60657750_e02d7706","updated":"2022-11-08 15:46:35.000000000","message":"+1 for the discrimination of different stats.\n\n-1: I think \u0027node_limited\u0027 could be confusing\n\n-1: I think it would be good to have some test coverage for the log messages. See https://review.opendev.org/c/openstack/swift/+/861653 (test_obj, test_container) for some examples of capturing the logs (in that case for proposed backend rate limiting but it will be a very similar test pattern in similar tests).","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"af272a4201cab5c28c7c0e494fa0cdfa6467a25f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"59b6ad71_9b378ec1","updated":"2022-11-09 19:03:02.000000000","message":"I need to look at the tests, just leaving a few (minor) comments - looking pretty good!","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f52915be97c9cbd0c46f7822f1bd652ad71db590","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ed04e82a_a5470d29","updated":"2022-11-10 05:01:02.000000000","message":"Thanks for the review!","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fb07b9b4622acd6e7248afc3055d3b66ec6d9c6c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6443f2cb_c8eb9282","updated":"2022-11-15 00:27:14.000000000","message":"New test from https://review.opendev.org/c/openstack/swift/+/861652/5/test/unit/proxy/test_server.py needs to be updated. I\u0027ll rebase and fix it.","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e6b331876817b2e7cf47b011dd782ebdbf16623","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"431a4a89_8d300c1d","updated":"2022-11-11 01:19:44.000000000","message":"thanks a lot for the review and follow-up!","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1d4d3d59aaf8fccf1b673a60c43c1d23fb93a61c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"5379d470_652c9663","updated":"2022-11-10 13:07:44.000000000","message":"thanks for making all the changes, I think this is good to go and my comments can be considered separately in the follow up https://review.opendev.org/c/openstack/swift/+/864199","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"8efe388e0421a6ab7a5f9514a1763d69583338c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"dcee6087_ff6796ee","updated":"2022-11-16 00:30:36.000000000","message":"Thanks a lot for the rebase, Tim!","commit_id":"38124221d7bbce8459fd544a15fb5e57dfc7213e"}],"swift/common/error_limiter.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"af272a4201cab5c28c7c0e494fa0cdfa6467a25f","unresolved":true,"context_lines":[{"line_number":84,"context_line":"        the given ``node``."},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        :param node: dictionary describing a node."},{"line_number":87,"context_line":"        :returns: True if suppression_limit is reached, False otherwise"},{"line_number":88,"context_line":"        \"\"\""},{"line_number":89,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":90,"context_line":"        error_stats \u003d self.stats[node_key]"}],"source_content_type":"text/x-python","patch_set":6,"id":"2f364259_df9edb78","line":87,"updated":"2022-11-09 19:03:02.000000000","message":"nit: True if suppression_limit is exceeded\n\nI don\u0027t like it being \u003e rather than \u003e\u003d suppression_limit, but that\u0027s how it is :)","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f52915be97c9cbd0c46f7822f1bd652ad71db590","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        the given ``node``."},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        :param node: dictionary describing a node."},{"line_number":87,"context_line":"        :returns: True if suppression_limit is reached, False otherwise"},{"line_number":88,"context_line":"        \"\"\""},{"line_number":89,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":90,"context_line":"        error_stats \u003d self.stats[node_key]"}],"source_content_type":"text/x-python","patch_set":6,"id":"f2e19013_65764773","line":87,"in_reply_to":"2f364259_df9edb78","updated":"2022-11-10 05:01:02.000000000","message":"Done","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"}],"swift/proxy/server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"821469d9ffea69126e013dcb12f66bdf5f4e27ed","unresolved":true,"context_lines":[{"line_number":660,"context_line":"            self.logger.debug("},{"line_number":661,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":662,"context_line":"            self.logger.increment(\u0027errlimited.%s\u0027 %"},{"line_number":663,"context_line":"                                  node[\u0027ip\u0027].replace(\".\", \"_\"))"},{"line_number":664,"context_line":"        return limited"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"    def error_limit(self, node, msg):"}],"source_content_type":"text/x-python","patch_set":1,"id":"b0930365_b764926b","line":663,"updated":"2022-11-03 23:03:57.000000000","message":"I\u0027m a little worried about the proliferation of metrics here... we could have some hundreds/thousands of nodes, yeah?","commit_id":"7abbfdea50aa29f70907afa90c54a46854ec3f82"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"77b5691deab3e940f903a270b0e185bea779748c","unresolved":false,"context_lines":[{"line_number":660,"context_line":"            self.logger.debug("},{"line_number":661,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":662,"context_line":"            self.logger.increment(\u0027errlimited.%s\u0027 %"},{"line_number":663,"context_line":"                                  node[\u0027ip\u0027].replace(\".\", \"_\"))"},{"line_number":664,"context_line":"        return limited"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"    def error_limit(self, node, msg):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7fc07acf_3e50ad7f","line":663,"in_reply_to":"b0930365_b764926b","updated":"2022-11-04 19:19:58.000000000","message":"changes done per discussion offline.","commit_id":"7abbfdea50aa29f70907afa90c54a46854ec3f82"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d9a75dc81e6060d73c206e62c5e5a7060ad907fc","unresolved":true,"context_lines":[{"line_number":658,"context_line":"        limited \u003d self.error_limiter.is_limited(node)"},{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.info("},{"line_number":661,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":662,"context_line":"            self.logger.increment(\u0027error_limited\u0027)"},{"line_number":663,"context_line":"        return limited"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"6f7d7fb5_79ab4029","line":661,"updated":"2022-11-07 11:13:49.000000000","message":"once a node becomes error limited it remains in that state for 60s, during which every request that tried to use the node is going to emit this log...which could be a lot of new log messages. I think we should consider if we want this at info.","commit_id":"cccb876053f9c8323bfb3fa70ed97ee929f53168"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f1c516c89d5b02cc36191aa6490376e9feacf01c","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        limited \u003d self.error_limiter.is_limited(node)"},{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.info("},{"line_number":661,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":662,"context_line":"            self.logger.increment(\u0027error_limited\u0027)"},{"line_number":663,"context_line":"        return limited"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"b0aa157f_76e4c405","line":661,"in_reply_to":"549b9617_820905fe","updated":"2022-11-08 06:08:07.000000000","message":"Done","commit_id":"cccb876053f9c8323bfb3fa70ed97ee929f53168"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0d5653890dd61d009de305b428776e9dceda7054","unresolved":true,"context_lines":[{"line_number":658,"context_line":"        limited \u003d self.error_limiter.is_limited(node)"},{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.info("},{"line_number":661,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":662,"context_line":"            self.logger.increment(\u0027error_limited\u0027)"},{"line_number":663,"context_line":"        return limited"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"549b9617_820905fe","line":661,"in_reply_to":"6f7d7fb5_79ab4029","updated":"2022-11-07 16:00:12.000000000","message":"Maybe we put the info-level logging (and stats? or maybe we emit two or three stats?) in error_limit() and error_occurred() so it\u0027s edge-triggered. I\u0027m torn about whether to keep or drop the debug-level logging here.","commit_id":"cccb876053f9c8323bfb3fa70ed97ee929f53168"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"11405401d6c352aab6707bb46acdfb31203b5b0d","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        \"\"\""},{"line_number":658,"context_line":"        limited \u003d self.error_limiter.is_limited(node)"},{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.increment(\u0027error_limiter.node_limited\u0027)"},{"line_number":661,"context_line":"            self.logger.debug("},{"line_number":662,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":663,"context_line":"        return limited"}],"source_content_type":"text/x-python","patch_set":4,"id":"02e5ff53_77ac028d","line":660,"updated":"2022-11-08 15:46:35.000000000","message":"\u0027node_limited\u0027 could easily be mistaken as meaning \u0027the node became limited\u0027. Perhaps use \u0027is_limited\u0027? (also consistent with the new ErrorLimiter interface)","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"57c43b94f068fd56b948685d6937ccf1e5442fef","unresolved":false,"context_lines":[{"line_number":657,"context_line":"        \"\"\""},{"line_number":658,"context_line":"        limited \u003d self.error_limiter.is_limited(node)"},{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.increment(\u0027error_limiter.node_limited\u0027)"},{"line_number":661,"context_line":"            self.logger.debug("},{"line_number":662,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":663,"context_line":"        return limited"}],"source_content_type":"text/x-python","patch_set":4,"id":"1217d5c5_996eefa7","line":660,"in_reply_to":"02e5ff53_77ac028d","updated":"2022-11-09 04:40:10.000000000","message":"Done","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"11405401d6c352aab6707bb46acdfb31203b5b0d","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        self.error_limiter.limit(node)"},{"line_number":676,"context_line":"        self.logger.increment(\u0027error_limiter.forced_limit\u0027)"},{"line_number":677,"context_line":"        self.logger.error(\u0027%(msg)s %(node)s\u0027,"},{"line_number":678,"context_line":"                          {\u0027msg\u0027: msg, \u0027node\u0027: node_to_string(node)})"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def error_occurred(self, node, msg):"},{"line_number":681,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"523a6ee1_1b58a5ce","line":678,"updated":"2022-11-08 15:46:35.000000000","message":"AFAICT every time this is called msg\u003d\u003d\u0027ERROR Insufficient Storage\u0027. I think it would be useful to make this log line have the same form as the new lines introduced below i.e.:\n\n\u0027Node will be error limited from now: %s, %s\u0027,\n                node_to_string(node),\n                msg)\n\nand perhaps make a note in the commit message that the log format changed (in case anyone is scraping and parsing the log messages)","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"57c43b94f068fd56b948685d6937ccf1e5442fef","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        self.error_limiter.limit(node)"},{"line_number":676,"context_line":"        self.logger.increment(\u0027error_limiter.forced_limit\u0027)"},{"line_number":677,"context_line":"        self.logger.error(\u0027%(msg)s %(node)s\u0027,"},{"line_number":678,"context_line":"                          {\u0027msg\u0027: msg, \u0027node\u0027: node_to_string(node)})"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def error_occurred(self, node, msg):"},{"line_number":681,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"779268fa_02013ecf","line":678,"in_reply_to":"523a6ee1_1b58a5ce","updated":"2022-11-09 04:40:10.000000000","message":"Done","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"11405401d6c352aab6707bb46acdfb31203b5b0d","unresolved":true,"context_lines":[{"line_number":688,"context_line":"            msg \u003d msg.decode(\u0027utf-8\u0027)"},{"line_number":689,"context_line":"        self.logger.error(\u0027%(msg)s %(node)s\u0027,"},{"line_number":690,"context_line":"                          {\u0027msg\u0027: msg, \u0027node\u0027: node_to_string(node)})"},{"line_number":691,"context_line":"        if self.error_limiter.increment(node):"},{"line_number":692,"context_line":"            self.logger.increment(\u0027error_limiter.incremented_limit\u0027)"},{"line_number":693,"context_line":"            self.logger.info("},{"line_number":694,"context_line":"                \u0027Node will be error limited from now: %s, error: %s\u0027,"},{"line_number":695,"context_line":"                node_to_string(node),"},{"line_number":696,"context_line":"                msg)"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"    def iter_nodes(self, ring, partition, logger, node_iter\u003dNone, policy\u003dNone):"},{"line_number":699,"context_line":"        return NodeIter(self, ring, partition, logger, node_iter\u003dnode_iter,"}],"source_content_type":"text/x-python","patch_set":4,"id":"8e3987e8_76a4ddba","line":696,"range":{"start_line":691,"start_character":8,"end_line":696,"end_character":20},"updated":"2022-11-08 15:46:35.000000000","message":"I would not be against adding an error_increment() method to avoid repeating this at line 724","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"57c43b94f068fd56b948685d6937ccf1e5442fef","unresolved":false,"context_lines":[{"line_number":688,"context_line":"            msg \u003d msg.decode(\u0027utf-8\u0027)"},{"line_number":689,"context_line":"        self.logger.error(\u0027%(msg)s %(node)s\u0027,"},{"line_number":690,"context_line":"                          {\u0027msg\u0027: msg, \u0027node\u0027: node_to_string(node)})"},{"line_number":691,"context_line":"        if self.error_limiter.increment(node):"},{"line_number":692,"context_line":"            self.logger.increment(\u0027error_limiter.incremented_limit\u0027)"},{"line_number":693,"context_line":"            self.logger.info("},{"line_number":694,"context_line":"                \u0027Node will be error limited from now: %s, error: %s\u0027,"},{"line_number":695,"context_line":"                node_to_string(node),"},{"line_number":696,"context_line":"                msg)"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"    def iter_nodes(self, ring, partition, logger, node_iter\u003dNone, policy\u003dNone):"},{"line_number":699,"context_line":"        return NodeIter(self, ring, partition, logger, node_iter\u003dnode_iter,"}],"source_content_type":"text/x-python","patch_set":4,"id":"da74234e_fcf878be","line":696,"range":{"start_line":691,"start_character":8,"end_line":696,"end_character":20},"in_reply_to":"8e3987e8_76a4ddba","updated":"2022-11-09 04:40:10.000000000","message":"Done","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"af272a4201cab5c28c7c0e494fa0cdfa6467a25f","unresolved":true,"context_lines":[{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.increment(\u0027error_limiter.is_limited\u0027)"},{"line_number":661,"context_line":"            self.logger.debug("},{"line_number":662,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":663,"context_line":"        return limited"},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"    def error_limit(self, node, msg):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad50c35_f644c499","line":662,"updated":"2022-11-09 19:03:02.000000000","message":"nit: if you need to revisit, perhaps this log could be clearer: \u0027Node is error limited\u0027","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f52915be97c9cbd0c46f7822f1bd652ad71db590","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        if limited:"},{"line_number":660,"context_line":"            self.logger.increment(\u0027error_limiter.is_limited\u0027)"},{"line_number":661,"context_line":"            self.logger.debug("},{"line_number":662,"context_line":"                \u0027Node error limited: %s\u0027, node_to_string(node))"},{"line_number":663,"context_line":"        return limited"},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"    def error_limit(self, node, msg):"}],"source_content_type":"text/x-python","patch_set":6,"id":"dc880dc6_8d041375","line":662,"in_reply_to":"9ad50c35_f644c499","updated":"2022-11-10 05:01:02.000000000","message":"Done","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"af272a4201cab5c28c7c0e494fa0cdfa6467a25f","unresolved":true,"context_lines":[{"line_number":679,"context_line":"            node_to_string(node),"},{"line_number":680,"context_line":"            msg)"},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"    def error_increment(self, node, msg):"},{"line_number":683,"context_line":"        \"\"\""},{"line_number":684,"context_line":"        Call increment() on error limiter once, emit metrics or log if error"},{"line_number":685,"context_line":"        suppression will be triggered."}],"source_content_type":"text/x-python","patch_set":6,"id":"29e06ed6_9755ee6f","line":682,"updated":"2022-11-09 19:03:02.000000000","message":"should perhaps be _error_increment to emphasize it\u0027s a helper and not intended as part of the interface - this method doesn\u0027t handle utf8 decoding from bytes","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f52915be97c9cbd0c46f7822f1bd652ad71db590","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            node_to_string(node),"},{"line_number":680,"context_line":"            msg)"},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"    def error_increment(self, node, msg):"},{"line_number":683,"context_line":"        \"\"\""},{"line_number":684,"context_line":"        Call increment() on error limiter once, emit metrics or log if error"},{"line_number":685,"context_line":"        suppression will be triggered."}],"source_content_type":"text/x-python","patch_set":6,"id":"2e539d08_d0c138e4","line":682,"in_reply_to":"29e06ed6_9755ee6f","updated":"2022-11-10 05:01:02.000000000","message":"Done","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"af272a4201cab5c28c7c0e494fa0cdfa6467a25f","unresolved":true,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"    def error_increment(self, node, msg):"},{"line_number":683,"context_line":"        \"\"\""},{"line_number":684,"context_line":"        Call increment() on error limiter once, emit metrics or log if error"},{"line_number":685,"context_line":"        suppression will be triggered."},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"        :param node: dictionary of node to handle errors for"}],"source_content_type":"text/x-python","patch_set":6,"id":"dcd765a6_68eafa9e","line":684,"range":{"start_line":684,"start_character":61,"end_line":684,"end_character":63},"updated":"2022-11-09 19:03:02.000000000","message":"nit: s/or/and/","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f52915be97c9cbd0c46f7822f1bd652ad71db590","unresolved":false,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"    def error_increment(self, node, msg):"},{"line_number":683,"context_line":"        \"\"\""},{"line_number":684,"context_line":"        Call increment() on error limiter once, emit metrics or log if error"},{"line_number":685,"context_line":"        suppression will be triggered."},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"        :param node: dictionary of node to handle errors for"}],"source_content_type":"text/x-python","patch_set":6,"id":"63d3aa13_ecaba5f3","line":684,"range":{"start_line":684,"start_character":61,"end_line":684,"end_character":63},"in_reply_to":"dcd765a6_68eafa9e","updated":"2022-11-10 05:01:02.000000000","message":"Done","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"af272a4201cab5c28c7c0e494fa0cdfa6467a25f","unresolved":true,"context_lines":[{"line_number":692,"context_line":"            self.logger.error("},{"line_number":693,"context_line":"                \u0027Node will be error limited from now: %s, error: %s\u0027,"},{"line_number":694,"context_line":"                node_to_string(node),"},{"line_number":695,"context_line":"                msg)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def error_occurred(self, node, msg):"},{"line_number":698,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"c36c3903_38194f8f","line":695,"range":{"start_line":695,"start_character":16,"end_line":695,"end_character":19},"updated":"2022-11-09 19:03:02.000000000","message":"hmmm, on the one hand I like that the cause of the node becoming limited is included in the log message, on the other hand I\u0027m concerned that anyone parsing logs for specific error msg strings will now get 2 hits for each error.\n\nmaybe we can get other opinions?","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f52915be97c9cbd0c46f7822f1bd652ad71db590","unresolved":false,"context_lines":[{"line_number":692,"context_line":"            self.logger.error("},{"line_number":693,"context_line":"                \u0027Node will be error limited from now: %s, error: %s\u0027,"},{"line_number":694,"context_line":"                node_to_string(node),"},{"line_number":695,"context_line":"                msg)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def error_occurred(self, node, msg):"},{"line_number":698,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"ccb0cc81_7c143c76","line":695,"range":{"start_line":695,"start_character":16,"end_line":695,"end_character":19},"in_reply_to":"c36c3903_38194f8f","updated":"2022-11-10 05:01:02.000000000","message":"yeah, the same error will show up twice if someone is parsing the logs. Let\u0027s remove the log msg here.","commit_id":"32bcbfdc186da02b165b0dc8ac38ab8e9c6e0782"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1d4d3d59aaf8fccf1b673a60c43c1d23fb93a61c","unresolved":true,"context_lines":[{"line_number":690,"context_line":"            self.logger.increment(\u0027error_limiter.incremented_limit\u0027)"},{"line_number":691,"context_line":"            self.logger.error("},{"line_number":692,"context_line":"                \u0027Node will be error limited from now: %s\u0027,"},{"line_number":693,"context_line":"                node_to_string(node))"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"    def error_occurred(self, node, msg):"},{"line_number":696,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"0ae26340_0a5d26c8","line":693,"updated":"2022-11-10 13:07:44.000000000","message":"ok: I think it is possible for _error_increment to be called for a node that has already exceeded suppression_limit, when the same node has been chosen for different requests being handled in different concurrent green threads. In that case \u0027Node will be error limited from now\u0027 will be repeated, which I think is appropriate because the error limit time is reset on each subsequent error i.e. the node IS error limited from NOW each time this log appears.\n\nI think it could be worth including the suppression_interval in the log message.","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e6b331876817b2e7cf47b011dd782ebdbf16623","unresolved":false,"context_lines":[{"line_number":690,"context_line":"            self.logger.increment(\u0027error_limiter.incremented_limit\u0027)"},{"line_number":691,"context_line":"            self.logger.error("},{"line_number":692,"context_line":"                \u0027Node will be error limited from now: %s\u0027,"},{"line_number":693,"context_line":"                node_to_string(node))"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"    def error_occurred(self, node, msg):"},{"line_number":696,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"71278a46_e7000a11","line":693,"in_reply_to":"0ae26340_0a5d26c8","updated":"2022-11-11 01:19:44.000000000","message":"Ack","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"}],"test/unit/common/test_error_limiter.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1d4d3d59aaf8fccf1b673a60c43c1d23fb93a61c","unresolved":true,"context_lines":[{"line_number":76,"context_line":"        for _ in range(limiter.suppression_limit):"},{"line_number":77,"context_line":"            self.assertFalse(limiter.increment(node))"},{"line_number":78,"context_line":"            node_errors \u003d limiter.stats.get(node_key)"},{"line_number":79,"context_line":"            self.assertGreater(node_errors[\u0027errors\u0027], last_errors)"},{"line_number":80,"context_line":"            self.assertFalse(limiter.is_limited(node))"},{"line_number":81,"context_line":"            last_errors \u003d node_errors[\u0027errors\u0027]"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"0d570514_30d0cb58","line":79,"updated":"2022-11-10 13:07:44.000000000","message":"off-topic: IDK why this is assertGreater rather than assertEqual the (loop index + 1)","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e6b331876817b2e7cf47b011dd782ebdbf16623","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        for _ in range(limiter.suppression_limit):"},{"line_number":77,"context_line":"            self.assertFalse(limiter.increment(node))"},{"line_number":78,"context_line":"            node_errors \u003d limiter.stats.get(node_key)"},{"line_number":79,"context_line":"            self.assertGreater(node_errors[\u0027errors\u0027], last_errors)"},{"line_number":80,"context_line":"            self.assertFalse(limiter.is_limited(node))"},{"line_number":81,"context_line":"            last_errors \u003d node_errors[\u0027errors\u0027]"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"4adcde03_f6bd66dd","line":79,"in_reply_to":"0d570514_30d0cb58","updated":"2022-11-11 01:19:44.000000000","message":"Ack","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1d4d3d59aaf8fccf1b673a60c43c1d23fb93a61c","unresolved":true,"context_lines":[{"line_number":86,"context_line":"        self.assertEqual(limiter.suppression_limit + 1,"},{"line_number":87,"context_line":"                         node_errors[\u0027errors\u0027])"},{"line_number":88,"context_line":"        self.assertTrue(limiter.is_limited(node))"},{"line_number":89,"context_line":"        last_time \u003d node_errors[\u0027last_error\u0027]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Simulate time with no errors have gone by."},{"line_number":92,"context_line":"        now \u003d last_time + limiter.suppression_interval + 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"3392db32_cde60083","line":89,"updated":"2022-11-10 13:07:44.000000000","message":"and maybe one more to assert what the return value is once the limiter has been suppressed","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e6b331876817b2e7cf47b011dd782ebdbf16623","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        self.assertEqual(limiter.suppression_limit + 1,"},{"line_number":87,"context_line":"                         node_errors[\u0027errors\u0027])"},{"line_number":88,"context_line":"        self.assertTrue(limiter.is_limited(node))"},{"line_number":89,"context_line":"        last_time \u003d node_errors[\u0027last_error\u0027]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Simulate time with no errors have gone by."},{"line_number":92,"context_line":"        now \u003d last_time + limiter.suppression_interval + 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"154d1d21_f41cf856","line":89,"in_reply_to":"3392db32_cde60083","updated":"2022-11-11 01:19:44.000000000","message":"Ack","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"11405401d6c352aab6707bb46acdfb31203b5b0d","unresolved":true,"context_lines":[{"line_number":71,"context_line":"from swift.common.utils import hash_path, storage_directory, \\"},{"line_number":72,"context_line":"    parse_content_type, parse_mime_headers, StatsdClient, \\"},{"line_number":73,"context_line":"    iter_multipart_mime_documents, public, mkdirs, NullLogger, md5, \\"},{"line_number":74,"context_line":"    node_to_string"},{"line_number":75,"context_line":"from swift.common.wsgi import loadapp, ConfigString, SwiftHttpProtocol"},{"line_number":76,"context_line":"from swift.proxy.controllers import base as proxy_base"},{"line_number":77,"context_line":"from swift.proxy.controllers.base import get_cache_key, cors_validation, \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"1acbc626_f0d678cc","line":74,"updated":"2022-11-08 15:46:35.000000000","message":"I usually run \u0027flake8 swift test\u0027 before pushing to gerrit - or to save time \u0027flake8 \u003cfiles I modified\u003e\u0027 😊","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"57c43b94f068fd56b948685d6937ccf1e5442fef","unresolved":false,"context_lines":[{"line_number":71,"context_line":"from swift.common.utils import hash_path, storage_directory, \\"},{"line_number":72,"context_line":"    parse_content_type, parse_mime_headers, StatsdClient, \\"},{"line_number":73,"context_line":"    iter_multipart_mime_documents, public, mkdirs, NullLogger, md5, \\"},{"line_number":74,"context_line":"    node_to_string"},{"line_number":75,"context_line":"from swift.common.wsgi import loadapp, ConfigString, SwiftHttpProtocol"},{"line_number":76,"context_line":"from swift.proxy.controllers import base as proxy_base"},{"line_number":77,"context_line":"from swift.proxy.controllers.base import get_cache_key, cors_validation, \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"7e77a828_afa7e57c","line":74,"in_reply_to":"1acbc626_f0d678cc","updated":"2022-11-09 04:40:10.000000000","message":"Done","commit_id":"fbd12b94617affbffbfe997d9ad85cba2b7dcfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1d4d3d59aaf8fccf1b673a60c43c1d23fb93a61c","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"                line \u003d logger.get_lines_for_level(\u0027error\u0027)[-1]"},{"line_number":1194,"context_line":"                self.assertIn(\u0027server-type server\u0027, line)"},{"line_number":1195,"context_line":"                self.assertIn(expected_info, line)"},{"line_number":1196,"context_line":"                self.assertIn(node[\u0027ip\u0027], line)"},{"line_number":1197,"context_line":"                self.assertIn(str(node[\u0027port\u0027]), line)"},{"line_number":1198,"context_line":"                self.assertIn(node[\u0027device\u0027], line)"},{"line_number":1199,"context_line":"                log_args, log_kwargs \u003d logger.log_dict[\u0027error\u0027][-1]"},{"line_number":1200,"context_line":"                self.assertTrue(log_kwargs[\u0027exc_info\u0027])"},{"line_number":1201,"context_line":"                self.assertIs(caught_exc, log_kwargs[\u0027exc_info\u0027][1])"}],"source_content_type":"text/x-python","patch_set":7,"id":"e5bcc810_617c4108","line":1198,"range":{"start_line":1196,"start_character":16,"end_line":1198,"end_character":51},"updated":"2022-11-10 13:07:44.000000000","message":"Now we have imposed uniform node_to_string representation we can make these assertions more compact","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e6b331876817b2e7cf47b011dd782ebdbf16623","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"                line \u003d logger.get_lines_for_level(\u0027error\u0027)[-1]"},{"line_number":1194,"context_line":"                self.assertIn(\u0027server-type server\u0027, line)"},{"line_number":1195,"context_line":"                self.assertIn(expected_info, line)"},{"line_number":1196,"context_line":"                self.assertIn(node[\u0027ip\u0027], line)"},{"line_number":1197,"context_line":"                self.assertIn(str(node[\u0027port\u0027]), line)"},{"line_number":1198,"context_line":"                self.assertIn(node[\u0027device\u0027], line)"},{"line_number":1199,"context_line":"                log_args, log_kwargs \u003d logger.log_dict[\u0027error\u0027][-1]"},{"line_number":1200,"context_line":"                self.assertTrue(log_kwargs[\u0027exc_info\u0027])"},{"line_number":1201,"context_line":"                self.assertIs(caught_exc, log_kwargs[\u0027exc_info\u0027][1])"}],"source_content_type":"text/x-python","patch_set":7,"id":"50429ea4_f0e34795","line":1198,"range":{"start_line":1196,"start_character":16,"end_line":1198,"end_character":51},"in_reply_to":"e5bcc810_617c4108","updated":"2022-11-11 01:19:44.000000000","message":"Ack","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1d4d3d59aaf8fccf1b673a60c43c1d23fb93a61c","unresolved":true,"context_lines":[{"line_number":1225,"context_line":"                 node_to_string(node)),"},{"line_number":1226,"context_line":"                lines[-1])"},{"line_number":1227,"context_line":"            self.assertEqual(1, logger.get_increment_counts().get("},{"line_number":1228,"context_line":"                \u0027error_limiter.incremented_limit\u0027, 0))"},{"line_number":1229,"context_line":""},{"line_number":1230,"context_line":"        do_test(\u0027success\u0027)"},{"line_number":1231,"context_line":"        do_test(\u0027succès\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"f431aad7_c6f989ac","line":1228,"updated":"2022-11-10 13:07:44.000000000","message":"nit: we can probably avoid the duplicated lines here - see my follow up patch","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"0e6b331876817b2e7cf47b011dd782ebdbf16623","unresolved":false,"context_lines":[{"line_number":1225,"context_line":"                 node_to_string(node)),"},{"line_number":1226,"context_line":"                lines[-1])"},{"line_number":1227,"context_line":"            self.assertEqual(1, logger.get_increment_counts().get("},{"line_number":1228,"context_line":"                \u0027error_limiter.incremented_limit\u0027, 0))"},{"line_number":1229,"context_line":""},{"line_number":1230,"context_line":"        do_test(\u0027success\u0027)"},{"line_number":1231,"context_line":"        do_test(\u0027succès\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"822f9b42_a1bcd708","line":1228,"in_reply_to":"f431aad7_c6f989ac","updated":"2022-11-11 01:19:44.000000000","message":"Ack","commit_id":"704c0254f594f42a474268b7a9b22efb2d43ad23"}]}