)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"79bc01e0dd8700d2b75e8a4a7c04c10628db7fde","unresolved":true,"context_lines":[{"line_number":18,"context_line":"such it probably isn\u0027t a good idea to fire questions hoping for a 301"},{"line_number":19,"context_line":"redirect response and overload it more. Instead let\u0027s skip the"},{"line_number":20,"context_line":"\"synchronous\" update part and write directly to async_pending and let"},{"line_number":21,"context_line":"the object updater deal with it later."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Hopefully this will buy the root container some extra time to get hold"},{"line_number":24,"context_line":"of itself."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7e50395b_a59535eb","line":21,"updated":"2024-05-15 19:02:40.000000000","message":"OK, so we still get the 3x deferred work, but at least we skip the inline update that was expected to time out anyway, and we\u0027ve got pretty good ratelimiting around the updater now.\n\nIt gives me an idea for the updater, though: if we see from the `db_state` that we should be getting back a redirect, just pick *one* replica at random to talk to, rather than all three at once. If it gets us a redirect, great; if it times out or errors out (which would likely be because of a DB timeout), oh well, move on. Only if it happens to eat the update (which should be unlikely) should we reach out to the other replicas.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"894b18e4cca3782d319c19d75dca582ca4789606","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"10933dd1_c418ce47","updated":"2024-05-15 07:33:05.000000000","message":"Also adding a bunch of increments and is might be useful to knwo how many we skip or if/when we get redirets.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"79bc01e0dd8700d2b75e8a4a7c04c10628db7fde","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7550e040_af1d89aa","updated":"2024-05-15 19:02:40.000000000","message":"Looks like it\u0027ll do what it says on the tin. I wouldn\u0027t mind seeing it in action locally before I +2","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a82f09902606c8760d571e865a5daee8e31e8c72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0fb5b312_4b9964af","updated":"2024-05-15 16:32:23.000000000","message":"Thanks for taking this up - I need to look at tests","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b0e874aadaa32fd5969899f61061b80a04a8942b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"eb096685_ee065163","updated":"2024-05-15 07:34:24.000000000","message":"probably need more tests too.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"359b0ec83698fd17fbf4cb000c87e0d34bb2782b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9f2fa01c_bc7a0eb6","updated":"2024-05-20 19:46:26.000000000","message":"I tried out lifting the decision to skip above the for loop:\n\n920063: sq? move stuff around | https://review.opendev.org/c/openstack/swift/+/920063\n\nended up accidently reworking stats some too...\n\nI\u0027m going to set that down and try to figure out if the tests can/should be refactored","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"03aa87bef264fc6620a8ad42c17d3c809de1c5fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d9de15ab_ff67007b","updated":"2024-05-17 04:24:33.000000000","message":"opps forgot to publish these drafts","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f625b1f561f78668850a65337a731de84c7b0cff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b5db322f_eb3e52e4","updated":"2025-04-10 00:21:33.000000000","message":"(I should probably revisit this.)","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"2f7e7395_1691766f","updated":"2024-08-05 12:03:06.000000000","message":"I agree this is a good direction to go to try to relieve some of the root container load we see in prod.\n\nSuggested test improvements: https://review.opendev.org/c/openstack/swift/+/925689\n\nI\u0027d suggest keeping any refactoring of async_update to a separate patch.\n\nI think I\u0027d prefer the proxy to control the decision i.e. send X-Backend-Skip-Sync-Update or similar.","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e2a50d2a97ac8ac925eec380e824ba3af612878a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7e74276a_34771d83","updated":"2025-01-15 17:47:04.000000000","message":"Since the proxy already passes the db_state on master, the trust of the review feedback here seems to be have it *also* send an explicit x-backend-skip-async-update\n\n... and then maybe some non-material rewordings in the code/tests","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4815595c_26cf5cd1","updated":"2024-07-15 20:32:00.000000000","message":"This seems to be working as expected, but it\u0027s opened some discussion about the correct way to manage (possibly multiple) sync updates (esp when we plan to go direct to async), the alternative option of making the decision in the proxy, the question of the desired behavior with `db_state\u003d\u0027sharding\u0027`.\n\nI\u0027ll hold off on approving for now until those discussions can be resolved\n\nI am mostly happy with the change as-is, I thought some additional investment could be made to \"improve\" testing (not sure what others will think of my attempts 924180: tests: more tests for skip_sync_update | https://review.opendev.org/c/openstack/swift/+/924180 - but I think the *could* be squashed if they\u0027re helpful as we examine other behaviors and/or interations of the various proxy container update headers with the object-server sync/async behavior)","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"}],"swift/obj/server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a82f09902606c8760d571e865a5daee8e31e8c72","unresolved":true,"context_lines":[{"line_number":289,"context_line":"        :param logger_thread_locals: The thread local values to be set on the"},{"line_number":290,"context_line":"                                     self.logger to retain transaction"},{"line_number":291,"context_line":"                                     logging information."},{"line_number":292,"context_line":"        :param container_path: optional path in the form `\u003caccount/container\u003e`"},{"line_number":293,"context_line":"            to which the update should be sent. If given this path will be used"},{"line_number":294,"context_line":"            instead of constructing a path from the ``account`` and"},{"line_number":295,"context_line":"            ``container`` params."}],"source_content_type":"text/x-python","patch_set":2,"id":"43b6615c_11bbc960","line":292,"updated":"2024-05-15 16:32:23.000000000","message":"oic, container_path has a / in it - this is the interface the proxy uses to tell the upate to go to `.shards_AUTH_test/some-shard-db-name` instad of normal `AUTH_test/root` from the object\u0027s path.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"03aa87bef264fc6620a8ad42c17d3c809de1c5fd","unresolved":true,"context_lines":[{"line_number":289,"context_line":"        :param logger_thread_locals: The thread local values to be set on the"},{"line_number":290,"context_line":"                                     self.logger to retain transaction"},{"line_number":291,"context_line":"                                     logging information."},{"line_number":292,"context_line":"        :param container_path: optional path in the form `\u003caccount/container\u003e`"},{"line_number":293,"context_line":"            to which the update should be sent. If given this path will be used"},{"line_number":294,"context_line":"            instead of constructing a path from the ``account`` and"},{"line_number":295,"context_line":"            ``container`` params."}],"source_content_type":"text/x-python","patch_set":2,"id":"966d37db_56ecf5a0","line":292,"in_reply_to":"43b6615c_11bbc960","updated":"2024-05-17 04:24:33.000000000","message":"yup, because originally we didn\u0027t want to loose the root path, so we can always fall back to the root location. In hindsight, the shard naming means we could determine the root from the path. But that ship sailed years ago.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"33ec8ca0b6c21d8e5a674cbf3e5e043c80c1d712","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        :param logger_thread_locals: The thread local values to be set on the"},{"line_number":290,"context_line":"                                     self.logger to retain transaction"},{"line_number":291,"context_line":"                                     logging information."},{"line_number":292,"context_line":"        :param container_path: optional path in the form `\u003caccount/container\u003e`"},{"line_number":293,"context_line":"            to which the update should be sent. If given this path will be used"},{"line_number":294,"context_line":"            instead of constructing a path from the ``account`` and"},{"line_number":295,"context_line":"            ``container`` params."}],"source_content_type":"text/x-python","patch_set":2,"id":"cb66fa6d_cdb97875","line":292,"in_reply_to":"966d37db_56ecf5a0","updated":"2024-07-11 03:29:09.000000000","message":"Acknowledged","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"894b18e4cca3782d319c19d75dca582ca4789606","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        # This is usually caused by the container servers being overloaded."},{"line_number":312,"context_line":"        # We don\u0027t want to add to the problem so we want to skip trying to"},{"line_number":313,"context_line":"        # query the root synchronously and write directly to async_pending"},{"line_number":314,"context_line":"        skip_sync_update \u003d db_state \u003d\u003d SHARDED and not container_path"},{"line_number":315,"context_line":"        if skip_sync_update:"},{"line_number":316,"context_line":"            self.logger.increment(\u0027sync_update.skip\u0027)"},{"line_number":317,"context_line":"        elif all([host, partition, contdevice]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"974e5581_b859a74c","line":314,"range":{"start_line":314,"start_character":39,"end_line":314,"end_character":46},"updated":"2024-05-15 07:33:05.000000000","message":"I was going to add SHARDING here too, but there may actaully not be a container_path ready yet. So just looking for SHARDED.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"359b0ec83698fd17fbf4cb000c87e0d34bb2782b","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        # This is usually caused by the container servers being overloaded."},{"line_number":312,"context_line":"        # We don\u0027t want to add to the problem so we want to skip trying to"},{"line_number":313,"context_line":"        # query the root synchronously and write directly to async_pending"},{"line_number":314,"context_line":"        skip_sync_update \u003d db_state \u003d\u003d SHARDED and not container_path"},{"line_number":315,"context_line":"        if skip_sync_update:"},{"line_number":316,"context_line":"            self.logger.increment(\u0027sync_update.skip\u0027)"},{"line_number":317,"context_line":"        elif all([host, partition, contdevice]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"77f10b3e_1c2254a7","line":314,"range":{"start_line":314,"start_character":39,"end_line":314,"end_character":46},"in_reply_to":"3f463c53_d27ec0fd","updated":"2024-05-20 19:46:26.000000000","message":"are you saying the 301 would be conditional on if the update falls into a shard db that\u0027s already created?  Aside for initial-sharding-migration I think it\u0027s rare for databases to split into \u003e2 shards; is our shard-batch-size really such that we could have a whole cycle-sized race window where a shard might 301 some updates and eat others for future cleaving?","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"03aa87bef264fc6620a8ad42c17d3c809de1c5fd","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        # This is usually caused by the container servers being overloaded."},{"line_number":312,"context_line":"        # We don\u0027t want to add to the problem so we want to skip trying to"},{"line_number":313,"context_line":"        # query the root synchronously and write directly to async_pending"},{"line_number":314,"context_line":"        skip_sync_update \u003d db_state \u003d\u003d SHARDED and not container_path"},{"line_number":315,"context_line":"        if skip_sync_update:"},{"line_number":316,"context_line":"            self.logger.increment(\u0027sync_update.skip\u0027)"},{"line_number":317,"context_line":"        elif all([host, partition, contdevice]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f463c53_d27ec0fd","line":314,"range":{"start_line":314,"start_character":39,"end_line":314,"end_character":46},"in_reply_to":"66ab2f62_e72eb2ce","updated":"2024-05-17 04:24:33.000000000","message":"@Tim right. But also while we\u0027re sharding we do it in batches. So we might have some CLEAVED and responsible for updates and some (at the end) still in CREATED which aren\u0027t. in the latter we should get the root container, and these would be skipped, when maybe we don\u0027t want that.\n\nOn the otherhand, holding off sending updates to a sharding container I guess could give it more time to shard and it\u0027s going to go via root anyway.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        # This is usually caused by the container servers being overloaded."},{"line_number":312,"context_line":"        # We don\u0027t want to add to the problem so we want to skip trying to"},{"line_number":313,"context_line":"        # query the root synchronously and write directly to async_pending"},{"line_number":314,"context_line":"        skip_sync_update \u003d db_state \u003d\u003d SHARDED and not container_path"},{"line_number":315,"context_line":"        if skip_sync_update:"},{"line_number":316,"context_line":"            self.logger.increment(\u0027sync_update.skip\u0027)"},{"line_number":317,"context_line":"        elif all([host, partition, contdevice]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9034be57_99dbb36f","line":314,"range":{"start_line":314,"start_character":39,"end_line":314,"end_character":46},"in_reply_to":"77f10b3e_1c2254a7","updated":"2024-07-15 20:32:00.000000000","message":"Regardless of the conclusion of this investigation I think the behivor on SHARDED is clearly what we want; it\u0027s just a question about if we\u0027ll want the same optimization for SHARDING and I don\u0027t think we have any evidence yet that will matter to us in prod - but we may determine in testing it would also be correct/desirable to optimize that path as well.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a82f09902606c8760d571e865a5daee8e31e8c72","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        # This is usually caused by the container servers being overloaded."},{"line_number":312,"context_line":"        # We don\u0027t want to add to the problem so we want to skip trying to"},{"line_number":313,"context_line":"        # query the root synchronously and write directly to async_pending"},{"line_number":314,"context_line":"        skip_sync_update \u003d db_state \u003d\u003d SHARDED and not container_path"},{"line_number":315,"context_line":"        if skip_sync_update:"},{"line_number":316,"context_line":"            self.logger.increment(\u0027sync_update.skip\u0027)"},{"line_number":317,"context_line":"        elif all([host, partition, contdevice]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fdbabba6_1f0eded9","line":314,"range":{"start_line":314,"start_character":39,"end_line":314,"end_character":46},"in_reply_to":"974e5581_b859a74c","updated":"2024-05-15 16:32:23.000000000","message":"if SHARDING dbs will return just return 301 anyway it might make sense let them go to async and find out the target shard from the root later.  But AFAIK our main problem has been with super enormous roots that have 10K shard-ranges in them so the 301 queries are slow - they\u0027ve been sharded for a long time already; so this is probably sufficient!","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"79bc01e0dd8700d2b75e8a4a7c04c10628db7fde","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        # This is usually caused by the container servers being overloaded."},{"line_number":312,"context_line":"        # We don\u0027t want to add to the problem so we want to skip trying to"},{"line_number":313,"context_line":"        # query the root synchronously and write directly to async_pending"},{"line_number":314,"context_line":"        skip_sync_update \u003d db_state \u003d\u003d SHARDED and not container_path"},{"line_number":315,"context_line":"        if skip_sync_update:"},{"line_number":316,"context_line":"            self.logger.increment(\u0027sync_update.skip\u0027)"},{"line_number":317,"context_line":"        elif all([host, partition, contdevice]):"}],"source_content_type":"text/x-python","patch_set":2,"id":"66ab2f62_e72eb2ce","line":314,"range":{"start_line":314,"start_character":39,"end_line":314,"end_character":46},"in_reply_to":"fdbabba6_1f0eded9","updated":"2024-05-15 19:02:40.000000000","message":"\u003e there may actaully not be a container_path ready yet\n\nMatt, are you thinking of when the sharder is still in the process of identifying `FOUND` shards, prior to creating the DBs and marking the shards `CREATED`? IIRC, the *db_state* at that point is still `UNSHARDED`, though the own-shard-range state is `SHARDING`. So yeah, I think it ought to be reasonable to look for `db_state in (SHARDING, SHARDED)`","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"79bc01e0dd8700d2b75e8a4a7c04c10628db7fde","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    except ValueError as err:"},{"line_number":335,"context_line":"                        self.logger.error("},{"line_number":336,"context_line":"                            \u0027Container update failed for %r; problem with \u0027"},{"line_number":337,"context_line":"                            \u0027redirect location: %s\u0027 % (obj, err))"},{"line_number":338,"context_line":"                else:"},{"line_number":339,"context_line":"                    self.logger.error("},{"line_number":340,"context_line":"                        \u0027ERROR Container update failed \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"8bd68909_7d5b922a","line":337,"updated":"2024-05-15 19:02:40.000000000","message":"We should probably have a\n```\nself.logger.increment(\u0027sync_update.error\u0027)\n```\nhere, too. (Though I don\u0027t think we can ever seen this error in prod.)","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":334,"context_line":"                    except ValueError as err:"},{"line_number":335,"context_line":"                        self.logger.error("},{"line_number":336,"context_line":"                            \u0027Container update failed for %r; problem with \u0027"},{"line_number":337,"context_line":"                            \u0027redirect location: %s\u0027 % (obj, err))"},{"line_number":338,"context_line":"                else:"},{"line_number":339,"context_line":"                    self.logger.error("},{"line_number":340,"context_line":"                        \u0027ERROR Container update failed \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a0cfab6_3eb26854","line":337,"in_reply_to":"8bd68909_7d5b922a","updated":"2024-07-15 20:32:00.000000000","message":"I think this should emit `sync_udpate.error.301` or `sync_update.success.301` depending...\n\nbut it seems untested; nice catch!\n\nI added tests and the behavior seems correct to my reading, i.e.\n\nhappy path 301:\n\n```\n+        self.assertEqual({\n+            \u0027sync_update.success.301\u0027: 1,\n+            \u0027async_pendings\u0027: 1,\n+        }, self.logger.statsd_client.get_increment_counts())\n```\n\nValueErorr path 301:\n\n```\n+            self.assertEqual({\n+                \u0027sync_update.error.301\u0027: 1,\n+                \u0027async_pendings\u0027: 1,\n+            }, self.logger.statsd_client.get_increment_counts())\n```","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a82f09902606c8760d571e865a5daee8e31e8c72","unresolved":true,"context_lines":[{"line_number":347,"context_line":"                self.logger.exception("},{"line_number":348,"context_line":"                    \u0027ERROR container update failed with \u0027"},{"line_number":349,"context_line":"                    \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,"},{"line_number":350,"context_line":"                    {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice})"},{"line_number":351,"context_line":"        data \u003d {\u0027op\u0027: op, \u0027account\u0027: account, \u0027container\u0027: container,"},{"line_number":352,"context_line":"                \u0027obj\u0027: obj, \u0027headers\u0027: headers_out, \u0027db_state\u0027: db_state}"},{"line_number":353,"context_line":"        if redirect_data:"}],"source_content_type":"text/x-python","patch_set":2,"id":"2c2cc6c8_863f4dd4","line":350,"updated":"2024-05-15 16:32:23.000000000","message":"can we get one more stat increment down here?\n\nIn the new world we\u0027d probably just add a single:\n\n    self.stats.increment(\u0027sync_update\u0027, {result\u003d\u0027redirect|error|timeout|success\u0027})\n    \n... in a finally block or something!","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"03aa87bef264fc6620a8ad42c17d3c809de1c5fd","unresolved":true,"context_lines":[{"line_number":347,"context_line":"                self.logger.exception("},{"line_number":348,"context_line":"                    \u0027ERROR container update failed with \u0027"},{"line_number":349,"context_line":"                    \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,"},{"line_number":350,"context_line":"                    {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice})"},{"line_number":351,"context_line":"        data \u003d {\u0027op\u0027: op, \u0027account\u0027: account, \u0027container\u0027: container,"},{"line_number":352,"context_line":"                \u0027obj\u0027: obj, \u0027headers\u0027: headers_out, \u0027db_state\u0027: db_state}"},{"line_number":353,"context_line":"        if redirect_data:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fb09a030_d57c6065","line":350,"in_reply_to":"2c2cc6c8_863f4dd4","updated":"2024-05-17 04:24:33.000000000","message":"oh true, I should probably familarise myself with the new world. As I\u0027m just adding more work later.\nIn the meantime I\u0027ll only call increment once so it\u0027s less burden to change.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"359b0ec83698fd17fbf4cb000c87e0d34bb2782b","unresolved":true,"context_lines":[{"line_number":350,"context_line":"                    \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,"},{"line_number":351,"context_line":"                    {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice})"},{"line_number":352,"context_line":"            finally:"},{"line_number":353,"context_line":"                self.logger.increment(\u0027sync_update.%s\u0027 % async_result)"},{"line_number":354,"context_line":"        data \u003d {\u0027op\u0027: op, \u0027account\u0027: account, \u0027container\u0027: container,"},{"line_number":355,"context_line":"                \u0027obj\u0027: obj, \u0027headers\u0027: headers_out, \u0027db_state\u0027: db_state}"},{"line_number":356,"context_line":"        if redirect_data:"}],"source_content_type":"text/x-python","patch_set":3,"id":"0f853564_a0a7a390","line":353,"updated":"2024-05-20 19:46:26.000000000","message":"might be clearer (but perhaps more code churn) if this finally bock also covered the skip case.","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":350,"context_line":"                    \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,"},{"line_number":351,"context_line":"                    {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice})"},{"line_number":352,"context_line":"            finally:"},{"line_number":353,"context_line":"                self.logger.increment(\u0027sync_update.%s\u0027 % async_result)"},{"line_number":354,"context_line":"        data \u003d {\u0027op\u0027: op, \u0027account\u0027: account, \u0027container\u0027: container,"},{"line_number":355,"context_line":"                \u0027obj\u0027: obj, \u0027headers\u0027: headers_out, \u0027db_state\u0027: db_state}"},{"line_number":356,"context_line":"        if redirect_data:"}],"source_content_type":"text/x-python","patch_set":3,"id":"d518b528_5071a8d9","line":353,"in_reply_to":"0f853564_a0a7a390","updated":"2024-07-15 20:32:00.000000000","message":"the latest patch unifies all metrics to the finally block","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1093ad9678c5d225f70d8f17441c22cfe99228ab","unresolved":true,"context_lines":[{"line_number":296,"context_line":"            ``container`` params."},{"line_number":297,"context_line":"        :param db_state: The current database state of the container as"},{"line_number":298,"context_line":"            supplied to us by the proxy."},{"line_number":299,"context_line":"        :param attempt_sync_update: boolean, when falsey all spawned threads"},{"line_number":300,"context_line":"                                    write the update data directly to async"},{"line_number":301,"context_line":"        \"\"\""},{"line_number":302,"context_line":"        if logger_thread_locals:"}],"source_content_type":"text/x-python","patch_set":6,"id":"553102ff_35e7bca8","line":299,"updated":"2024-05-27 06:32:56.000000000","message":"This was for demonstration purposes only. I\u0027ll revert later.","commit_id":"2290a8ae79b66d0377185fdecbb2b9234a18ae07"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            ``container`` params."},{"line_number":297,"context_line":"        :param db_state: The current database state of the container as"},{"line_number":298,"context_line":"            supplied to us by the proxy."},{"line_number":299,"context_line":"        :param attempt_sync_update: boolean, when falsey all spawned threads"},{"line_number":300,"context_line":"                                    write the update data directly to async"},{"line_number":301,"context_line":"        \"\"\""},{"line_number":302,"context_line":"        if logger_thread_locals:"}],"source_content_type":"text/x-python","patch_set":6,"id":"c0cf5ade_abd39a4f","line":299,"in_reply_to":"553102ff_35e7bca8","updated":"2024-07-15 20:32:00.000000000","message":"what do you mean?  revert the doc string spelling fix?  I think `false` is fine...","commit_id":"2290a8ae79b66d0377185fdecbb2b9234a18ae07"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"baad32ce2f1f22f3178c7ba1649fd7d88038ca0d","unresolved":true,"context_lines":[{"line_number":269,"context_line":"        return self._diskfile_router[policy].get_diskfile("},{"line_number":270,"context_line":"            device, partition, account, container, obj, policy, **kwargs)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def async_update(self, op, account, container, obj, host, partition,"},{"line_number":273,"context_line":"                     contdevice, headers_out, objdevice, policy,"},{"line_number":274,"context_line":"                     logger_thread_locals\u003dNone, container_path\u003dNone,"},{"line_number":275,"context_line":"                     db_state\u003dNone, attempt_sync_update\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":7,"id":"dd6b8f7e_f4016996","line":272,"updated":"2024-06-03 16:41:53.000000000","message":"this function is called \"async_update\", but actually it tries sync_update first, if container servers timeout or getting a 301 redirect, it then does the real async_update, and the ``return`` of ``sync_update`` is deeply nested within this function. \n\nI feel we should split this function into three (``update_container``, ``sync_update`` and ``async_update``), and the newly added parameter ``attempt_sync_update`` made me leaning toward this more, something like this below:\n\n```\ndef update_container(self, ...):\n    if attempt_sync_update:\n         response \u003d self.sync_update(....)\n         if is_success(response.status):\n             return\n    # skip sync update, or get redirect/errors from sync_update\n    self.async_update(...)\n```","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"56c4a857c99162c2e93811f8aec1b95febbe02e6","unresolved":true,"context_lines":[{"line_number":269,"context_line":"        return self._diskfile_router[policy].get_diskfile("},{"line_number":270,"context_line":"            device, partition, account, container, obj, policy, **kwargs)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def async_update(self, op, account, container, obj, host, partition,"},{"line_number":273,"context_line":"                     contdevice, headers_out, objdevice, policy,"},{"line_number":274,"context_line":"                     logger_thread_locals\u003dNone, container_path\u003dNone,"},{"line_number":275,"context_line":"                     db_state\u003dNone, attempt_sync_update\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":7,"id":"2b35e3d3_a4647c19","line":272,"in_reply_to":"8b401058_babdd779","updated":"2024-07-17 08:05:33.000000000","message":"Maybe something like: https://review.opendev.org/c/openstack/swift/+/924304 although that still needs much more work and refactoring.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":269,"context_line":"        return self._diskfile_router[policy].get_diskfile("},{"line_number":270,"context_line":"            device, partition, account, container, obj, policy, **kwargs)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def async_update(self, op, account, container, obj, host, partition,"},{"line_number":273,"context_line":"                     contdevice, headers_out, objdevice, policy,"},{"line_number":274,"context_line":"                     logger_thread_locals\u003dNone, container_path\u003dNone,"},{"line_number":275,"context_line":"                     db_state\u003dNone, attempt_sync_update\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":7,"id":"8b401058_babdd779","line":272,"in_reply_to":"cb94ed1c_3b96f166","updated":"2024-07-15 20:32:00.000000000","message":"i think Jian has the right idea, but I\u0027ve struggled with how to phrase it correctly myself because the sync_update has a number of exit paths\n\n1) it has to update multiple containers (think 3-replica db and 2-replica object)\n2) it can complete the sync update but SLOWLY and the object-server will return success before finishing the sync update because it knows if it fails it will go to async\n\nTo do this \"right\" - you\u0027d have to have a \"watcher thread\" that gets pushed to the background and starts and watches the sync updates, the object-server would then wait for the watcher to complete but returns to to the object-server regardless after a timeout.  Then this watcher thread would be left in the background to be the \"one and only\" place we maybe create at async-pending if \"any\" of the container-updater fail.  We\u0027d have to be careful to ensure any unexpected exception in the sync udpate threads path always writes an async tho - I think that\u0027s somewhat easier/more-obvious to write when the same greenthread doing the sync update will create an asnyc-pending if it fails.\n\nMaybe more than we want to take on in THIS patch, but \"attempt_sync_udpate\" might be getting us closer.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"33ec8ca0b6c21d8e5a674cbf3e5e043c80c1d712","unresolved":true,"context_lines":[{"line_number":269,"context_line":"        return self._diskfile_router[policy].get_diskfile("},{"line_number":270,"context_line":"            device, partition, account, container, obj, policy, **kwargs)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def async_update(self, op, account, container, obj, host, partition,"},{"line_number":273,"context_line":"                     contdevice, headers_out, objdevice, policy,"},{"line_number":274,"context_line":"                     logger_thread_locals\u003dNone, container_path\u003dNone,"},{"line_number":275,"context_line":"                     db_state\u003dNone, attempt_sync_update\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":7,"id":"cb94ed1c_3b96f166","line":272,"in_reply_to":"dd6b8f7e_f4016996","updated":"2024-07-11 03:29:09.000000000","message":"I\u0027ve reworked this multiple ways, and it seems to come out a bit messy because of so much shared state. Still not saying this isn\u0027t worth it. Just as each need the same variables etc there seems to be a bunch of boiler plate just to split them up and haven\u0027t been happy with any of the results so far. So will maybe push up a version without this change first.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"baad32ce2f1f22f3178c7ba1649fd7d88038ca0d","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        redirect_data \u003d None"},{"line_number":312,"context_line":"        try:"},{"line_number":313,"context_line":"            if attempt_sync_update and all([host, partition, contdevice]):"},{"line_number":314,"context_line":"                sync_update_ctx \u003d {\u0027success\u0027: False}"},{"line_number":315,"context_line":"                with ConnectionTimeout(self.conn_timeout):"},{"line_number":316,"context_line":"                    ip, port \u003d host.rsplit(\u0027:\u0027, 1)"},{"line_number":317,"context_line":"                    conn \u003d http_connect(ip, port, contdevice, partition, op,"}],"source_content_type":"text/x-python","patch_set":7,"id":"2b105684_f7dbb36d","line":314,"updated":"2024-06-03 16:41:53.000000000","message":"``sync_update_ctx`` includes many status string and each of them is a single key/value in the dict: ``success``, ``redirect``, ``skip``, ``timeout``, ``exception``, but it seems those status strings don\u0027t overlap with each other, so we could just use one status string variable?","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":311,"context_line":"        redirect_data \u003d None"},{"line_number":312,"context_line":"        try:"},{"line_number":313,"context_line":"            if attempt_sync_update and all([host, partition, contdevice]):"},{"line_number":314,"context_line":"                sync_update_ctx \u003d {\u0027success\u0027: False}"},{"line_number":315,"context_line":"                with ConnectionTimeout(self.conn_timeout):"},{"line_number":316,"context_line":"                    ip, port \u003d host.rsplit(\u0027:\u0027, 1)"},{"line_number":317,"context_line":"                    conn \u003d http_connect(ip, port, contdevice, partition, op,"}],"source_content_type":"text/x-python","patch_set":7,"id":"0904c182_b824ad27","line":314,"in_reply_to":"18c6e9d7_48b3c56e","updated":"2024-07-15 20:32:00.000000000","message":"I\u0027m attempted to think through how I\u0027d write this if we had the new labeled metrics interface.  Since we don\u0027t currently have a way to do annotated labels in metrics at the end of the method we have to \"compress\" the ctx dict into a single string - so, yes, we COULD just keep a single string a modify it as we go - but I actually find the new label/ctx style a easier to reason about and will be quite happy to start doing stats this way once we have labeled metrics.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"33ec8ca0b6c21d8e5a674cbf3e5e043c80c1d712","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        redirect_data \u003d None"},{"line_number":312,"context_line":"        try:"},{"line_number":313,"context_line":"            if attempt_sync_update and all([host, partition, contdevice]):"},{"line_number":314,"context_line":"                sync_update_ctx \u003d {\u0027success\u0027: False}"},{"line_number":315,"context_line":"                with ConnectionTimeout(self.conn_timeout):"},{"line_number":316,"context_line":"                    ip, port \u003d host.rsplit(\u0027:\u0027, 1)"},{"line_number":317,"context_line":"                    conn \u003d http_connect(ip, port, contdevice, partition, op,"}],"source_content_type":"text/x-python","patch_set":7,"id":"18c6e9d7_48b3c56e","line":314,"in_reply_to":"2b105684_f7dbb36d","updated":"2024-07-11 03:29:09.000000000","message":"yeah there\u0027s success, redirect, skip, error and status. I guess the booleans are used for checking. But could probably reduce some of this, maybe I\u0027ll pull them out of the dict and see how it looks. I mean we have redirect_data which could just as easily as be used instead of the redirect key. And error and status could just really be one variable","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f3fd2584fe984e3d555f61f974b3a8879c11fced","unresolved":true,"context_lines":[{"line_number":342,"context_line":"                        {\u0027status\u0027: response.status, \u0027ip\u0027: ip, \u0027port\u0027: port,"},{"line_number":343,"context_line":"                         \u0027dev\u0027: contdevice})"},{"line_number":344,"context_line":"            else:"},{"line_number":345,"context_line":"                sync_update_ctx \u003d {\u0027skip\u0027: True}"},{"line_number":346,"context_line":"        except (Exception, Timeout) as e:"},{"line_number":347,"context_line":"            if isinstance(e, Timeout):"},{"line_number":348,"context_line":"                error \u003d \u0027timeout\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"2809b38e_9c747cff","line":345,"updated":"2024-06-21 22:58:13.000000000","message":"Might want to flip the conditional, like\n```\nif not attempt_sync_update or not all([host, partition, contdevice]):\n    sync_update_ctx \u003d {\u0027skip\u0027: True}\nelse:\n    ...\n```\njust to make it easier to track `if`/`else`s.\n\nAlso makes it more clear that we\u0027re adding some new logic around `not all([host, partition, contdevice])` regardless of the `attempt_sync_update` flag... was that intentional?\n\nHmm.. makes me wonder... What if the proxy just skipped sending container-server host/device/partition info when *it* knows the DB\u0027s sharded but can\u0027t get shards? Do we need to make any object-server changes at all to get the behavior change we want?","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":342,"context_line":"                        {\u0027status\u0027: response.status, \u0027ip\u0027: ip, \u0027port\u0027: port,"},{"line_number":343,"context_line":"                         \u0027dev\u0027: contdevice})"},{"line_number":344,"context_line":"            else:"},{"line_number":345,"context_line":"                sync_update_ctx \u003d {\u0027skip\u0027: True}"},{"line_number":346,"context_line":"        except (Exception, Timeout) as e:"},{"line_number":347,"context_line":"            if isinstance(e, Timeout):"},{"line_number":348,"context_line":"                error \u003d \u0027timeout\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"56f413b2_90c5d3cf","line":345,"in_reply_to":"11c44b81_cd63020a","updated":"2024-07-22 22:41:50.000000000","message":"OK, so [partition *is* still needed](https://github.com/openstack/swift/blob/2.33.0/swift/obj/server.py#L408-L409) and I\u0027m realizing that\n\n1. \"skip sending host and drive to write directly to async_pendings\" might [only go back to 1.8.0](https://github.com/openstack/swift/commit/6ff644b945dfc025414f6d0775f409a938d85512)\n2. looking at [the review that made that work](https://review.opendev.org/c/openstack/swift/+/18263), I don\u0027t think anyone realized/noticed that functional change\n3. I don\u0027t think anyone has ever **used** this behavior before, and\n4. Clay\u0027s right, it\u0027s a weird string to pull at to get the behavior you want.\n\nAs far as\n\n\u003e \"doesn\u0027t require object-server changes\" isn\u0027t obviously virtuous\n\nthough, the big win (IMO) is trying to avoid thinking much about rolling-upgrades. But maybe in light of the above, it\u0027s worth having these updates to both sides.\n\n---\n\nI guess part of my thinking was that it felt weird to have identical conditions (\"container is sharded and sufficiently overloaded that I can\u0027t know shard ranges\") checked at two layers and in rather different ways -- so much so that the connection seems not entirely obvious.\n\nAnd only *one* of them is actually capable of trying to get the info it needs 😕","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1301b7d3f1660aa2f7f431da4238380547747e45","unresolved":true,"context_lines":[{"line_number":342,"context_line":"                        {\u0027status\u0027: response.status, \u0027ip\u0027: ip, \u0027port\u0027: port,"},{"line_number":343,"context_line":"                         \u0027dev\u0027: contdevice})"},{"line_number":344,"context_line":"            else:"},{"line_number":345,"context_line":"                sync_update_ctx \u003d {\u0027skip\u0027: True}"},{"line_number":346,"context_line":"        except (Exception, Timeout) as e:"},{"line_number":347,"context_line":"            if isinstance(e, Timeout):"},{"line_number":348,"context_line":"                error \u003d \u0027timeout\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"dce9989e_cdc20298","line":345,"in_reply_to":"2809b38e_9c747cff","updated":"2024-06-25 00:49:57.000000000","message":"Yeah I\u0027ll take a look, that could be more readable.\n\nWe should still send container update info because we still want to update the container info via async, because we don\u0027t want a ghost listing. Otherwise what we fail to write the object in question?","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"56c4a857c99162c2e93811f8aec1b95febbe02e6","unresolved":true,"context_lines":[{"line_number":342,"context_line":"                        {\u0027status\u0027: response.status, \u0027ip\u0027: ip, \u0027port\u0027: port,"},{"line_number":343,"context_line":"                         \u0027dev\u0027: contdevice})"},{"line_number":344,"context_line":"            else:"},{"line_number":345,"context_line":"                sync_update_ctx \u003d {\u0027skip\u0027: True}"},{"line_number":346,"context_line":"        except (Exception, Timeout) as e:"},{"line_number":347,"context_line":"            if isinstance(e, Timeout):"},{"line_number":348,"context_line":"                error \u003d \u0027timeout\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"11c44b81_cd63020a","line":345,"in_reply_to":"dcb643ae_97c2a21b","updated":"2024-07-17 08:05:33.000000000","message":"\u003e \u003e What if the proxy just skipped sending container-server host/device/partition info when it knows the DB\u0027s sharded but can\u0027t get shards\n\u003e \n\u003e I think that Tim is right: there\u0027s multiple ways to achive this behavior - we could put the proxy in control or have object server make it\u0027s own decisions.  But \"doesn\u0027t require object-server changes\" isn\u0027t obviously virtous if if means \"we re-use this *other* thing to MEAN the db is sharded and I couldn\u0027t get container_path so don\u0027t do a sync update\" instead of just passing along what we know.\n\nIt does sound like a good idea, I just worry that then we\u0027d need yet another way of writing the obj to the container when it is ready. Currently this is async_pending and created on the object server. We could just drop the object PUT and return a 50x back in the proxy, leaving it to the client to try again later. Otherwise, just not sending them  to the object server and put it into async pending, we\u0027d be getting missing listings for objects all over the place.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":342,"context_line":"                        {\u0027status\u0027: response.status, \u0027ip\u0027: ip, \u0027port\u0027: port,"},{"line_number":343,"context_line":"                         \u0027dev\u0027: contdevice})"},{"line_number":344,"context_line":"            else:"},{"line_number":345,"context_line":"                sync_update_ctx \u003d {\u0027skip\u0027: True}"},{"line_number":346,"context_line":"        except (Exception, Timeout) as e:"},{"line_number":347,"context_line":"            if isinstance(e, Timeout):"},{"line_number":348,"context_line":"                error \u003d \u0027timeout\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"dcb643ae_97c2a21b","line":345,"in_reply_to":"dce9989e_cdc20298","updated":"2024-07-15 20:32:00.000000000","message":"\u003e just to make it easier to track if/elses.\n\nI feel like the order here is \"happy path first\" - but take your point about keeping the else closer to the if; I was probably thinking of it more like a try/except.  Maybe just extract the if block to a parameterized method if the distance between the if and else is too long?  (currently ~20 lines, I think \"success\" \u003d\u003d 2XX \u003d\u003e return vs \"success\" \u003d\u003d 301 \u003d\u003e async will require some kind tuple return value signature)\n\n\u003e regardless of the attempt_sync_update flag... was that intentional?\n\nYes, that\u0027s the correct behavior as we\u0027ve defined it, maybe a better param name would be \"force_skip_sync\" to indicate \"skip_sync\" *might* happen anyway - BUT we could alternatively lift the `all()` check up one level to the `for conthost, contdevice in updates` loop and just pass in `attempt_sync\u003dTrue|False` but at that point it seems weird to pass in the `contdevice \u003d None` at all or even to wrap it in spawn 😭\n\n\u003e What if the proxy just skipped sending container-server host/device/partition info when it knows the DB\u0027s sharded but can\u0027t get shards\n\nI think that Tim is right: there\u0027s multiple ways to achive this behavior - we could put the proxy in control or have object server make it\u0027s own decisions.  But \"doesn\u0027t require object-server changes\" isn\u0027t obviously virtous if if means \"we re-use this *other* thing to MEAN the db is sharded and I couldn\u0027t get container_path so don\u0027t do a sync update\" instead of just passing along what we know.\n\n\u003e Do we need to make any object-server changes at all to get the behavior change we want?\n\nI *think* now that we have db_state (which we eventually want for the benifit of the updater) it\u0027s easier to make this determination (and test/assert the desired behavior!) in the object-server; but MAYBE we could also achive this in `_get_update_target` in the proxy; but we\u0027d still want a strong test saying \"when you send a PUT for an object in a sharded database that fails get shards it goes directly to async\" - if that behavior is split between the proxy pulling a string in the object server AND the object-server doing the right thing with that implicit string you\u0027d have to assert the behavior across multiple layers (more complicated test of the interaction between two components).  If the desired behavior is purely an object-server behavior we just have to test the object server - which this change does.  I will admit that \"proxy doesn\u0027t send contpath\" is a *kind of* implicit way of saying \"I couldn\u0027t get the shard update path\" but it feels a little more obviously to the point (given the code/comments in `container_update`) than \"proxy didn\u0027t send `cont[host|devices]` so we CAN\u0027T do the update, and hopefully that\u0027s a good thing.\" - but that could just be me because I\u0027ve recently become more familiar with that proxy code path.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"baad32ce2f1f22f3178c7ba1649fd7d88038ca0d","unresolved":true,"context_lines":[{"line_number":436,"context_line":"            # mid-upgrade to shard-aware proxy?  this should be rare; we will"},{"line_number":437,"context_line":"            # make an attempt to get a 301 out of the root before we write down"},{"line_number":438,"context_line":"            # async_pending"},{"line_number":439,"context_line":"            attempt_sync_update \u003d True"},{"line_number":440,"context_line":"        elif contdbstate \u003d\u003d SHARDED:"},{"line_number":441,"context_line":"            # if db_state is sharded but we didn\u0027t get the shard to pass it to"},{"line_number":442,"context_line":"            # must mean we failed to get this info from the proxy.  This is"}],"source_content_type":"text/x-python","patch_set":7,"id":"f3b95722_f50c76f9","line":439,"updated":"2024-06-03 16:41:53.000000000","message":"move the two places of ``attempt_sync_update \u003d True`` to line 421?","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"33ec8ca0b6c21d8e5a674cbf3e5e043c80c1d712","unresolved":false,"context_lines":[{"line_number":436,"context_line":"            # mid-upgrade to shard-aware proxy?  this should be rare; we will"},{"line_number":437,"context_line":"            # make an attempt to get a 301 out of the root before we write down"},{"line_number":438,"context_line":"            # async_pending"},{"line_number":439,"context_line":"            attempt_sync_update \u003d True"},{"line_number":440,"context_line":"        elif contdbstate \u003d\u003d SHARDED:"},{"line_number":441,"context_line":"            # if db_state is sharded but we didn\u0027t get the shard to pass it to"},{"line_number":442,"context_line":"            # must mean we failed to get this info from the proxy.  This is"}],"source_content_type":"text/x-python","patch_set":7,"id":"457e2236_35bb4001","line":439,"in_reply_to":"20427a9b_e575e970","updated":"2024-07-11 03:29:09.000000000","message":"yup your both right here. Needed less setting of things. Tim, I like your approach as it\u0027s less lines and neatly defines what we\u0027re calling \"skipable\", so will go with that 😊","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f3fd2584fe984e3d555f61f974b3a8879c11fced","unresolved":true,"context_lines":[{"line_number":436,"context_line":"            # mid-upgrade to shard-aware proxy?  this should be rare; we will"},{"line_number":437,"context_line":"            # make an attempt to get a 301 out of the root before we write down"},{"line_number":438,"context_line":"            # async_pending"},{"line_number":439,"context_line":"            attempt_sync_update \u003d True"},{"line_number":440,"context_line":"        elif contdbstate \u003d\u003d SHARDED:"},{"line_number":441,"context_line":"            # if db_state is sharded but we didn\u0027t get the shard to pass it to"},{"line_number":442,"context_line":"            # must mean we failed to get this info from the proxy.  This is"}],"source_content_type":"text/x-python","patch_set":7,"id":"20427a9b_e575e970","line":439,"in_reply_to":"f3b95722_f50c76f9","updated":"2024-06-21 22:58:13.000000000","message":"Or have L421 like\n```\n# If db_state is sharded but we didn\u0027t get the shard to pass it to\n# must mean we failed to get this info from the proxy.  This is\n# usually caused by the container servers being overloaded.  We\n# don\u0027t want to add to the problem so skip trying to query the\n# root synchronously and write directly to async_pending\nskip_sync_update \u003d (contdbstate \u003d\u003d SHARDED and not contpath)\n```\nand say `attempt_sync_update\u003dnot skip_sync_update` below.","commit_id":"da8ca404d552aa9d53090d8209a57fa820eda17a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":358,"context_line":"                {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice,"},{"line_number":359,"context_line":"                 \u0027error_type\u0027: status_or_error})"},{"line_number":360,"context_line":"        finally:"},{"line_number":361,"context_line":"            # self.stats.increment(\u0027sync_update\u0027, **sync_update_ctx)"},{"line_number":362,"context_line":"            if skip:"},{"line_number":363,"context_line":"                legacy_ctx \u003d \u0027skip\u0027"},{"line_number":364,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ecdf80da_41449691","line":361,"updated":"2024-07-15 20:32:00.000000000","message":"probably not relevant w/o 909882: stats: API for native labeled metrics | https://review.opendev.org/c/openstack/swift/+/909882","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":299,"context_line":"            ``container`` params."},{"line_number":300,"context_line":"        :param db_state: The current database state of the container as"},{"line_number":301,"context_line":"            supplied to us by the proxy."},{"line_number":302,"context_line":"        :param attempt_sync_update: boolean, when false all spawned threads"},{"line_number":303,"context_line":"                                    write the update data directly to async"},{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        if logger_thread_locals:"}],"source_content_type":"text/x-python","patch_set":10,"id":"f7e9e988_d886f8fb","line":302,"range":{"start_line":302,"start_character":56,"end_line":302,"end_character":75},"updated":"2024-07-22 22:41:50.000000000","message":"I wonder a little if `container_update` should get something like\n```\nif skip_sync_update:\n    updates \u003d updates[:1]\n```\nso we just write the one async...","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":315,"context_line":"        success \u003d False"},{"line_number":316,"context_line":"        skip \u003d not attempt_sync_update or \\"},{"line_number":317,"context_line":"            not all([host, partition, contdevice])"},{"line_number":318,"context_line":"        try:"},{"line_number":319,"context_line":"            if not skip:"},{"line_number":320,"context_line":"                # Do an sync update"},{"line_number":321,"context_line":"                with ConnectionTimeout(self.conn_timeout):"}],"source_content_type":"text/x-python","patch_set":10,"id":"d9cc7aa0_ef4900c5","line":318,"updated":"2024-08-05 12:03:06.000000000","message":"it struck me as odd to have the ``if``, with no ``else`` inside the ``try``, but I guess it\u0027s about using the finally to emit the metric?\n\nI tried a different way of writing it, maybe clearer, IDK?\n\n```\ndiff --git a/swift/obj/server.py b/swift/obj/server.py\nindex cf89bd4ed..20869867e 100644\n--- a/swift/obj/server.py\n+++ b/swift/obj/server.py\n@@ -315,8 +315,10 @@ class ObjectController(BaseStorageServer):\n         success \u003d False\n         skip \u003d not attempt_sync_update or \\\n             not all([host, partition, contdevice])\n-        try:\n-            if not skip:\n+        if skip:\n+            self.logger.increment(\u0027sync_update.skip\u0027)\n+        else:\n+            try:\n                 # Do an sync update\n                 with ConnectionTimeout(self.conn_timeout):\n                     ip, port \u003d host.rsplit(\u0027:\u0027, 1)\n@@ -347,27 +349,24 @@ class ObjectController(BaseStorageServer):\n                         {\u0027status\u0027: response.status, \u0027ip\u0027: ip, \u0027port\u0027: port,\n                          \u0027dev\u0027: contdevice})\n\n-        except (Exception, Timeout) as e:\n-            if isinstance(e, Timeout):\n-                status_or_error \u003d \u0027timeout\u0027\n-            else:\n-                status_or_error \u003d \u0027exception\u0027\n-            self.logger.exception(\n-                \u0027ERROR container update failed (%(error_type)s) with \u0027\n-                \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,\n-                {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice,\n-                 \u0027error_type\u0027: status_or_error})\n-        finally:\n-            # self.stats.increment(\u0027sync_update\u0027, **sync_update_ctx)\n-            if skip:\n-                legacy_ctx \u003d \u0027skip\u0027\n-            else:\n-                legacy_ctx \u003d \u0027success\u0027 if success else \u0027error\u0027\n-                legacy_ctx +\u003d \u0027.%s\u0027 % (status_or_error)\n-            legacy_metric_name \u003d \u0027sync_update.%s\u0027 % legacy_ctx\n-            self.logger.increment(legacy_metric_name)\n+            except (Exception, Timeout) as e:\n+                if isinstance(e, Timeout):\n+                    status_or_error \u003d \u0027timeout\u0027\n+                else:\n+                    status_or_error \u003d \u0027exception\u0027\n+                self.logger.exception(\n+                    \u0027ERROR container update failed (%(error_type)s) with \u0027\n+                    \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,\n+                    {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice,\n+                     \u0027error_type\u0027: status_or_error})\n+            finally:\n+                legacy_metric_name \u003d \u0027sync_update.%s.%s\u0027 % (\n+                    \u0027success\u0027 if success else \u0027error\u0027, status_or_error)\n+                self.logger.increment(legacy_metric_name)\n+\n         data \u003d {\u0027op\u0027: op, \u0027account\u0027: account, \u0027container\u0027: container,\n                 \u0027obj\u0027: obj, \u0027headers\u0027: headers_out, \u0027db_state\u0027: db_state}\n+\n         if redirect_data:\n             self.logger.debug(\n                 \u0027Update to %(path)s redirected to %(redirect)s\u0027,\n```","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":317,"context_line":"            not all([host, partition, contdevice])"},{"line_number":318,"context_line":"        try:"},{"line_number":319,"context_line":"            if not skip:"},{"line_number":320,"context_line":"                # Do an sync update"},{"line_number":321,"context_line":"                with ConnectionTimeout(self.conn_timeout):"},{"line_number":322,"context_line":"                    ip, port \u003d host.rsplit(\u0027:\u0027, 1)"},{"line_number":323,"context_line":"                    conn \u003d http_connect(ip, port, contdevice, partition, op,"}],"source_content_type":"text/x-python","patch_set":10,"id":"e30b48d6_74a85d3c","line":320,"range":{"start_line":320,"start_character":21,"end_line":320,"end_character":23},"updated":"2024-07-22 22:41:50.000000000","message":"nit: \"a\"","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":326,"context_line":"                    response \u003d conn.getresponse()"},{"line_number":327,"context_line":"                    response.read()"},{"line_number":328,"context_line":"                status_or_error \u003d response.status"},{"line_number":329,"context_line":"                if is_success(response.status):"},{"line_number":330,"context_line":"                    success \u003d True"},{"line_number":331,"context_line":"                    return"},{"line_number":332,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7f0d4b08_1443132e","line":329,"range":{"start_line":329,"start_character":30,"end_line":329,"end_character":45},"updated":"2024-07-22 22:41:50.000000000","message":"Worth using `status_or_error`, now that we\u0027ve pulled it out? 🤷","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":353,"context_line":"            else:"},{"line_number":354,"context_line":"                status_or_error \u003d \u0027exception\u0027"},{"line_number":355,"context_line":"            self.logger.exception("},{"line_number":356,"context_line":"                \u0027ERROR container update failed (%(error_type)s) with \u0027"},{"line_number":357,"context_line":"                \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,"},{"line_number":358,"context_line":"                {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice,"},{"line_number":359,"context_line":"                 \u0027error_type\u0027: status_or_error})"}],"source_content_type":"text/x-python","patch_set":10,"id":"1fcca346_f1f50207","line":356,"range":{"start_line":356,"start_character":47,"end_line":356,"end_character":63},"updated":"2024-07-22 22:41:50.000000000","message":"Nice!","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":353,"context_line":"            else:"},{"line_number":354,"context_line":"                status_or_error \u003d \u0027exception\u0027"},{"line_number":355,"context_line":"            self.logger.exception("},{"line_number":356,"context_line":"                \u0027ERROR container update failed (%(error_type)s) with \u0027"},{"line_number":357,"context_line":"                \u0027%(ip)s:%(port)s/%(dev)s (saving for async update later)\u0027,"},{"line_number":358,"context_line":"                {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice,"},{"line_number":359,"context_line":"                 \u0027error_type\u0027: status_or_error})"}],"source_content_type":"text/x-python","patch_set":10,"id":"cc059f6b_ba13548a","line":356,"range":{"start_line":356,"start_character":47,"end_line":356,"end_character":63},"in_reply_to":"1fcca346_f1f50207","updated":"2024-08-05 12:03:06.000000000","message":"not tested (nor on master)","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":358,"context_line":"                {\u0027ip\u0027: ip, \u0027port\u0027: port, \u0027dev\u0027: contdevice,"},{"line_number":359,"context_line":"                 \u0027error_type\u0027: status_or_error})"},{"line_number":360,"context_line":"        finally:"},{"line_number":361,"context_line":"            # self.stats.increment(\u0027sync_update\u0027, **sync_update_ctx)"},{"line_number":362,"context_line":"            if skip:"},{"line_number":363,"context_line":"                legacy_ctx \u003d \u0027skip\u0027"},{"line_number":364,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"301e8bfd_1404d08b","line":361,"updated":"2024-07-22 22:41:50.000000000","message":"Probably ought to drop until we\u0027ve got labeled metrics 😉","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af5c47d5bfe12031ac4bcf591248a15396c4e755","unresolved":true,"context_lines":[{"line_number":424,"context_line":"        # must mean we failed to get this info from the proxy.  This is"},{"line_number":425,"context_line":"        # usually caused by the container servers being overloaded.  We"},{"line_number":426,"context_line":"        # don\u0027t want to add to the problem so skip trying to query the"},{"line_number":427,"context_line":"        # root synchronously and write directly to async_pending"},{"line_number":428,"context_line":"        skip_sync_update \u003d (contdbstate \u003d\u003d SHARDED and not contpath)"},{"line_number":429,"context_line":"        if contpath:"},{"line_number":430,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"178ca21c_27961aa2","line":427,"updated":"2024-07-22 22:41:50.000000000","message":"\"... must mean ...\", \"This is usually...\" -- it feels like we\u0027re making assumptions and relying on things unspoken. What about a more explicit flag, like `x-backend-update-mode: sync|async|none`?","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":424,"context_line":"        # must mean we failed to get this info from the proxy.  This is"},{"line_number":425,"context_line":"        # usually caused by the container servers being overloaded.  We"},{"line_number":426,"context_line":"        # don\u0027t want to add to the problem so skip trying to query the"},{"line_number":427,"context_line":"        # root synchronously and write directly to async_pending"},{"line_number":428,"context_line":"        skip_sync_update \u003d (contdbstate \u003d\u003d SHARDED and not contpath)"},{"line_number":429,"context_line":"        if contpath:"},{"line_number":430,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"f7a5acef_43d00fa1","line":427,"in_reply_to":"178ca21c_27961aa2","updated":"2024-08-05 12:03:06.000000000","message":"I think I\u0027d prefer the proxy to be directive rather than the object server learning about sharding states etc i.e send an explicit flag.\n\nI know I can be inconsistent on these calls - sometimes I\u0027ll argue for pushing state around and making decisions late, but here it feels like the proxy is the \"smart director\" and the object server is the \"dumb actor\" in that the proxy already provides all the info required for the object server to attempt an update (hosts, partition etc). Except container_path which (I had to remind myself) is only passed by proxy when the container is sharded ... but that doesn\u0027t seem implicit from the header name, I mean, is is set in stone that the proxy will never send the container path for an unsharded  container?\n\n(The object-updater is of course by necessity smarter than the object-server).\n\nI _think_ that if I was writing this from scratch (i.e. no upgrade paths to worry about) I\u0027d think in terms of the proxy _instructing_ the object server to send a sync update (or not) and passing all the params required for the object server to do that, rather than passing a bunch of params to the object server and leaving it to figure out what it should do.","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":459,"context_line":"                       objdevice, policy,"},{"line_number":460,"context_line":"                       logger_thread_locals\u003dself.logger.thread_locals,"},{"line_number":461,"context_line":"                       container_path\u003dcontpath, db_state\u003dcontdbstate,"},{"line_number":462,"context_line":"                       attempt_sync_update\u003dnot skip_sync_update)"},{"line_number":463,"context_line":"            update_greenthreads.append(gt)"},{"line_number":464,"context_line":"        # Wait a little bit to see if the container updates are successful."},{"line_number":465,"context_line":"        # If we immediately return after firing off the greenthread above, then"}],"source_content_type":"text/x-python","patch_set":10,"id":"53210010_2ce3c2eb","line":462,"updated":"2024-08-05 12:03:06.000000000","message":"here we go from ``skip...`` to ``attempt...`` but then ``async_update`` tests ``not attempt_sync_update`` - can we just pass ``skip_sync_update`` and avoid flipping the sense of the flag?","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b0e874aadaa32fd5969899f61061b80a04a8942b","unresolved":true,"context_lines":[{"line_number":6185,"context_line":"            \u0027X-Backend-Container-Update-Override-Content-Type\u0027: \u0027ignored\u0027,"},{"line_number":6186,"context_line":"            \u0027X-Backend-Container-Update-Override-Foo\u0027: \u0027ignored\u0027})"},{"line_number":6187,"context_line":""},{"line_number":6188,"context_line":"    def test_PUT_container_update_to_old_style_shard(self):"},{"line_number":6189,"context_line":"        # verify that alternate container update path is respected when"},{"line_number":6190,"context_line":"        # included in request headers"},{"line_number":6191,"context_line":"        def do_test(container_path, expected_path, expected_container_path,"}],"source_content_type":"text/x-python","patch_set":2,"id":"b5f247ae_a1dfcf30","line":6188,"range":{"start_line":6188,"start_character":8,"end_line":6188,"end_character":52},"updated":"2024-05-15 07:34:24.000000000","message":"I also don\u0027t like how repeditive these 2 tests are. But also maybe doing too much in a single test.","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":6185,"context_line":"            \u0027X-Backend-Container-Update-Override-Content-Type\u0027: \u0027ignored\u0027,"},{"line_number":6186,"context_line":"            \u0027X-Backend-Container-Update-Override-Foo\u0027: \u0027ignored\u0027})"},{"line_number":6187,"context_line":""},{"line_number":6188,"context_line":"    def test_PUT_container_update_to_old_style_shard(self):"},{"line_number":6189,"context_line":"        # verify that alternate container update path is respected when"},{"line_number":6190,"context_line":"        # included in request headers"},{"line_number":6191,"context_line":"        def do_test(container_path, expected_path, expected_container_path,"}],"source_content_type":"text/x-python","patch_set":2,"id":"af2d8e12_c50cd2e7","line":6188,"range":{"start_line":6188,"start_character":8,"end_line":6188,"end_character":52},"in_reply_to":"b5f247ae_a1dfcf30","updated":"2024-07-15 20:32:00.000000000","message":"yes, good judgement to extract re-usable test infra KUDOS","commit_id":"d9e0703829fea641685e349ad7bf6fd1128ed947"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"359b0ec83698fd17fbf4cb000c87e0d34bb2782b","unresolved":true,"context_lines":[{"line_number":6349,"context_line":"            req \u003d Request.blank(\u0027/sda1/0/a/c/o\u0027, method\u003d\u0027PUT\u0027,"},{"line_number":6350,"context_line":"                                headers\u003dheaders, body\u003d\u0027\u0027)"},{"line_number":6351,"context_line":"            self.logger.clear()"},{"line_number":6352,"context_line":"            if expect_skip_to_async:"},{"line_number":6353,"context_line":"                with fake_spawn():"},{"line_number":6354,"context_line":"                    resp \u003d req.get_response(self.object_controller)"},{"line_number":6355,"context_line":"                self.assertEqual("}],"source_content_type":"text/x-python","patch_set":3,"id":"20de3286_38a539e6","line":6352,"updated":"2024-05-20 19:46:26.000000000","message":"the do_test(expect_thing\u003dTrue|False) pattern is NOT my favorite.\n\ni prefer we re-organize as:\n\n```\ndef test_specific_desirable_behavior(self):\n    self._turn_crank_consistently_with_some_varience(**kwargs)\n    self.assertSpecificThingsRelevantToBehavior()\n```","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":6349,"context_line":"            req \u003d Request.blank(\u0027/sda1/0/a/c/o\u0027, method\u003d\u0027PUT\u0027,"},{"line_number":6350,"context_line":"                                headers\u003dheaders, body\u003d\u0027\u0027)"},{"line_number":6351,"context_line":"            self.logger.clear()"},{"line_number":6352,"context_line":"            if expect_skip_to_async:"},{"line_number":6353,"context_line":"                with fake_spawn():"},{"line_number":6354,"context_line":"                    resp \u003d req.get_response(self.object_controller)"},{"line_number":6355,"context_line":"                self.assertEqual("}],"source_content_type":"text/x-python","patch_set":3,"id":"0b814940_3979dc53","line":6352,"in_reply_to":"20de3286_38a539e6","updated":"2024-07-15 20:32:00.000000000","message":"heh, I\u0027m not beyond using this pattern myself tho it turns out.  I just wrote:\n\n```\n+        for bad_redirect_headers, expected_error_logs in \\\n+                example_bad_headers_and_error_logs:\n+            do_test(bad_redirect_headers, expected_error_logs)\n```","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"359b0ec83698fd17fbf4cb000c87e0d34bb2782b","unresolved":true,"context_lines":[{"line_number":6416,"context_line":"        # defaulting to root path?"},{"line_number":6417,"context_line":"        # Also it seems when we fail to split these paths we put in a"},{"line_number":6418,"context_line":"        # container_path of None, which triggers the skip code.. might"},{"line_number":6419,"context_line":"        # need to think on this some more."},{"line_number":6420,"context_line":"        do_test(\u0027garbage\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6421,"context_line":"        do_test(\u0027/\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6422,"context_line":"        do_test(\u0027/no-acct\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e4f76912_c5d08541","line":6419,"updated":"2024-05-20 19:46:26.000000000","message":"wat?  oh, the TODO\n\nhttps://github.com/NVIDIA/swift/blob/master/swift/obj/server.py#L389-L402\n\nso the \"send to the root\" isn\u0027t new - but the \"go directly to async\" instead of \"forward to root and see what happens\" when we encounter \"bad\" contpath is different.\n\nIf we wanted to change it we could remove the db_state, but I think it\u0027s fine for now.\n\nHonestly we should probably lift more of the \"skip_async_update\" logic out of the `async_udpate` into `container_update` along with the \"late parsing\" of the X-Backend[-Quoted]-Container-Path header - we don\u0027t need each conthost, conddevice to individually decide if they should skip or not.  And apparently on skip all 3 will call `pickle_async_update` ?!","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":6416,"context_line":"        # defaulting to root path?"},{"line_number":6417,"context_line":"        # Also it seems when we fail to split these paths we put in a"},{"line_number":6418,"context_line":"        # container_path of None, which triggers the skip code.. might"},{"line_number":6419,"context_line":"        # need to think on this some more."},{"line_number":6420,"context_line":"        do_test(\u0027garbage\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6421,"context_line":"        do_test(\u0027/\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6422,"context_line":"        do_test(\u0027/no-acct\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"711e7a4b_00d9a774","line":6419,"in_reply_to":"e4f76912_c5d08541","updated":"2024-07-15 20:32:00.000000000","message":"ok, in the latest version of this patch there\u0027s no change to this behavior WRT to master.","commit_id":"67f7400931a0e1804cb4663a0f7aaed8dfae1e9c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6244,"context_line":"                \u0027X-Container-Host\u0027: \u0027chost:cport\u0027,"},{"line_number":6245,"context_line":"                \u0027X-Container-Partition\u0027: \u0027cpartition\u0027,"},{"line_number":6246,"context_line":"                \u0027X-Container-Device\u0027: \u0027cdevice\u0027,"},{"line_number":6247,"context_line":"                \u0027X-Container-Root-Db-State\u0027: \u0027unsharded\u0027,"},{"line_number":6248,"context_line":"                \u0027Content-Type\u0027: \u0027text/plain\u0027,"},{"line_number":6249,"context_line":"                \u0027X-Object-Sysmeta-Ec-Frag-Index\u0027: 0,"},{"line_number":6250,"context_line":"                \u0027X-Backend-Storage-Policy-Index\u0027: int(policy),"}],"source_content_type":"text/x-python","patch_set":9,"id":"6cac4d1c_2b4e5004","line":6247,"updated":"2024-07-15 20:32:00.000000000","message":"wait, so we can sometimes \"expect_skip_to_async\" even when the Db-State is unsharded?!","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6251,"context_line":"            }"},{"line_number":6252,"context_line":"            if container_path is not None:"},{"line_number":6253,"context_line":"                headers[container_path_key] \u003d container_path"},{"line_number":6254,"context_line":"                headers[\u0027X-Container-Root-Db-State\u0027] \u003d \u0027sharded\u0027"},{"line_number":6255,"context_line":""},{"line_number":6256,"context_line":"            req \u003d Request.blank(\u0027/sda1/0/a/c/o\u0027, method\u003d\u0027PUT\u0027,"},{"line_number":6257,"context_line":"                                headers\u003dheaders, body\u003d\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"b845399a_605328cc","line":6254,"updated":"2024-07-15 20:32:00.000000000","message":"oh... we set Db-State to sharded - it\u0027s tied up with the caller sending container_path","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":6251,"context_line":"            }"},{"line_number":6252,"context_line":"            if container_path is not None:"},{"line_number":6253,"context_line":"                headers[container_path_key] \u003d container_path"},{"line_number":6254,"context_line":"                headers[\u0027X-Container-Root-Db-State\u0027] \u003d \u0027sharded\u0027"},{"line_number":6255,"context_line":""},{"line_number":6256,"context_line":"            req \u003d Request.blank(\u0027/sda1/0/a/c/o\u0027, method\u003d\u0027PUT\u0027,"},{"line_number":6257,"context_line":"                                headers\u003dheaders, body\u003d\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba8918d4_1fc1a6bd","line":6254,"in_reply_to":"b845399a_605328cc","updated":"2024-08-05 12:03:06.000000000","message":"this doesn\u0027t match real world when proxy just doesn\u0027t send the header. We should add db_state as a parameter to the helper and set these two headers based on independent conditions.","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6313,"context_line":"                expected_data[\u0027db_state\u0027] \u003d \u0027sharded\u0027"},{"line_number":6314,"context_line":"            self.assertEqual(expected_data, data)"},{"line_number":6315,"context_line":""},{"line_number":6316,"context_line":"        do_test(\u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027)"},{"line_number":6317,"context_line":"        do_test(\u0027\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6318,"context_line":"        do_test(None, \u0027a/c\u0027, None)"},{"line_number":6319,"context_line":"        # TODO: should these cases trigger a 400 response rather than"}],"source_content_type":"text/x-python","patch_set":9,"id":"f938d2bd_afa193aa","line":6316,"updated":"2024-07-15 20:32:00.000000000","message":"this is `expect_skip_to_async\u003dFalse`","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6314,"context_line":"            self.assertEqual(expected_data, data)"},{"line_number":6315,"context_line":""},{"line_number":6316,"context_line":"        do_test(\u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027)"},{"line_number":6317,"context_line":"        do_test(\u0027\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6318,"context_line":"        do_test(None, \u0027a/c\u0027, None)"},{"line_number":6319,"context_line":"        # TODO: should these cases trigger a 400 response rather than"},{"line_number":6320,"context_line":"        # defaulting to root path?"}],"source_content_type":"text/x-python","patch_set":9,"id":"f65d81c0_5eba4c93","line":6317,"updated":"2024-07-15 20:32:00.000000000","message":"wouldn\u0027t the proxy actually just not send the contpath header rather than an *empty* contpath header!?","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":6314,"context_line":"            self.assertEqual(expected_data, data)"},{"line_number":6315,"context_line":""},{"line_number":6316,"context_line":"        do_test(\u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027)"},{"line_number":6317,"context_line":"        do_test(\u0027\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6318,"context_line":"        do_test(None, \u0027a/c\u0027, None)"},{"line_number":6319,"context_line":"        # TODO: should these cases trigger a 400 response rather than"},{"line_number":6320,"context_line":"        # defaulting to root path?"}],"source_content_type":"text/x-python","patch_set":9,"id":"6d085782_6725cf8c","line":6317,"in_reply_to":"f65d81c0_5eba4c93","updated":"2024-08-05 12:03:06.000000000","message":"correct, asserted here\n\nhttps://github.com/openstack/swift/blob/e8affa7db5817e50cd82130a8f4c250725b80884/test/unit/proxy/test_server.py#L5029","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6315,"context_line":""},{"line_number":6316,"context_line":"        do_test(\u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027, \u0027a_shard/c_shard\u0027)"},{"line_number":6317,"context_line":"        do_test(\u0027\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6318,"context_line":"        do_test(None, \u0027a/c\u0027, None)"},{"line_number":6319,"context_line":"        # TODO: should these cases trigger a 400 response rather than"},{"line_number":6320,"context_line":"        # defaulting to root path?"},{"line_number":6321,"context_line":"        do_test(\u0027garbage\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":9,"id":"8ae84de7_d8479d02","line":6318,"updated":"2024-07-15 20:32:00.000000000","message":"this is also `expect_skip_to_async\u003dFalse`","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6317,"context_line":"        do_test(\u0027\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dTrue)"},{"line_number":6318,"context_line":"        do_test(None, \u0027a/c\u0027, None)"},{"line_number":6319,"context_line":"        # TODO: should these cases trigger a 400 response rather than"},{"line_number":6320,"context_line":"        # defaulting to root path?"},{"line_number":6321,"context_line":"        do_test(\u0027garbage\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"},{"line_number":6322,"context_line":"        do_test(\u0027/\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"},{"line_number":6323,"context_line":"        do_test(\u0027/no-acct\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7eeea16c_932384ba","line":6320,"updated":"2024-07-15 20:32:00.000000000","message":"is this still \"TODO\" or have we decided \"invalid contpath \u003d\u003e let root sort it out\"","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":true,"context_lines":[{"line_number":6323,"context_line":"        do_test(\u0027/no-acct\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"},{"line_number":6324,"context_line":"        do_test(\u0027no-cont/\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"},{"line_number":6325,"context_line":"        do_test(\u0027too/many/parts\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"},{"line_number":6326,"context_line":"        do_test(\u0027/leading/slash\u0027, \u0027a/c\u0027, None, expect_skip_to_async\u003dFalse)"},{"line_number":6327,"context_line":""},{"line_number":6328,"context_line":"    def test_PUT_container_update_to_old_style_shard(self):"},{"line_number":6329,"context_line":"        # verify that old style container update path is respected when"}],"source_content_type":"text/x-python","patch_set":9,"id":"ec384c76_0456ad69","line":6326,"updated":"2024-07-15 20:32:00.000000000","message":"yeah, it doesn\u0027t look like there\u0027s any change to the current behavior on the most recent patch.","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":6333,"context_line":"    def test_PUT_container_update_to_shard(self):"},{"line_number":6334,"context_line":"        # verify that alternate container update path is respected when"},{"line_number":6335,"context_line":"        # included in request headers"},{"line_number":6336,"context_line":"        self._put_container_update_to_shard(\u0027X-Backend-Quoted-Container-Path\u0027)"},{"line_number":6337,"context_line":""},{"line_number":6338,"context_line":"    def test_container_update_async(self):"},{"line_number":6339,"context_line":"        policy \u003d random.choice(list(POLICIES))"}],"source_content_type":"text/x-python","patch_set":9,"id":"eb08d858_2244d050","line":6336,"updated":"2024-07-15 20:32:00.000000000","message":"this is probably a resonable use of DRY in tests.","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a35feb473a15682749caab9587ee7ef0255ff14c","unresolved":false,"context_lines":[{"line_number":7905,"context_line":"            \u0027account\u0027: \u0027.expiring_objects\u0027,"},{"line_number":7906,"context_line":"            \u0027container\u0027: delete_at_container,"},{"line_number":7907,"context_line":"            \u0027obj\u0027: \u0027%s-a/c/o\u0027 % put_delete_at,"},{"line_number":7908,"context_line":"            \u0027db_state\u0027: None,"},{"line_number":7909,"context_line":"            \u0027headers\u0027: {"},{"line_number":7910,"context_line":"                \u0027X-Backend-Storage-Policy-Index\u0027: \u00270\u0027,"},{"line_number":7911,"context_line":"                # only POST-1 has to clear the orig PUT delete-at"}],"source_content_type":"text/x-python","patch_set":9,"id":"31b820ec_b4c16b20","line":7908,"updated":"2024-07-15 20:32:00.000000000","message":"this is None because this is a .expiring_objects update - I think this actually got squashed into the parent.","commit_id":"d32da227e3b5751f803826e390b21b993394db53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":2668,"context_line":"        self.assertEqual(resp.status_int, 201)"},{"line_number":2669,"context_line":"        self.assertEqual(self.logger.statsd_client.get_increment_counts(),"},{"line_number":2670,"context_line":"                         {\u0027sync_update.error.exception\u0027: 1,"},{"line_number":2671,"context_line":"                          \u0027async_pendings\u0027: 1})"},{"line_number":2672,"context_line":""},{"line_number":2673,"context_line":"    def test_EC_PUT_GET_data(self):"},{"line_number":2674,"context_line":"        for policy in self.ec_policies:"}],"source_content_type":"text/x-python","patch_set":10,"id":"4324cd83_2aba2919","line":2671,"updated":"2024-08-05 12:03:06.000000000","message":"we could add a case with a timeout here","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4f5019ead56d8283c17457456e34f32b7251eaf4","unresolved":true,"context_lines":[{"line_number":6221,"context_line":""},{"line_number":6222,"context_line":"    def _put_container_update_to_shard(self, container_path_key):"},{"line_number":6223,"context_line":"        def do_test(container_path, expected_path, expected_container_path,"},{"line_number":6224,"context_line":"                    expect_skip_to_async\u003dFalse):"},{"line_number":6225,"context_line":"            policy \u003d random.choice(list(POLICIES))"},{"line_number":6226,"context_line":"            container_updates \u003d []"},{"line_number":6227,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"f7f6d65c_efee9273","line":6224,"range":{"start_line":6224,"start_character":27,"end_line":6224,"end_character":40},"updated":"2024-08-05 12:03:06.000000000","message":"now we have 3 ways of saying it :D ``skip_sync_update``, ``attempt_sync_update`` and ``skip_to_async``\n\ncan we call this ``expect_skip_sync_update``? ...","commit_id":"14696fdb2954e2a37bda4773645655c62e75201d"}]}