)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29d30db8b54ea2d8abbb609a60abadc61a309764","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c1524882_429dc390","updated":"2024-03-15 16:38:47.000000000","message":"This is an attempt at an alternative to https://review.opendev.org/c/openstack/swift/+/908969\n\nI haven\u0027t written any tests :( but at this point I\u0027d like to see how we feel about the relative complexity of the two directions.","commit_id":"db4b830044aa7980a889af58bbb8cb03e1b55026"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"429f632a720fe4a4ac2a28e2c3b8186407f84357","unresolved":true,"context_lines":[{"line_number":904,"context_line":"        # \u0027unicode\u0027 with python2, here we cast \u0027unicode\u0027 back to \u0027str\u0027"},{"line_number":905,"context_line":"        return [[lower.encode(\u0027utf-8\u0027), name.encode(\u0027utf-8\u0027)]"},{"line_number":906,"context_line":"                for lower, name in bounds]"},{"line_number":907,"context_line":"    return bounds"},{"line_number":908,"context_line":""},{"line_number":909,"context_line":""},{"line_number":910,"context_line":"def get_namespaces_from_cache(req, cache_key, skip_chance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"443f0b1a_547fa8e9","line":907,"updated":"2024-03-15 16:46:03.000000000","message":"this change is probably unrelated - I picked it up from basing this patch on  https://review.opendev.org/c/openstack/swift/+/908969","commit_id":"db4b830044aa7980a889af58bbb8cb03e1b55026"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"429f632a720fe4a4ac2a28e2c3b8186407f84357","unresolved":true,"context_lines":[{"line_number":355,"context_line":"                        self.app.shard_ranges_cache_token_sleep_interval"},{"line_number":356,"context_line":"                ) as sleeper:"},{"line_number":357,"context_line":"                    yield sleeper, \u0027token_ok\u0027"},{"line_number":358,"context_line":"            except MemcacheConnectionError:"},{"line_number":359,"context_line":"                yield [], \u0027token_error\u0027"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def _get_update_shard(self, req, account, container, obj):"}],"source_content_type":"text/x-python","patch_set":1,"id":"436864d2_32501c79","line":358,"updated":"2024-03-15 16:46:03.000000000","message":"I considered just emitting the metric here and returning [] , but in the future the metric name will be specific to the caller context (updating vs listing) so I followed the pattern of other functions that return (result, state_string)","commit_id":"db4b830044aa7980a889af58bbb8cb03e1b55026"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"429f632a720fe4a4ac2a28e2c3b8186407f84357","unresolved":true,"context_lines":[{"line_number":389,"context_line":"            # set of updating shard ranges from backend"},{"line_number":390,"context_line":"            with self._get_cooperative_token("},{"line_number":391,"context_line":"                    req, cache_key) as (sleep_countdown, token_cache_state):"},{"line_number":392,"context_line":"                record_cache_op_metrics("},{"line_number":393,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":394,"context_line":"                    token_cache_state, None)"},{"line_number":395,"context_line":"                backend_get_state \u003d \u0027token_granted\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3cd9c838_d16a21ac","line":392,"updated":"2024-03-15 16:46:03.000000000","message":"we could maybe use a class method helper that provides an abbreviated version of this","commit_id":"db4b830044aa7980a889af58bbb8cb03e1b55026"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"429f632a720fe4a4ac2a28e2c3b8186407f84357","unresolved":true,"context_lines":[{"line_number":392,"context_line":"                record_cache_op_metrics("},{"line_number":393,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":394,"context_line":"                    token_cache_state, None)"},{"line_number":395,"context_line":"                backend_get_state \u003d \u0027token_granted\u0027"},{"line_number":396,"context_line":"                for _ in sleep_countdown:"},{"line_number":397,"context_line":"                    backend_get_state \u003d \u0027token_expired\u0027"},{"line_number":398,"context_line":"                    ns_bound_list, get_cache_state \u003d get_namespaces_from_cache("}],"source_content_type":"text/x-python","patch_set":1,"id":"8bf02c2f_7b7e7eb1","line":395,"updated":"2024-03-15 16:46:03.000000000","message":"the variety of metrics/states needs some more thought IMO, dropped this in as a placeholder","commit_id":"db4b830044aa7980a889af58bbb8cb03e1b55026"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"7d6ef2fe4fc827a3c681dd73e73afc63a9b6233c","unresolved":true,"context_lines":[{"line_number":393,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":394,"context_line":"                    token_cache_state, None)"},{"line_number":395,"context_line":"                backend_get_state \u003d \u0027token_granted\u0027"},{"line_number":396,"context_line":"                for _ in sleep_countdown:"},{"line_number":397,"context_line":"                    backend_get_state \u003d \u0027token_expired\u0027"},{"line_number":398,"context_line":"                    ns_bound_list, get_cache_state \u003d get_namespaces_from_cache("},{"line_number":399,"context_line":"                        req, cache_key, skip_chance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f581a1a6_26f6102b","line":396,"updated":"2024-03-18 05:23:57.000000000","message":"thanks for trying this. my first implementation was kind of like this(except without the new context manager, and it was much easier to code), and then Clay strongly suggested to make an abstract function or class to reuse all the important logic. He had a try himself using several abstract functions in this patch before: https://review.opendev.org/c/openstack/swift/+/892979. That was a very good suggestion, other than updating shard range cache, we could apply the cooperative token to listing shard range cache, the future SLO manifest cache and etc, all of those use cases shouldn\u0027t implement the same logic on their own path.\n\nI consider the logic to fetch token from memcached is the \"top half\", how to use the token and query the backend/fill in the cache cooperatively is the \"bottom half\", and both of them are important and part of the complete \"cooperative token\" solution which other usage cases can reuse. A reusable abstraction is hard but worth the effort.\n\nsince the new \"get_cooperative_token_or_sleep\" context manager function has received negative feedbacks and objection from Clay, \nhttps://review.opendev.org/c/openstack/swift/+/890174. I am rolling back to use the class approach, and more importantly working on the complete test suites to validate the approach with a preliminary interface.","commit_id":"db4b830044aa7980a889af58bbb8cb03e1b55026"}]}