)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"At least, when there\u0027s more than one policy. Otherwise, our current"},{"line_number":10,"context_line":"assumption of Policy-0 means we may erroneously send back a 404 where a"},{"line_number":11,"context_line":"503 would\u0027ve been more appropriate."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"While we\u0027re at it, defend the object-servers a little more when the"},{"line_number":14,"context_line":"container doesn\u0027t exist; we should be fairly confident that the object"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"34c10ad2_85883d38","line":11,"updated":"2020-12-04 15:40:38.000000000","message":"idk, a 2XX would have even better","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"While we\u0027re at it, defend the object-servers a little more when the"},{"line_number":14,"context_line":"container doesn\u0027t exist; we should be fairly confident that the object"},{"line_number":15,"context_line":"doesn\u0027t either."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I14bc184d6414b73b43ccc65f3358f193480b7eaf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"d69a2b42_21d1f36e","line":15,"updated":"2020-12-04 15:40:38.000000000","message":"Counting up disks involved between a container and *all the objects in the container* - I\u0027m inclined to think the object servers will be FINE.","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"41f607db23a7991c190a831a14a171c9864f569b","unresolved":true,"context_lines":[{"line_number":1735,"context_line":"            info[\u0027partition\u0027] \u003d None"},{"line_number":1736,"context_line":"            info[\u0027nodes\u0027] \u003d None"},{"line_number":1737,"context_line":"        elif not info or not is_success(info.get(\u0027status\u0027)):"},{"line_number":1738,"context_line":"            info \u003d headers_to_container_info({}, 0)"},{"line_number":1739,"context_line":"            info[\u0027partition\u0027] \u003d None"},{"line_number":1740,"context_line":"            info[\u0027nodes\u0027] \u003d None"},{"line_number":1741,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"aa9e62e0_b671e14c","line":1738,"range":{"start_line":1738,"start_character":49,"end_line":1738,"end_character":50},"updated":"2021-01-04 20:04:25.000000000","message":"Maybe this 0 should be a 503... this always smelled weird to me.","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":1735,"context_line":"            info[\u0027partition\u0027] \u003d None"},{"line_number":1736,"context_line":"            info[\u0027nodes\u0027] \u003d None"},{"line_number":1737,"context_line":"        elif not info or not is_success(info.get(\u0027status\u0027)):"},{"line_number":1738,"context_line":"            info \u003d headers_to_container_info({}, 0)"},{"line_number":1739,"context_line":"            info[\u0027partition\u0027] \u003d None"},{"line_number":1740,"context_line":"            info[\u0027nodes\u0027] \u003d None"},{"line_number":1741,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fffa90fc_bd6249e5","line":1738,"updated":"2020-12-04 15:40:38.000000000","message":"so previously 404 landed here?","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        if container_info[\u0027status\u0027] \u003d\u003d HTTP_NOT_FOUND:"},{"line_number":243,"context_line":"            # If the container doesn\u0027t exist, the object shouldn\u0027t either"},{"line_number":244,"context_line":"            return HTTPNotFound(request\u003dreq)"},{"line_number":245,"context_line":"        elif (is_server_error(container_info[\u0027status\u0027]) or"},{"line_number":246,"context_line":"              not container_info[\u0027status\u0027]) and len(POLICIES) \u003e 1:"},{"line_number":247,"context_line":"            # We don\u0027t know what policy this container uses (assuming it"}],"source_content_type":"text/x-python","patch_set":1,"id":"93c5409b_36f0a62c","line":244,"updated":"2020-12-04 15:40:38.000000000","message":"idk, I guess we\u0027re better about returning 503 if all 3 primary containers timeout?  So we can \"trust\" this 404?  Maybe?  Unless some rouge process with old rings is polluting the cache...","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":243,"context_line":"            # If the container doesn\u0027t exist, the object shouldn\u0027t either"},{"line_number":244,"context_line":"            return HTTPNotFound(request\u003dreq)"},{"line_number":245,"context_line":"        elif (is_server_error(container_info[\u0027status\u0027]) or"},{"line_number":246,"context_line":"              not container_info[\u0027status\u0027]) and len(POLICIES) \u003e 1:"},{"line_number":247,"context_line":"            # We don\u0027t know what policy this container uses (assuming it"},{"line_number":248,"context_line":"            # even exists); it\u0027s this or trying every policy."},{"line_number":249,"context_line":"            # TODO: Maybe this should also cover other 4xx?"}],"source_content_type":"text/x-python","patch_set":1,"id":"af71cef9_8c43f86a","line":246,"updated":"2020-12-04 15:40:38.000000000","message":"is_server_error(0) returns... false?  maybe \"0\" isn\u0027t a great answer to cache for \"not is_success\"","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ae2f707098967d7d1bfef8006fc6b9c1402de96","unresolved":true,"context_lines":[{"line_number":243,"context_line":"            # If the container doesn\u0027t exist, the object shouldn\u0027t either"},{"line_number":244,"context_line":"            return HTTPNotFound(request\u003dreq)"},{"line_number":245,"context_line":"        elif (is_server_error(container_info[\u0027status\u0027]) or"},{"line_number":246,"context_line":"              not container_info[\u0027status\u0027]) and len(POLICIES) \u003e 1:"},{"line_number":247,"context_line":"            # We don\u0027t know what policy this container uses (assuming it"},{"line_number":248,"context_line":"            # even exists); it\u0027s this or trying every policy."},{"line_number":249,"context_line":"            # TODO: Maybe this should also cover other 4xx?"}],"source_content_type":"text/x-python","patch_set":1,"id":"378e70e7_4588d898","line":246,"in_reply_to":"0d1aa3be_3b0fc960","updated":"2021-01-04 20:54:16.000000000","message":"OK, got more context loaded up in my head around this: the container info really *was* coming out 404, so this isn\u0027t going to help the way I thought it would 😞\n\nThe trouble was an internal-client running on a node with very stale ring files (it had been disabled and wasn\u0027t supposed to be participating in the cluster). I don\u0027t think there\u0027s a good way for us to defend against that short of:\n\n* getting rings loaded up in the backend servers\u0027 heads,\n* sending back headers like \"yeah, I think I\u0027m supposed to be responsible for this partition\" or \"you were expecting me to be a hand-off, right?\", and then\n* having the proxy ignore 404s from backends that say they\u0027re handoffs.\n\nThen the client with the stale rings would\u0027ve gotten nothing but 503s, but at least it wouldn\u0027t go poisoning the cache. The downside, of course, would be needing to load up rings for every worker of every service...\n\nMaybe I should revisit https://review.opendev.org/#/c/670674/.","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"41f607db23a7991c190a831a14a171c9864f569b","unresolved":true,"context_lines":[{"line_number":243,"context_line":"            # If the container doesn\u0027t exist, the object shouldn\u0027t either"},{"line_number":244,"context_line":"            return HTTPNotFound(request\u003dreq)"},{"line_number":245,"context_line":"        elif (is_server_error(container_info[\u0027status\u0027]) or"},{"line_number":246,"context_line":"              not container_info[\u0027status\u0027]) and len(POLICIES) \u003e 1:"},{"line_number":247,"context_line":"            # We don\u0027t know what policy this container uses (assuming it"},{"line_number":248,"context_line":"            # even exists); it\u0027s this or trying every policy."},{"line_number":249,"context_line":"            # TODO: Maybe this should also cover other 4xx?"}],"source_content_type":"text/x-python","patch_set":1,"id":"0d1aa3be_3b0fc960","line":246,"in_reply_to":"af71cef9_8c43f86a","updated":"2021-01-04 20:04:25.000000000","message":"I don\u0027t think we *do* cache the zero, though -- we just might return it to callers :-/\n\nThis is the part of the change I care about, btw -- I got reports of proxies querying the wrong policy when there was a container-layer failure. Only container HEADs that came back were 404s (don\u0027t remember for sure if they were from handoffs or primaries-during-a-rebalance), but I think the container controller must\u0027ve arrived at a 503 response.\n\nThe thing is, going to disk is *sometimes a reasonable thing to do*, even with a 503 from the container layer. If we always did the 503, it could cause availability issues for clusters with large container DBs but only one policy.\n\nAs it is, this patch *does* cause availability issues for clusters with more than one policy and most of the large containers were in policy zero, but I\u0027m not sure that\u0027s a compelling reason to erroneously respond with 404 for containers with data *not* in policy zero.","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":5802,"context_line":"                    get_cache_key(\u0027a\u0027): {\u0027status\u0027: 200},"},{"line_number":5803,"context_line":"                    get_cache_key(\u0027a\u0027, \u0027c\u0027): container_info}})"},{"line_number":5804,"context_line":"            res \u003d controller.GET(req)"},{"line_number":5805,"context_line":"        self.assertEqual(res.status[:4], exp_status + \u0027 \u0027)"},{"line_number":5806,"context_line":"        self.assertEqual(policy_headers,"},{"line_number":5807,"context_line":"                         [] if exp_policy is None else [str(exp_policy)])"},{"line_number":5808,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a9f7cd77_dc9d8d65","line":5805,"updated":"2020-12-04 15:40:38.000000000","message":"is there no .status_int?","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0ad0c82b3c17aff1bcc867ea02c122722cd74235","unresolved":true,"context_lines":[{"line_number":5833,"context_line":"        }, \u0027200\u0027, 0)"},{"line_number":5834,"context_line":"        self._check_GET_policy_header({\u0027status\u0027: 404}, \u0027404\u0027, None)"},{"line_number":5835,"context_line":"        # Unlike prior test, if there\u0027s only one policy we can still check it"},{"line_number":5836,"context_line":"        self._check_GET_policy_header({\u0027status\u0027: 503}, \u0027200\u0027, 0)"},{"line_number":5837,"context_line":"        self._check_GET_policy_header({\u0027status\u0027: 0}, \u0027200\u0027, 0)"},{"line_number":5838,"context_line":""},{"line_number":5839,"context_line":"    def _check_GET_respects_read_affinity(self, conf, policy, expected_nodes):"}],"source_content_type":"text/x-python","patch_set":1,"id":"216dec84_a5678d38","line":5836,"updated":"2020-12-04 15:40:38.000000000","message":"so while upgrading to storage policies we would \"assume\" sp0 when an old container server didn\u0027t specifiy - but being slightly more available with only 1 policy seems weird.","commit_id":"c148a9b4984a585752f2b0033ed4e47540a76dc5"}]}