)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b624299aa8d34e6903297036ef1d1e9fd336fdf6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2b5c6719_b9cfd20c","updated":"2022-04-27 09:30:37.000000000","message":"I\u0027d suggest raising the existing MemcacheConnectionError for consistency, and there\u0027s at least one other error we should raise on (value too large -\u003e not stored in set)","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9a7231104b90ab2802e0ed63ba923a08b336543f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6890174c_d2d45325","updated":"2022-05-13 14:32:03.000000000","message":"LGTM.\n\nI agree with @Clay\u0027s comments that, given a fresh start, there\u0027s scope for a better interface, but I also agree that this is good forwards progress as it stands and is providing useful insight in production.","commit_id":"9bed525bfbe3ec05cb45c9835333fe45f8e04b40"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3f8d639b6ce15f4c9bf3f1680d2707c309548a59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2d75f005_ed28314f","updated":"2022-05-11 20:58:58.000000000","message":"Oh hey, I had some draft comments from like 2 weeks ago...","commit_id":"9bed525bfbe3ec05cb45c9835333fe45f8e04b40"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c16482b1319321c898426626638bc9c90bf77cab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3fba028a_73e7f49c","updated":"2022-05-02 22:52:14.000000000","message":"The best interface I can imagine would look like:\n\n    def safe_set():  # raw impl; always raises\n        pass\n\n    def unsafe_set():\n        try:\n            return safe_set()\n        except MemcacheError:\n            # trolololo!!\n            return\n\n    def set(raise_on_error\u003dFalse):\n        if raise_on_error:\n            return safe_set()\n        else:\n            warning(\"we\u0027re sorry this default sucks\")\n            return unsafe_set()\n\nI think the existing interface, while perhaps convienient, hides necessary complexity from callers.  \n\nHowever I don\u0027t think the value we can get out of this change is deminished in anyway waiting on agreement about an interface like that.  In fact Swift could theoretically get better on many deminsions befores we agreed on the best interface for our memcache client - especially what to do about memcache rings now that we should reocmmend operators deploy a finely tuned McRouter.","commit_id":"9bed525bfbe3ec05cb45c9835333fe45f8e04b40"}],"swift/common/memcached.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b624299aa8d34e6903297036ef1d1e9fd336fdf6","unresolved":true,"context_lines":[{"line_number":287,"context_line":"        self._client_cache[server].put((fp, sock))"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def set(self, key, value, serialize\u003dTrue, time\u003d0,"},{"line_number":290,"context_line":"            min_compress_len\u003d0, raise_on_error\u003dFalse):"},{"line_number":291,"context_line":"        \"\"\""},{"line_number":292,"context_line":"        Set a key/value pair in memcache"},{"line_number":293,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b9dd73d5_1e6d8756","line":290,"range":{"start_line":290,"start_character":32,"end_line":290,"end_character":52},"updated":"2022-04-27 09:30:37.000000000","message":"hmmm, or perhaps ignore_errors\u003dTrue ?\n\nI even wonder if I\u0027d prefer a new method let set_or_fail - with hindsight, the current set/get might be safe_set/safe_get","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b624299aa8d34e6903297036ef1d1e9fd336fdf6","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                    sock.sendall(set_msg(key, flags, timeout, value))"},{"line_number":325,"context_line":"                    # Wait for the set to complete"},{"line_number":326,"context_line":"                    msg \u003d fp.readline().strip()"},{"line_number":327,"context_line":"                    if msg !\u003d b\u0027STORED\u0027:"},{"line_number":328,"context_line":"                        if not six.PY2:"},{"line_number":329,"context_line":"                            msg \u003d msg.decode(\u0027ascii\u0027)"},{"line_number":330,"context_line":"                        self.logger.error("},{"line_number":331,"context_line":"                            \"Error setting value in memcached: \""},{"line_number":332,"context_line":"                            \"%(server)s: %(msg)s\","},{"line_number":333,"context_line":"                            {\u0027server\u0027: server, \u0027msg\u0027: msg})"},{"line_number":334,"context_line":"                    if 0 \u003c\u003d self.item_size_warning_threshold \u003c\u003d len(value):"},{"line_number":335,"context_line":"                        self.logger.warning("},{"line_number":336,"context_line":"                            \"Item size larger than warning threshold: \""}],"source_content_type":"text/x-python","patch_set":1,"id":"a3bf3693_dfd5b996","line":333,"range":{"start_line":327,"start_character":20,"end_line":333,"end_character":59},"updated":"2022-04-27 09:30:37.000000000","message":"this should also raise an exception","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9a7231104b90ab2802e0ed63ba923a08b336543f","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                    sock.sendall(set_msg(key, flags, timeout, value))"},{"line_number":325,"context_line":"                    # Wait for the set to complete"},{"line_number":326,"context_line":"                    msg \u003d fp.readline().strip()"},{"line_number":327,"context_line":"                    if msg !\u003d b\u0027STORED\u0027:"},{"line_number":328,"context_line":"                        if not six.PY2:"},{"line_number":329,"context_line":"                            msg \u003d msg.decode(\u0027ascii\u0027)"},{"line_number":330,"context_line":"                        self.logger.error("},{"line_number":331,"context_line":"                            \"Error setting value in memcached: \""},{"line_number":332,"context_line":"                            \"%(server)s: %(msg)s\","},{"line_number":333,"context_line":"                            {\u0027server\u0027: server, \u0027msg\u0027: msg})"},{"line_number":334,"context_line":"                    if 0 \u003c\u003d self.item_size_warning_threshold \u003c\u003d len(value):"},{"line_number":335,"context_line":"                        self.logger.warning("},{"line_number":336,"context_line":"                            \"Item size larger than warning threshold: \""}],"source_content_type":"text/x-python","patch_set":1,"id":"66e83565_b8844512","line":333,"range":{"start_line":327,"start_character":20,"end_line":333,"end_character":59},"in_reply_to":"01e29df9_c317f474","updated":"2022-05-13 14:32:03.000000000","message":"Ack","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"42670166aa6f84c36c1449d118dfe5cc499b4e62","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                    sock.sendall(set_msg(key, flags, timeout, value))"},{"line_number":325,"context_line":"                    # Wait for the set to complete"},{"line_number":326,"context_line":"                    msg \u003d fp.readline().strip()"},{"line_number":327,"context_line":"                    if msg !\u003d b\u0027STORED\u0027:"},{"line_number":328,"context_line":"                        if not six.PY2:"},{"line_number":329,"context_line":"                            msg \u003d msg.decode(\u0027ascii\u0027)"},{"line_number":330,"context_line":"                        self.logger.error("},{"line_number":331,"context_line":"                            \"Error setting value in memcached: \""},{"line_number":332,"context_line":"                            \"%(server)s: %(msg)s\","},{"line_number":333,"context_line":"                            {\u0027server\u0027: server, \u0027msg\u0027: msg})"},{"line_number":334,"context_line":"                    if 0 \u003c\u003d self.item_size_warning_threshold \u003c\u003d len(value):"},{"line_number":335,"context_line":"                        self.logger.warning("},{"line_number":336,"context_line":"                            \"Item size larger than warning threshold: \""}],"source_content_type":"text/x-python","patch_set":1,"id":"f392e8de_333f5fd8","line":333,"range":{"start_line":327,"start_character":20,"end_line":333,"end_character":59},"in_reply_to":"a3bf3693_dfd5b996","updated":"2022-04-27 13:25:10.000000000","message":"why do we not continue? is there an assumption that if the value fails with one server then it will fail with all? if the value is too large then that is reasonable,  but can there be other reasons to fail in this way?\n\nor is the lack of continue unintended?","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3f8d639b6ce15f4c9bf3f1680d2707c309548a59","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                    sock.sendall(set_msg(key, flags, timeout, value))"},{"line_number":325,"context_line":"                    # Wait for the set to complete"},{"line_number":326,"context_line":"                    msg \u003d fp.readline().strip()"},{"line_number":327,"context_line":"                    if msg !\u003d b\u0027STORED\u0027:"},{"line_number":328,"context_line":"                        if not six.PY2:"},{"line_number":329,"context_line":"                            msg \u003d msg.decode(\u0027ascii\u0027)"},{"line_number":330,"context_line":"                        self.logger.error("},{"line_number":331,"context_line":"                            \"Error setting value in memcached: \""},{"line_number":332,"context_line":"                            \"%(server)s: %(msg)s\","},{"line_number":333,"context_line":"                            {\u0027server\u0027: server, \u0027msg\u0027: msg})"},{"line_number":334,"context_line":"                    if 0 \u003c\u003d self.item_size_warning_threshold \u003c\u003d len(value):"},{"line_number":335,"context_line":"                        self.logger.warning("},{"line_number":336,"context_line":"                            \"Item size larger than warning threshold: \""}],"source_content_type":"text/x-python","patch_set":1,"id":"01e29df9_c317f474","line":333,"range":{"start_line":327,"start_character":20,"end_line":333,"end_character":59},"in_reply_to":"f392e8de_333f5fd8","updated":"2022-05-11 20:58:58.000000000","message":"It was definitely a limitation I was thinking about -- as an auth system, I kinda want things to work agains the primary location or not at all. If I try to store an auth token, fail to set the primary, but succeed at a secondary location, it\u0027s not clear we should tell clients about the token -- on subsequent use, we may be able to talk to the primary again, get a cache miss, and respond 401 despite having a \"fresh\" token.\n\nConversely, if I try to check a token, fail at the primary, then get a cache miss at the secondary, should we actually 401? Or should we 503 and hope the primary is around next time (or the load balancer gets the client to a proxy that *can* talk to the primary)? I suppose the 401\u0027s probably fairly justifiable in this case (it\u0027s not so different from memcache restarting on the primary or something).","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b624299aa8d34e6903297036ef1d1e9fd336fdf6","unresolved":true,"context_lines":[{"line_number":343,"context_line":"            except (Exception, Timeout) as e:"},{"line_number":344,"context_line":"                self._exception_occurred(server, e, sock\u003dsock, fp\u003dfp)"},{"line_number":345,"context_line":"                if raise_on_error:"},{"line_number":346,"context_line":"                    raise"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def get(self, key, raise_on_error\u003dFalse):"},{"line_number":349,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"0fb640de_e4698f4f","line":346,"updated":"2022-04-27 09:30:37.000000000","message":"How about raising a MemcacheConnectionError to be consistent with incr and decr and so that callers are spared the implementation details?","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3f8d639b6ce15f4c9bf3f1680d2707c309548a59","unresolved":true,"context_lines":[{"line_number":343,"context_line":"            except (Exception, Timeout) as e:"},{"line_number":344,"context_line":"                self._exception_occurred(server, e, sock\u003dsock, fp\u003dfp)"},{"line_number":345,"context_line":"                if raise_on_error:"},{"line_number":346,"context_line":"                    raise"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def get(self, key, raise_on_error\u003dFalse):"},{"line_number":349,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"fe5096eb_3a1c3060","line":346,"in_reply_to":"0fb640de_e4698f4f","updated":"2022-05-11 20:58:58.000000000","message":"The more I think about it, incr() makes me nervous... with overloaded servers, you could call incr() 10 times, but on get()s have some weird smear of sometimes getting 6, or 3, or 1...","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9a7231104b90ab2802e0ed63ba923a08b336543f","unresolved":false,"context_lines":[{"line_number":343,"context_line":"            except (Exception, Timeout) as e:"},{"line_number":344,"context_line":"                self._exception_occurred(server, e, sock\u003dsock, fp\u003dfp)"},{"line_number":345,"context_line":"                if raise_on_error:"},{"line_number":346,"context_line":"                    raise"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def get(self, key, raise_on_error\u003dFalse):"},{"line_number":349,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"ca6cb0d4_0401e96e","line":346,"in_reply_to":"fe5096eb_3a1c3060","updated":"2022-05-13 14:32:03.000000000","message":"Done","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"42670166aa6f84c36c1449d118dfe5cc499b4e62","unresolved":true,"context_lines":[{"line_number":358,"context_line":"        \"\"\""},{"line_number":359,"context_line":"        key \u003d md5hash(key)"},{"line_number":360,"context_line":"        value \u003d None"},{"line_number":361,"context_line":"        for (server, fp, sock) in self._get_conns(key):"},{"line_number":362,"context_line":"            try:"},{"line_number":363,"context_line":"                with Timeout(self._io_timeout):"},{"line_number":364,"context_line":"                    sock.sendall(b\u0027get \u0027 + key + b\u0027\\r\\n\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"21603d63_d058914e","line":361,"range":{"start_line":361,"start_character":8,"end_line":361,"end_character":55},"updated":"2022-04-27 13:25:10.000000000","message":"the errors we have seen in production are while connecting to servers in _get_conns(), so no servers are yielded to this loop and the except clause at line 385 is not reached","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9a7231104b90ab2802e0ed63ba923a08b336543f","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        \"\"\""},{"line_number":359,"context_line":"        key \u003d md5hash(key)"},{"line_number":360,"context_line":"        value \u003d None"},{"line_number":361,"context_line":"        for (server, fp, sock) in self._get_conns(key):"},{"line_number":362,"context_line":"            try:"},{"line_number":363,"context_line":"                with Timeout(self._io_timeout):"},{"line_number":364,"context_line":"                    sock.sendall(b\u0027get \u0027 + key + b\u0027\\r\\n\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b2d41f43_45cf1d5c","line":361,"range":{"start_line":361,"start_character":8,"end_line":361,"end_character":55},"in_reply_to":"21603d63_d058914e","updated":"2022-05-13 14:32:03.000000000","message":"Done","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b624299aa8d34e6903297036ef1d1e9fd336fdf6","unresolved":true,"context_lines":[{"line_number":385,"context_line":"            except (Exception, Timeout) as e:"},{"line_number":386,"context_line":"                self._exception_occurred(server, e, sock\u003dsock, fp\u003dfp)"},{"line_number":387,"context_line":"                if raise_on_error:"},{"line_number":388,"context_line":"                    raise"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    def incr(self, key, delta\u003d1, time\u003d0):"},{"line_number":391,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1b8e26f7_9769be4c","line":388,"updated":"2022-04-27 09:30:37.000000000","message":"we may also want to upgrade delete and the multi-* methods, but not necessarily in this patch","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"}],"swift/proxy/controllers/container.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b624299aa8d34e6903297036ef1d1e9fd336fdf6","unresolved":true,"context_lines":[{"line_number":156,"context_line":"                            cached_ranges \u003d memcache.get("},{"line_number":157,"context_line":"                                cache_key, raise_on_error\u003dTrue)"},{"line_number":158,"context_line":"                            cache_state \u003d \u0027hit\u0027 if cached_ranges else \u0027miss\u0027"},{"line_number":159,"context_line":"                        except (Exception, Timeout):"},{"line_number":160,"context_line":"                            cache_state \u003d \u0027error\u0027"},{"line_number":161,"context_line":"                        self.logger.increment(\u0027shard_listing.cache.%s\u0027 % cache_state)"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"be3389f2_208edae9","line":159,"range":{"start_line":159,"start_character":31,"end_line":159,"end_character":51},"updated":"2022-04-27 09:30:37.000000000","message":"perhaps unfortunate that callers need to know to catch both Exceptions and Timeouts - see comment in memcache about mapping to a single MemcacheException class","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3f8d639b6ce15f4c9bf3f1680d2707c309548a59","unresolved":true,"context_lines":[{"line_number":156,"context_line":"                            cached_ranges \u003d memcache.get("},{"line_number":157,"context_line":"                                cache_key, raise_on_error\u003dTrue)"},{"line_number":158,"context_line":"                            cache_state \u003d \u0027hit\u0027 if cached_ranges else \u0027miss\u0027"},{"line_number":159,"context_line":"                        except (Exception, Timeout):"},{"line_number":160,"context_line":"                            cache_state \u003d \u0027error\u0027"},{"line_number":161,"context_line":"                        self.logger.increment(\u0027shard_listing.cache.%s\u0027 % cache_state)"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b5544f68_7c56bffb","line":159,"range":{"start_line":159,"start_character":31,"end_line":159,"end_character":51},"in_reply_to":"be3389f2_208edae9","updated":"2022-05-11 20:58:58.000000000","message":"+1","commit_id":"46a49bf2e43cfca59184f6a10410205bc7b21e6d"}]}