)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6dbb49a2159f7e69f5961ab5b7e974d6abb63af9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"902dd2d8_7da65c7b","updated":"2022-01-25 20:55:28.000000000","message":"Definitely a step in the right direction! I don\u0027t think we\u0027re quite to parity with what we did in https://review.opendev.org/c/openstack/swift/+/822104 though.","commit_id":"da38fd3ccf94f44c5c270dacc81de15e26323aff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa08a50252d2412cb752cad95f6b08aef4f34aa3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6815137e_9501c0a3","updated":"2022-01-26 16:26:04.000000000","message":"@Clay thanks for the extra test!","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"95452641_60a6f7c0","updated":"2022-01-26 15:33:31.000000000","message":"idk, i guess more stats are good - this seems fine\n\nhttps://review.opendev.org/c/openstack/swift/+/826489","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1862c4761930f6f854e13d56c1b5dcde24db2b7f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"04b93e06_0140fc54","updated":"2022-01-26 13:34:44.000000000","message":"recheck","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef257e1f2d9ec66218ae2db46d84e17c48829f4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3c22eb01_2d3309d4","updated":"2022-01-26 20:21:34.000000000","message":"recheck","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"41f1f9d55249bf508fdd5cb819418f351f0606fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e1ba279a_7cc0caf7","updated":"2022-01-28 19:01:27.000000000","message":"I\u0027m on board.","commit_id":"114440487889ebcc58c010a4986c406adcb18f0c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6357865c5db87f9e0922b1e77cf23aa62ff2c0c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"2522596b_7ec17f85","updated":"2022-02-06 22:38:43.000000000","message":"recheck","commit_id":"114440487889ebcc58c010a4986c406adcb18f0c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"131c58374e188d729544f4c1e71ba68d1e6a46f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"de6c5574_d8d26039","updated":"2022-02-04 06:57:23.000000000","message":"recheck","commit_id":"114440487889ebcc58c010a4986c406adcb18f0c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"87f642da89b0cb4c57e57ea8f301f0faf551143c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c7d0a861_f1ccb951","in_reply_to":"2522596b_7ec17f85","updated":"2022-02-06 22:39:32.000000000","message":"opss, must\u0027ve been seeing a cached version :P","commit_id":"114440487889ebcc58c010a4986c406adcb18f0c"}],"swift/proxy/controllers/container.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6dbb49a2159f7e69f5961ab5b7e974d6abb63af9","unresolved":true,"context_lines":[{"line_number":294,"context_line":"            # cache deleted shard ranges."},{"line_number":295,"context_line":"            resp \u003d self._GET_using_cache(req)"},{"line_number":296,"context_line":"        else:"},{"line_number":297,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":300,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"56a15129_25c9bd69","line":297,"updated":"2022-01-25 20:55:28.000000000","message":"So if listing-shard-range caching is disabled (because recheck_listing_shard_ranges \u003d\u003d 0), we don\u0027t get any stats?","commit_id":"da38fd3ccf94f44c5c270dacc81de15e26323aff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"870e60f4cf9cd2b736bc03b3b0756439de83baf3","unresolved":true,"context_lines":[{"line_number":294,"context_line":"            # cache deleted shard ranges."},{"line_number":295,"context_line":"            resp \u003d self._GET_using_cache(req)"},{"line_number":296,"context_line":"        else:"},{"line_number":297,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":300,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5dafb420_3cefe5e3","line":297,"in_reply_to":"56a15129_25c9bd69","updated":"2022-01-26 11:52:44.000000000","message":"I avoided the _GETorHEAD_from_backend path because we don\u0027t know if the container is sharded or not, so it wouldn\u0027t be possible to handle the failed requests, but I\u0027ve looked at it again and at the cost of an additional memcache get we can inspect info[\u0027sharding_state\u0027] the same as the _GET_using_cache path","commit_id":"da38fd3ccf94f44c5c270dacc81de15e26323aff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":true,"context_lines":[{"line_number":294,"context_line":"            # cache deleted shard ranges."},{"line_number":295,"context_line":"            resp \u003d self._GET_using_cache(req)"},{"line_number":296,"context_line":"        else:"},{"line_number":297,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":300,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fbaa7169_a3962d1a","line":297,"in_reply_to":"5dafb420_3cefe5e3","updated":"2022-01-26 15:33:31.000000000","message":"... but only if the container info IS cached - it seems in general an error from an unknown state wouldn\u0027t be enough to know how to classify the failure","commit_id":"da38fd3ccf94f44c5c270dacc81de15e26323aff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa08a50252d2412cb752cad95f6b08aef4f34aa3","unresolved":true,"context_lines":[{"line_number":294,"context_line":"            # cache deleted shard ranges."},{"line_number":295,"context_line":"            resp \u003d self._GET_using_cache(req)"},{"line_number":296,"context_line":"        else:"},{"line_number":297,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":300,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"59758e24_daa1c842","line":297,"in_reply_to":"fbaa7169_a3962d1a","updated":"2022-01-26 16:26:04.000000000","message":"yes, which is what I have tried to flag up in the comment at line 310","commit_id":"da38fd3ccf94f44c5c270dacc81de15e26323aff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        else:"},{"line_number":139,"context_line":"            info \u003d _get_info_from_caches(self.app, req.environ,"},{"line_number":140,"context_line":"                                         self.account_name,"},{"line_number":141,"context_line":"                                         self.container_name)"},{"line_number":142,"context_line":"        if (info and is_success(info[\u0027status\u0027]) and"},{"line_number":143,"context_line":"                info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027):"},{"line_number":144,"context_line":"            # container is sharded so we may have the shard ranges cached"}],"source_content_type":"text/x-python","patch_set":2,"id":"ed75d71f_6b08093d","side":"PARENT","line":141,"updated":"2022-01-26 15:33:31.000000000","message":"oh, so it\u0027s not an EXTRA get_info call - you just lifted it up one level","commit_id":"c57f5849818fdc705e76cde625c33262aacccd0e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa08a50252d2412cb752cad95f6b08aef4f34aa3","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        else:"},{"line_number":139,"context_line":"            info \u003d _get_info_from_caches(self.app, req.environ,"},{"line_number":140,"context_line":"                                         self.account_name,"},{"line_number":141,"context_line":"                                         self.container_name)"},{"line_number":142,"context_line":"        if (info and is_success(info[\u0027status\u0027]) and"},{"line_number":143,"context_line":"                info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027):"},{"line_number":144,"context_line":"            # container is sharded so we may have the shard ranges cached"}],"source_content_type":"text/x-python","patch_set":2,"id":"d6b73b83_e436101d","side":"PARENT","line":141,"in_reply_to":"ed75d71f_6b08093d","updated":"2022-01-26 16:26:04.000000000","message":"it\u0027s extra w.r.t. the _GETorHEAD_from_backend path","commit_id":"c57f5849818fdc705e76cde625c33262aacccd0e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":true,"context_lines":[{"line_number":297,"context_line":"                      and resp_record_type \u003d\u003d \u0027shard\u0027)"},{"line_number":298,"context_line":"                     or (info"},{"line_number":299,"context_line":"                         and is_success(info[\u0027status\u0027])"},{"line_number":300,"context_line":"                         and info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027))):"},{"line_number":301,"context_line":"            # We either got shard ranges from backend, or we expected to get"},{"line_number":302,"context_line":"            # them but the request failed. In some corner cases the metric"},{"line_number":303,"context_line":"            # reporting is imperfect:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5134d1ef_f74fbbc4","line":300,"updated":"2022-01-26 15:33:31.000000000","message":"this boolean logic is pretty nasty; i wonder how much I could pull out with any failing tests...\n\nI was confused about is_success(resp) and is_success(info), but it\u0027s basically \"we can only log error status IF the cached result told us we\u0027re querying a root\"","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef257e1f2d9ec66218ae2db46d84e17c48829f4d","unresolved":true,"context_lines":[{"line_number":297,"context_line":"                      and resp_record_type \u003d\u003d \u0027shard\u0027)"},{"line_number":298,"context_line":"                     or (info"},{"line_number":299,"context_line":"                         and is_success(info[\u0027status\u0027])"},{"line_number":300,"context_line":"                         and info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027))):"},{"line_number":301,"context_line":"            # We either got shard ranges from backend, or we expected to get"},{"line_number":302,"context_line":"            # them but the request failed. In some corner cases the metric"},{"line_number":303,"context_line":"            # reporting is imperfect:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3e6207a9_f967ddd6","line":300,"in_reply_to":"5134d1ef_f74fbbc4","updated":"2022-01-26 20:21:34.000000000","message":"Agreed that it\u0027s a little hairy, but I think it\u0027s about as clear as we could hope for. About the only changes I could think of would be\n\n- dropping the \"is_success(resp.status_int)\", since \"resp_record_type \u003d\u003d \u0027shard\u0027\" should only ever be true of successful responses;\n- moving the \"and not cached_results\" into the \"resp_record_type \u003d\u003d \u0027shard\u0027\" clause, since we\u0027ll never serve the failure from cache; and\n- ahead of the if, defining something like\n\n expected_sharded_container \u003d (info and is_success(info[\u0027status\u0027]) and\n                               info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027)\n\nMight make it a little more readable?\n\n if may_be_shard_listing and (expected_sharded_container or (\n         resp_record_type \u003d\u003d \u0027shard\u0027 and not cached_results)):","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            #   couldn\u0027t be parsed from the request body: we still increment"},{"line_number":312,"context_line":"            #   shard_listing.backend.200"},{"line_number":313,"context_line":"            self.logger.increment("},{"line_number":314,"context_line":"                \u0027shard_listing.backend.%s\u0027 % resp.status_int)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"},{"line_number":317,"context_line":"               resp_record_type.lower() \u003d\u003d \u0027shard\u0027)):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8752e302_63f05fec","line":314,"updated":"2022-01-26 15:33:31.000000000","message":"most of the time when we\u0027re tracking by status code we go ahead and collect *timing* - since that includes count\n\nDo we not care about timing?  Is it harder to capture?  Are we worried about the overhead of the extra metrics?","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa08a50252d2412cb752cad95f6b08aef4f34aa3","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            #   couldn\u0027t be parsed from the request body: we still increment"},{"line_number":312,"context_line":"            #   shard_listing.backend.200"},{"line_number":313,"context_line":"            self.logger.increment("},{"line_number":314,"context_line":"                \u0027shard_listing.backend.%s\u0027 % resp.status_int)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"},{"line_number":317,"context_line":"               resp_record_type.lower() \u003d\u003d \u0027shard\u0027)):"}],"source_content_type":"text/x-python","patch_set":2,"id":"e03a46b8_3a4c9c8c","line":314,"in_reply_to":"8752e302_63f05fec","updated":"2022-01-26 16:26:04.000000000","message":"In this patch I wanted to mirror what we already have for shard_updating","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef257e1f2d9ec66218ae2db46d84e17c48829f4d","unresolved":true,"context_lines":[{"line_number":135,"context_line":"            self.logger.debug("},{"line_number":136,"context_line":"                \u0027Skipping shard cache lookup (x-newest) for %s\u0027, req.path_qs)"},{"line_number":137,"context_line":"        if (info and is_success(info[\u0027status\u0027]) and"},{"line_number":138,"context_line":"                info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027 and not get_newest):"},{"line_number":139,"context_line":"            # container is sharded so we may have the shard ranges cached"},{"line_number":140,"context_line":"            headers \u003d headers_from_container_info(info)"},{"line_number":141,"context_line":"            if headers:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e02c41ca_4eb6c73b","line":138,"updated":"2022-01-26 20:21:34.000000000","message":"Alternatively, we could do an elif -- w/e","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"feab5857ea76e2cba75da369a0ad3727a4e2e645","unresolved":true,"context_lines":[{"line_number":135,"context_line":"            self.logger.debug("},{"line_number":136,"context_line":"                \u0027Skipping shard cache lookup (x-newest) for %s\u0027, req.path_qs)"},{"line_number":137,"context_line":"        if (info and is_success(info[\u0027status\u0027]) and"},{"line_number":138,"context_line":"                info.get(\u0027sharding_state\u0027) \u003d\u003d \u0027sharded\u0027 and not get_newest):"},{"line_number":139,"context_line":"            # container is sharded so we may have the shard ranges cached"},{"line_number":140,"context_line":"            headers \u003d headers_from_container_info(info)"},{"line_number":141,"context_line":"            if headers:"}],"source_content_type":"text/x-python","patch_set":3,"id":"6d6ccea2_0decc2d4","line":138,"in_reply_to":"e02c41ca_4eb6c73b","updated":"2022-01-27 14:13:46.000000000","message":"yeah! I was considering removing the debug log but didn\u0027t get round to it so elif works :)","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef257e1f2d9ec66218ae2db46d84e17c48829f4d","unresolved":true,"context_lines":[{"line_number":255,"context_line":"        # x-backend-record-type may be sent via internal client e.g. from"},{"line_number":256,"context_line":"        # the sharder or in probe tests"},{"line_number":257,"context_line":"        record_type \u003d req.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027).lower()"},{"line_number":258,"context_line":"        if not record_type:"},{"line_number":259,"context_line":"            record_type \u003d \u0027auto\u0027"},{"line_number":260,"context_line":"            req.headers[\u0027X-Backend-Record-Type\u0027] \u003d \u0027auto\u0027"},{"line_number":261,"context_line":"            params[\u0027states\u0027] \u003d \u0027listing\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"8077f2e1_9ee93859","line":258,"updated":"2022-01-26 20:21:34.000000000","message":"Off-topic: I wonder if this should have a\n\n and req.method \u003d\u003d \u0027GET\u0027","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"feab5857ea76e2cba75da369a0ad3727a4e2e645","unresolved":true,"context_lines":[{"line_number":255,"context_line":"        # x-backend-record-type may be sent via internal client e.g. from"},{"line_number":256,"context_line":"        # the sharder or in probe tests"},{"line_number":257,"context_line":"        record_type \u003d req.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027).lower()"},{"line_number":258,"context_line":"        if not record_type:"},{"line_number":259,"context_line":"            record_type \u003d \u0027auto\u0027"},{"line_number":260,"context_line":"            req.headers[\u0027X-Backend-Record-Type\u0027] \u003d \u0027auto\u0027"},{"line_number":261,"context_line":"            params[\u0027states\u0027] \u003d \u0027listing\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f7550b5_cd55bc42","line":258,"in_reply_to":"8077f2e1_9ee93859","updated":"2022-01-27 14:13:46.000000000","message":"same could also be true for the params[\u0027format\u0027] \u003d \u0027json\u0027","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef257e1f2d9ec66218ae2db46d84e17c48829f4d","unresolved":true,"context_lines":[{"line_number":265,"context_line":"        if (req.method \u003d\u003d \u0027GET\u0027"},{"line_number":266,"context_line":"                and get_param(req, \u0027states\u0027) \u003d\u003d \u0027listing\u0027"},{"line_number":267,"context_line":"                and record_type !\u003d \u0027object\u0027):"},{"line_number":268,"context_line":"            maybe_shard_listing \u003d True"},{"line_number":269,"context_line":"            info \u003d _get_info_from_caches(self.app, req.environ,"},{"line_number":270,"context_line":"                                         self.account_name,"},{"line_number":271,"context_line":"                                         self.container_name)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1c02dd9f_ef4bb2fe","line":268,"updated":"2022-01-26 20:21:34.000000000","message":"Right; so it\u0027s \"maybe\" because we\u0027re only looking at the request so far. I can\u0027t quite articulate why, but I feel like \"may_be_shard_listing\" might be a better name.","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef257e1f2d9ec66218ae2db46d84e17c48829f4d","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            #   increment shard_listing.backend.\u003cstatus_int\u003e"},{"line_number":307,"context_line":"            #   - it\u0027s possible that info suggested that the container was"},{"line_number":308,"context_line":"            #   sharded, but it isn\u0027t, and the request failed: we still"},{"line_number":309,"context_line":"            #   increment shard_listing.backend.\u003cstatus_int\u003e"},{"line_number":310,"context_line":"            #   - it\u0027s possible that the request succeeded but shard ranges"},{"line_number":311,"context_line":"            #   couldn\u0027t be parsed from the request body: we still increment"},{"line_number":312,"context_line":"            #   shard_listing.backend.200"}],"source_content_type":"text/x-python","patch_set":3,"id":"c473a0af_4b1eb331","line":309,"updated":"2022-01-26 20:21:34.000000000","message":"I think the point is more that info suggested that the container was sharded, and the request failed -- at that point, we have no way of knowing the container isn\u0027t sharded, we have to just assume it was based on cache.\n\nThere\u0027s also the possibility that info suggests the container is sharded, but when we make the (successful!) backend request *we discover it isn\u0027t*. At the moment, that would still increment shard_listing.backend.200 -- though I\u0027m not convinced that that\u0027s *wrong* exactly... or maybe we want my suggestion above to be more like\n\n if may_be_shard_listing and (\n         (not is_success(resp.status_int) and expected_sharded_container)\n         or (resp_record_type \u003d\u003d \u0027shard\u0027 and not cached_results)):","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"feab5857ea76e2cba75da369a0ad3727a4e2e645","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            #   increment shard_listing.backend.\u003cstatus_int\u003e"},{"line_number":307,"context_line":"            #   - it\u0027s possible that info suggested that the container was"},{"line_number":308,"context_line":"            #   sharded, but it isn\u0027t, and the request failed: we still"},{"line_number":309,"context_line":"            #   increment shard_listing.backend.\u003cstatus_int\u003e"},{"line_number":310,"context_line":"            #   - it\u0027s possible that the request succeeded but shard ranges"},{"line_number":311,"context_line":"            #   couldn\u0027t be parsed from the request body: we still increment"},{"line_number":312,"context_line":"            #   shard_listing.backend.200"}],"source_content_type":"text/x-python","patch_set":3,"id":"7b6f7a17_3564cab3","line":309,"in_reply_to":"c473a0af_4b1eb331","updated":"2022-01-27 14:13:46.000000000","message":"\u003e I think the point is more that info suggested that the container was sharded, and the request failed -- at that point, we have no way of knowing the container isn\u0027t sharded, we have to just assume it was based on cache.\n \nThat\u0027s the superset of what I\u0027m saying, but I\u0027m just highlighting the case where the metric would be wrong (i.e. container is unsharded)\n\n\u003e There\u0027s also the possibility that info suggests the container is sharded, but when we make the (successful!) backend request *we discover it isn\u0027t*. At the moment, that would still increment shard_listing.backend.200 -- though I\u0027m not convinced that that\u0027s *wrong* exactly... or maybe we want my suggestion above to be more like\n\u003e \n\u003e  if may_be_shard_listing and (\n\u003e          (not is_success(resp.status_int) and expected_sharded_container)\n\u003e          or (resp_record_type \u003d\u003d \u0027shard\u0027 and not cached_results)):\n\nwill fix","commit_id":"bd209cb56c189dccdcc04472ace64c144617c66d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"feab5857ea76e2cba75da369a0ad3727a4e2e645","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            # else:"},{"line_number":312,"context_line":"            #  The request failed, but in the absence of info we cannot assume"},{"line_number":313,"context_line":"            #  the container is sharded, so we don\u0027t increment the metric"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"},{"line_number":316,"context_line":"               resp_record_type.lower() \u003d\u003d \u0027shard\u0027)):"},{"line_number":317,"context_line":"            resp \u003d self._get_from_shards(req, resp)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b1073064_8cc12df5","line":314,"updated":"2022-01-27 14:13:46.000000000","message":"@Clay @Tim thanks for your feedback and suggestions, I took stab at making this easier to grok (?) but obvs it is less compact :)","commit_id":"df6e60aaae9ba0da43e09ab9311784f841cb2769"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b7c9bcf67aaf360ee945899697db1a041b320b8c","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            # else:"},{"line_number":312,"context_line":"            #  The request failed, but in the absence of info we cannot assume"},{"line_number":313,"context_line":"            #  the container is sharded, so we don\u0027t increment the metric"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"},{"line_number":316,"context_line":"               resp_record_type.lower() \u003d\u003d \u0027shard\u0027)):"},{"line_number":317,"context_line":"            resp \u003d self._get_from_shards(req, resp)"}],"source_content_type":"text/x-python","patch_set":5,"id":"91386d92_352cc12e","line":314,"in_reply_to":"49f9049a_f7bdcde2","updated":"2022-02-03 19:42:38.000000000","message":"We\u0027ll get failure metrics as long as\n\n* shard ranges *weren\u0027t* in  cache, so we had to go to the backend to get them,\n* that request failed, but\n* container *info* is in cache, and indicates the container is sharded\n\nKeeping container info in cache may get interesting, of course -- it\u0027s got a much shorter cache time, but it should also be a lot cheaper to get from the backend.","commit_id":"df6e60aaae9ba0da43e09ab9311784f841cb2769"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"45e2ae79ac79ac7e6724448eeea3795565ddcb3e","unresolved":true,"context_lines":[{"line_number":311,"context_line":"            # else:"},{"line_number":312,"context_line":"            #  The request failed, but in the absence of info we cannot assume"},{"line_number":313,"context_line":"            #  the container is sharded, so we don\u0027t increment the metric"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        if all((req.method \u003d\u003d \"GET\", record_type \u003d\u003d \u0027auto\u0027,"},{"line_number":316,"context_line":"               resp_record_type.lower() \u003d\u003d \u0027shard\u0027)):"},{"line_number":317,"context_line":"            resp \u003d self._get_from_shards(req, resp)"}],"source_content_type":"text/x-python","patch_set":5,"id":"49f9049a_f7bdcde2","line":314,"in_reply_to":"b1073064_8cc12df5","updated":"2022-01-31 05:03:28.000000000","message":"So it seems we\u0027ll only ever see bad status metrics if the shards are cached then? Makes sense. It\u0027s the kind of info though that would be good to write somewhere, something useful for ops. Do we have doc anywhere for metrics.\n\nSo we can only get non success metrics if there was 1 successful shard ranges GET before it failed and before it TTLs out of memcache. Totally understand why, but does mean we\u0027d might see a spike until the good TTLs out and then we don\u0027t see the error numbers spike anymore, and that might be good to know.\n\nNot a blocker from my POV, this is _much_ better then what we currently have. Just one of those things that\u0027s good to understand and might easily be forgotten (by me) :P","commit_id":"df6e60aaae9ba0da43e09ab9311784f841cb2769"}],"test/unit/proxy/controllers/test_container.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":true,"context_lines":[{"line_number":2397,"context_line":"        self.assertEqual(b\u0027\u0027, resp.body)"},{"line_number":2398,"context_line":"        self.assertEqual(404, resp.status_int)"},{"line_number":2399,"context_line":"        self.assertEqual({\u0027container.shard_listing.cache.miss\u0027: 1,"},{"line_number":2400,"context_line":"                          \u0027container.shard_listing.backend.404\u0027: 1},"},{"line_number":2401,"context_line":"                         self.logger.get_increment_counts())"},{"line_number":2402,"context_line":""},{"line_number":2403,"context_line":"    def _do_test_GET_shard_ranges_read_from_cache(self, params, record_type):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b67fbbc3_5cc8acef","line":2400,"updated":"2022-01-26 15:33:31.000000000","message":"this is the only assertion i saw for a non 200 metric name","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa08a50252d2412cb752cad95f6b08aef4f34aa3","unresolved":false,"context_lines":[{"line_number":2397,"context_line":"        self.assertEqual(b\u0027\u0027, resp.body)"},{"line_number":2398,"context_line":"        self.assertEqual(404, resp.status_int)"},{"line_number":2399,"context_line":"        self.assertEqual({\u0027container.shard_listing.cache.miss\u0027: 1,"},{"line_number":2400,"context_line":"                          \u0027container.shard_listing.backend.404\u0027: 1},"},{"line_number":2401,"context_line":"                         self.logger.get_increment_counts())"},{"line_number":2402,"context_line":""},{"line_number":2403,"context_line":"    def _do_test_GET_shard_ranges_read_from_cache(self, params, record_type):"}],"source_content_type":"text/x-python","patch_set":2,"id":"66065bdb_4dc61fcf","line":2400,"in_reply_to":"b67fbbc3_5cc8acef","updated":"2022-01-26 16:26:04.000000000","message":"Ack","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c161f9d7c3b519463da9ad08f09793646c746846","unresolved":true,"context_lines":[{"line_number":2828,"context_line":"        self._do_test_GET_shards_no_cache(sharding_state,"},{"line_number":2829,"context_line":"                                          {\u0027states\u0027: \u0027listing\u0027})"},{"line_number":2830,"context_line":"        self.assertEqual("},{"line_number":2831,"context_line":"            [mock.call.get(\u0027container/a/c\u0027),"},{"line_number":2832,"context_line":"             mock.call.set(\u0027container/a/c\u0027, mock.ANY, time\u003d60)],"},{"line_number":2833,"context_line":"            self.memcache.calls)"},{"line_number":2834,"context_line":"        self.assertEqual(sharding_state,"}],"source_content_type":"text/x-python","patch_set":2,"id":"08c8c0aa_ad6ca630","line":2831,"updated":"2022-01-26 15:33:31.000000000","message":"is this the extra get for listings?  otherwise it looks pretty much the same as -do_test_GET_shards_no_cache_updating","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa08a50252d2412cb752cad95f6b08aef4f34aa3","unresolved":true,"context_lines":[{"line_number":2828,"context_line":"        self._do_test_GET_shards_no_cache(sharding_state,"},{"line_number":2829,"context_line":"                                          {\u0027states\u0027: \u0027listing\u0027})"},{"line_number":2830,"context_line":"        self.assertEqual("},{"line_number":2831,"context_line":"            [mock.call.get(\u0027container/a/c\u0027),"},{"line_number":2832,"context_line":"             mock.call.set(\u0027container/a/c\u0027, mock.ANY, time\u003d60)],"},{"line_number":2833,"context_line":"            self.memcache.calls)"},{"line_number":2834,"context_line":"        self.assertEqual(sharding_state,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fe8542cb_f6ea8309","line":2831,"in_reply_to":"08c8c0aa_ad6ca630","updated":"2022-01-26 16:26:04.000000000","message":"yes","commit_id":"03575b6126d737c875d56284cc62929193c1a5f2"}]}