)]}'
{"swift/common/memcached.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bac7954d1f4b4dd2f9c4e8a080284f5c6561abd4","unresolved":false,"context_lines":[{"line_number":218,"context_line":"            return"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        now \u003d time.time()"},{"line_number":221,"context_line":"        self._errors[server].append(time.time())"},{"line_number":222,"context_line":"        if len(self._errors[server]) \u003e\u003d ERROR_LIMIT_COUNT:"},{"line_number":223,"context_line":"            self._errors[server] \u003d [err for err in self._errors[server]"},{"line_number":224,"context_line":"                                    if err \u003e now - ERROR_LIMIT_TIME]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_f2898426","line":221,"updated":"2020-10-23 12:59:56.000000000","message":"nit: I guess now could be used in place of time.time() on this line","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a6167bee39a459a10ebb27a40d437bba3abea7e9","unresolved":false,"context_lines":[{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        now \u003d time.time()"},{"line_number":221,"context_line":"        self._errors[server].append(time.time())"},{"line_number":222,"context_line":"        if len(self._errors[server]) \u003e\u003d ERROR_LIMIT_COUNT:"},{"line_number":223,"context_line":"            self._errors[server] \u003d [err for err in self._errors[server]"},{"line_number":224,"context_line":"                                    if err \u003e now - ERROR_LIMIT_TIME]"},{"line_number":225,"context_line":"            remaining_servers \u003d sum("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_87039f42","line":222,"updated":"2020-10-26 06:41:55.000000000","message":"By adding the \u0027\u003d\u0027 aren\u0027t we changing the contract people might have come to expect? Ie. we used to error limit, according to this code one the 11th time in the last minute (because \u003e ERROR_LIMIT_COUNT) and now we\u0027ll error limit on the 10th (\u003e\u003d ERROR_LMIT_COUNT).\n\nI\u0027m not against the change, but it might mean it\u0027s something that should be in a change log or something as it\u0027s a change to a default. Though maybe I\u0027m being too picky :)\n\nAnd as this is hard-coded, yeah maybe we should look at making these configurable, but that\u0027s a discussion for the PTG :)","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bac7954d1f4b4dd2f9c4e8a080284f5c6561abd4","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                                    if err \u003e now - ERROR_LIMIT_TIME]"},{"line_number":225,"context_line":"            remaining_servers \u003d sum("},{"line_number":226,"context_line":"                1 for _server, value in self._error_limited.items()"},{"line_number":227,"context_line":"                if value \u003c\u003d now)"},{"line_number":228,"context_line":"            if len(self._errors[server]) \u003e\u003d ERROR_LIMIT_COUNT and \\"},{"line_number":229,"context_line":"                    remaining_servers \u003e 1:"},{"line_number":230,"context_line":"                self._error_limited[server] \u003d now + ERROR_LIMIT_DURATION"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_b29eec60","line":227,"updated":"2020-10-23 12:59:56.000000000","message":"ok. at line 246 server is skipped if error limit \u003e now, therefore server is available is error limit \u003c\u003d now.","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a6167bee39a459a10ebb27a40d437bba3abea7e9","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                                    if err \u003e now - ERROR_LIMIT_TIME]"},{"line_number":225,"context_line":"            remaining_servers \u003d sum("},{"line_number":226,"context_line":"                1 for _server, value in self._error_limited.items()"},{"line_number":227,"context_line":"                if value \u003c\u003d now)"},{"line_number":228,"context_line":"            if len(self._errors[server]) \u003e\u003d ERROR_LIMIT_COUNT and \\"},{"line_number":229,"context_line":"                    remaining_servers \u003e 1:"},{"line_number":230,"context_line":"                self._error_limited[server] \u003d now + ERROR_LIMIT_DURATION"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_27bd6b65","line":227,"in_reply_to":"3f65232a_b29eec60","updated":"2020-10-26 06:41:55.000000000","message":"Yeah, looks like all the filtering out of old stale times for self.errors happen in this function, but stale error_limitted isn\u0027t cleaned, just checked when calling.\n\nI guess it\u0027s only ever 1 value per server so it\u0027s ok. and only used in a few places like Al mentioned. But starting to feel like we could add some helper methods to hide the complexity if we ever need to start adding more of these. Like self.is_error_limited(server) and self.get_avaible_servers() or self.get_error_limited_servers().\n\nBut that\u0027s really just NIT and something to add if we start doing more of this.","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bac7954d1f4b4dd2f9c4e8a080284f5c6561abd4","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                if value \u003c\u003d now)"},{"line_number":228,"context_line":"            if len(self._errors[server]) \u003e\u003d ERROR_LIMIT_COUNT and \\"},{"line_number":229,"context_line":"                    remaining_servers \u003e 1:"},{"line_number":230,"context_line":"                self._error_limited[server] \u003d now + ERROR_LIMIT_DURATION"},{"line_number":231,"context_line":"                self.logger.error(\u0027Error limiting server %s\u0027, server)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _get_conns(self, key):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_18941e8b","line":230,"updated":"2020-10-23 12:59:56.000000000","message":"so as a consequence this server escapes being error-limited going forwards even once another server subsequently becomes available. How about an alternative strategy whereby this server is error-limited and the server closest to becoming un-error-limited is prematurely made available?","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5b07dcfc9e8f092bbfcf2491e74e87d8debf655f","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                if value \u003c\u003d now)"},{"line_number":228,"context_line":"            if len(self._errors[server]) \u003e\u003d ERROR_LIMIT_COUNT and \\"},{"line_number":229,"context_line":"                    remaining_servers \u003e 1:"},{"line_number":230,"context_line":"                self._error_limited[server] \u003d now + ERROR_LIMIT_DURATION"},{"line_number":231,"context_line":"                self.logger.error(\u0027Error limiting server %s\u0027, server)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _get_conns(self, key):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_0d0b3b8e","line":230,"in_reply_to":"3f65232a_18941e8b","updated":"2020-10-23 19:06:06.000000000","message":"Once another server becomes available, the next error we get while talking to the server that skipped error-limiting will kick it over to being error-limited; it\u0027s not like we cleared self._errors[server] or anything.","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"}],"test/unit/common/test_memcached.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bac7954d1f4b4dd2f9c4e8a080284f5c6561abd4","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class ExplodingMockMemcached(object):"},{"line_number":53,"context_line":"    should_explode \u003d True"},{"line_number":54,"context_line":"    exploded \u003d False"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def sendall(self, string):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_183d3ebb","line":53,"updated":"2020-10-23 12:59:56.000000000","message":"nit: should_explode as a parameter of an ExplodingMockMemcached - perhaps we could use a separate SafeMockMemcached class?","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bac7954d1f4b4dd2f9c4e8a080284f5c6561abd4","unresolved":false,"context_lines":[{"line_number":518,"context_line":""},{"line_number":519,"context_line":"    def test_error_limiting(self):"},{"line_number":520,"context_line":"        memcache_client \u003d memcached.MemcacheRing("},{"line_number":521,"context_line":"            [\u00271.2.3.4:11211\u0027, \u00271.2.3.5:11211\u0027], logger\u003dself.logger)"},{"line_number":522,"context_line":"        mock1 \u003d ExplodingMockMemcached()"},{"line_number":523,"context_line":"        mock2 \u003d ExplodingMockMemcached()"},{"line_number":524,"context_line":"        mock2.should_explode \u003d False"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_786392dd","line":521,"updated":"2020-10-23 12:59:56.000000000","message":"nit:  I wonder if a test with just a single server might be good for completeness","commit_id":"3fde7677f7a0e11b367adb19d91c737290e98852"}]}