)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bd846d32556e516c5995c5786d1cffeda1f6ae8c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b9ccc7f9_38a9785a","updated":"2021-12-08 13:12:20.000000000","message":"I had a go at inlining the unpickling https://review.opendev.org/c/openstack/swift/+/821020","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3afbc71414a670050496007cc91431aa29c7d75b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"210eaa86_1a6dafba","updated":"2021-12-07 23:42:04.000000000","message":"I like it -- feels similar to https://review.opendev.org/c/openstack/swift/+/819931, but updates the next-available time *before* yielding, so we don\u0027t have to do the weird\n\n max(0, elapsed_time - expected_time)\n\nthing. I think there\u0027s still some value in backing off *more* when the server\u0027s *more overloaded*, though.","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3a00a2de_7eea9a02","updated":"2021-12-08 22:33:30.000000000","message":"yeah this might be ok","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8f928f1fc1b60920d2f3b872c2347fca5f4a2232","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"73a793c2_4f0146e3","updated":"2021-12-17 14:34:28.000000000","message":"I\u0027m not sure how serious this is (since we can configure the # of buckets) - but it seems like this is to deterministic","commit_id":"d777c3aa9b7cb20377c463e4d4d6a6fa0596d5bc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6a75cc351707256139db8078bf50a6c46cd9f300","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b2b2add8_835e60f8","updated":"2021-12-11 05:34:29.000000000","message":"recheck","commit_id":"d777c3aa9b7cb20377c463e4d4d6a6fa0596d5bc"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3dd78760ae846354d65bd1e8bfbc2b31acea7a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b36c5c56_d848a525","updated":"2022-01-10 16:34:13.000000000","message":"I\u0027m satisfied here; IMHO we could merge or try to restack with some of the really nice stats handling from the child lifted up here.","commit_id":"de888629817a2a326b6e8dc66edb0ce3168818a7"}],"etc/object-server.conf-sample":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":473,"context_line":"# Send at most this many object updates per second"},{"line_number":474,"context_line":"# objects_per_second \u003d 50"},{"line_number":475,"context_line":"#"},{"line_number":476,"context_line":"# Send at most this many object updates per bucket per second."},{"line_number":477,"context_line":"# max_objects_per_container_per_second \u003d 0"},{"line_number":478,"context_line":"#"},{"line_number":479,"context_line":"# The per_container ratelimit implementation uses a hashring to constrain"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"428a558c_ddc91426","line":476,"updated":"2021-12-09 15:29:25.000000000","message":"Add a note that 0 means unlimited","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":474,"context_line":"# objects_per_second \u003d 50"},{"line_number":475,"context_line":"#"},{"line_number":476,"context_line":"# Send at most this many object updates per bucket per second."},{"line_number":477,"context_line":"# max_objects_per_container_per_second \u003d 0"},{"line_number":478,"context_line":"#"},{"line_number":479,"context_line":"# The per_container ratelimit implementation uses a hashring to constrain"},{"line_number":480,"context_line":"# memory requirements.  Orders of magnitude more buckets will use (nominally)"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"b96694be_43e94ad4","line":477,"range":{"start_line":477,"start_character":14,"end_line":477,"end_character":27},"updated":"2021-12-09 15:29:25.000000000","message":"should we use \"per_bucket\" in the option names to reflect that it may not be per-container?","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"}],"swift/obj/updater.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3afbc71414a670050496007cc91431aa29c7d75b","unresolved":true,"context_lines":[{"line_number":303,"context_line":"        with ContextPool(self.concurrency) as pool:"},{"line_number":304,"context_line":"            for update in ap_iter:"},{"line_number":305,"context_line":"                pool.spawn(self.process_object_update,"},{"line_number":306,"context_line":"                           update[\u0027path\u0027], update[\u0027device\u0027], update[\u0027policy\u0027])"},{"line_number":307,"context_line":"                now \u003d time.time()"},{"line_number":308,"context_line":"                if now - last_status_update \u003e\u003d self.report_interval:"},{"line_number":309,"context_line":"                    this_sweep \u003d self.stats.since(start_stats)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c070d8a6_68eee07e","side":"PARENT","line":306,"updated":"2021-12-07 23:42:04.000000000","message":"Wait, you said update came out None on quarantine? Was this KeyError\u0027ing before in that case?","commit_id":"1859f2e161f396023d8328489fb4b88c303797a9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"06cdc04be2f229f5338062c0473f7d3a01e38cfa","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        with ContextPool(self.concurrency) as pool:"},{"line_number":304,"context_line":"            for update in ap_iter:"},{"line_number":305,"context_line":"                pool.spawn(self.process_object_update,"},{"line_number":306,"context_line":"                           update[\u0027path\u0027], update[\u0027device\u0027], update[\u0027policy\u0027])"},{"line_number":307,"context_line":"                now \u003d time.time()"},{"line_number":308,"context_line":"                if now - last_status_update \u003e\u003d self.report_interval:"},{"line_number":309,"context_line":"                    this_sweep \u003d self.stats.since(start_stats)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8af0967e_20868d56","side":"PARENT","line":306,"in_reply_to":"c070d8a6_68eee07e","updated":"2021-12-08 00:00:45.000000000","message":"Oh! I see -- we didn\u0027t have _load_update before, and the quarantine was getting handled down in process_object_update.","commit_id":"1859f2e161f396023d8328489fb4b88c303797a9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9e3ecb9a05cc3bb36e2056b9fcbd7d8cff317593","unresolved":true,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _bucket_key(self, data):"},{"line_number":74,"context_line":"        hash_path \u003d \u0027%(account)s/%(container)s\u0027 % data"},{"line_number":75,"context_line":"        return int(md5(hash_path).hexdigest(), 16) % self.num_buckets"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    def next(self):"},{"line_number":78,"context_line":"        for update in self.iterator:"}],"source_content_type":"text/x-python","patch_set":1,"id":"232d39b2_e7013327","line":75,"range":{"start_line":75,"start_character":19,"end_line":75,"end_character":22},"updated":"2021-12-08 16:07:11.000000000","message":"This is the cause of the pep8 failure; it needs a usedforsecurity\u003dFalse -- or we use another hash function; this choice was pretty arbitrary.","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3afbc71414a670050496007cc91431aa29c7d75b","unresolved":true,"context_lines":[{"line_number":88,"context_line":"            now \u003d time.time()"},{"line_number":89,"context_line":"            if self.next_update[bucket_key] \u003e now:"},{"line_number":90,"context_line":"                continue"},{"line_number":91,"context_line":"            update[\u0027cached_pickle\u0027] \u003d data"},{"line_number":92,"context_line":"            self.next_update[bucket_key] \u003d now + self.bucket_update_delta"},{"line_number":93,"context_line":"            return update"},{"line_number":94,"context_line":"        raise StopIteration()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3162ff5d_7aab0bb8","line":91,"updated":"2021-12-07 23:42:04.000000000","message":"Should we move this up to just after we determined data is not None?","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c247c10cbc3c3ea4c189e44d6f95132c05663aef","unresolved":true,"context_lines":[{"line_number":168,"context_line":"            float(conf.get(\u0027objects_per_second\u0027,"},{"line_number":169,"context_line":"                           objects_per_second))"},{"line_number":170,"context_line":"        self.max_objects_per_container_per_second \u003d float("},{"line_number":171,"context_line":"            conf.get(\u0027max_objects_per_container_per_second\u0027, 0))"},{"line_number":172,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 10))"},{"line_number":173,"context_line":"        self.conn_timeout \u003d float(conf.get(\u0027conn_timeout\u0027, 0.5))"},{"line_number":174,"context_line":"        self.report_interval \u003d float(conf.get(\u0027report_interval\u0027, 300))"}],"source_content_type":"text/x-python","patch_set":1,"id":"578910d1_74e6ee19","line":171,"updated":"2021-12-07 23:50:44.000000000","message":"defaults to off","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dce7ed08e9a06e6c8591bf3a39c30ebf66e6f8c5","unresolved":true,"context_lines":[{"line_number":337,"context_line":"                        except OSError as e:"},{"line_number":338,"context_line":"                            if e.errno !\u003d errno.ENOENT:"},{"line_number":339,"context_line":"                                raise"},{"line_number":340,"context_line":"                    else:"},{"line_number":341,"context_line":"                        last_obj_hash \u003d obj_hash"},{"line_number":342,"context_line":"                        yield {\u0027device\u0027: device, \u0027policy\u0027: policy,"},{"line_number":343,"context_line":"                               \u0027path\u0027: update_path,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e307c23_a3096424","line":340,"updated":"2021-12-08 04:19:11.000000000","message":"If we move the unpickling here and place the data into the yielded container (and deal with quarantine once on failure), it should clean up a bunch of the other unpickling right?","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3afbc71414a670050496007cc91431aa29c7d75b","unresolved":true,"context_lines":[{"line_number":359,"context_line":"        ap_iter \u003d RateLimitedIterator("},{"line_number":360,"context_line":"            self._iter_async_pendings(device),"},{"line_number":361,"context_line":"            elements_per_second\u003dself.max_objects_per_second)"},{"line_number":362,"context_line":"        ap_iter \u003d BucketizedUpdateSkippingLimiter("},{"line_number":363,"context_line":"            ap_iter, 1000, self.max_objects_per_container_per_second,"},{"line_number":364,"context_line":"            self._load_update)"},{"line_number":365,"context_line":"        with ContextPool(self.concurrency) as pool:"}],"source_content_type":"text/x-python","patch_set":1,"id":"6f3b3587_a787b220","line":362,"updated":"2021-12-07 23:42:04.000000000","message":"Maybe add a\n\n if self.max_objects_per_container_per_second \u003e 0:\n     # ... wrap iter ...\n\nto reduce overhead when there\u0027s no extra rate-limiting?","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dce7ed08e9a06e6c8591bf3a39c30ebf66e6f8c5","unresolved":true,"context_lines":[{"line_number":359,"context_line":"        ap_iter \u003d RateLimitedIterator("},{"line_number":360,"context_line":"            self._iter_async_pendings(device),"},{"line_number":361,"context_line":"            elements_per_second\u003dself.max_objects_per_second)"},{"line_number":362,"context_line":"        ap_iter \u003d BucketizedUpdateSkippingLimiter("},{"line_number":363,"context_line":"            ap_iter, 1000, self.max_objects_per_container_per_second,"},{"line_number":364,"context_line":"            self._load_update)"},{"line_number":365,"context_line":"        with ContextPool(self.concurrency) as pool:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8e60ad0f_d79a1b23","line":362,"in_reply_to":"6f3b3587_a787b220","updated":"2021-12-08 04:19:11.000000000","message":"Yes, only wrapping when we need to too makes sense.","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dce7ed08e9a06e6c8591bf3a39c30ebf66e6f8c5","unresolved":true,"context_lines":[{"line_number":360,"context_line":"            self._iter_async_pendings(device),"},{"line_number":361,"context_line":"            elements_per_second\u003dself.max_objects_per_second)"},{"line_number":362,"context_line":"        ap_iter \u003d BucketizedUpdateSkippingLimiter("},{"line_number":363,"context_line":"            ap_iter, 1000, self.max_objects_per_container_per_second,"},{"line_number":364,"context_line":"            self._load_update)"},{"line_number":365,"context_line":"        with ContextPool(self.concurrency) as pool:"},{"line_number":366,"context_line":"            for update in ap_iter:"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e582878_2ea9a7d1","line":363,"updated":"2021-12-08 04:19:11.000000000","message":"Should the 1000 here be configurable? What good commenting of limiting a potential blast radius if we can\u0027t configure it? Or at least let\u0027s default it in the class maybe?","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c247c10cbc3c3ea4c189e44d6f95132c05663aef","unresolved":true,"context_lines":[{"line_number":422,"context_line":"                pass"},{"line_number":423,"context_line":"            return"},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"    def process_object_update(self, update):"},{"line_number":426,"context_line":"        \"\"\""},{"line_number":427,"context_line":"        Process the object information to be updated and update."},{"line_number":428,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e7c6c169_9c999816","line":425,"updated":"2021-12-07 23:50:44.000000000","message":"this signature change was kind of annoying - but it simplified passing in the cached read of the pickle data","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dce7ed08e9a06e6c8591bf3a39c30ebf66e6f8c5","unresolved":true,"context_lines":[{"line_number":422,"context_line":"                pass"},{"line_number":423,"context_line":"            return"},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"    def process_object_update(self, update):"},{"line_number":426,"context_line":"        \"\"\""},{"line_number":427,"context_line":"        Process the object information to be updated and update."},{"line_number":428,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"524027df_59b7f85e","line":425,"in_reply_to":"e7c6c169_9c999816","updated":"2021-12-08 04:19:11.000000000","message":"Or always pass it in with the unpickled data as mentioned in chat and above by always unpicking in `_iter_async_pendings`.","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c247c10cbc3c3ea4c189e44d6f95132c05663aef","unresolved":true,"context_lines":[{"line_number":430,"context_line":"                       device, and storage policy of object update"},{"line_number":431,"context_line":"        \"\"\""},{"line_number":432,"context_line":"        if update is None:"},{"line_number":433,"context_line":"            return"},{"line_number":434,"context_line":"        update_path \u003d update[\u0027path\u0027]"},{"line_number":435,"context_line":"        device \u003d update[\u0027device\u0027]"},{"line_number":436,"context_line":"        policy \u003d update[\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"05f6ca55_ef17983c","line":433,"updated":"2021-12-07 23:50:44.000000000","message":"having the quarantine yield None is wonky","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3afbc71414a670050496007cc91431aa29c7d75b","unresolved":true,"context_lines":[{"line_number":434,"context_line":"        update_path \u003d update[\u0027path\u0027]"},{"line_number":435,"context_line":"        device \u003d update[\u0027device\u0027]"},{"line_number":436,"context_line":"        policy \u003d update[\u0027policy\u0027]"},{"line_number":437,"context_line":"        if \u0027cached_pickle\u0027 in update:"},{"line_number":438,"context_line":"            update \u003d update[\u0027cached_pickle\u0027]"},{"line_number":439,"context_line":"        else:"},{"line_number":440,"context_line":"            update \u003d self._load_update(update)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f134bb6d_d644b1ab","line":437,"updated":"2021-12-07 23:42:04.000000000","message":"It feels weird to me that we don\u0027t know whether the update\u0027s been unpickled yet or not, since we\u0027re always trying to unpickle in BucketizedUpdateSkippingLimiter.","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dce7ed08e9a06e6c8591bf3a39c30ebf66e6f8c5","unresolved":true,"context_lines":[{"line_number":435,"context_line":"        device \u003d update[\u0027device\u0027]"},{"line_number":436,"context_line":"        policy \u003d update[\u0027policy\u0027]"},{"line_number":437,"context_line":"        if \u0027cached_pickle\u0027 in update:"},{"line_number":438,"context_line":"            update \u003d update[\u0027cached_pickle\u0027]"},{"line_number":439,"context_line":"        else:"},{"line_number":440,"context_line":"            update \u003d self._load_update(update)"},{"line_number":441,"context_line":"            if update is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"c134d98a_7207e013","line":438,"updated":"2021-12-08 04:19:11.000000000","message":"I find it a liitle wierd, that we go from a update dict object to a completely different update dict object that really holds different data. Makes reading the code a little confusing. \n\nReally the 2nd update is the de-pickled async_update, the first is what, an update_job or something.\n\nMaybe we should rename the signiture update param to something like update_job so it\u0027s obvious they are different.","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c247c10cbc3c3ea4c189e44d6f95132c05663aef","unresolved":true,"context_lines":[{"line_number":439,"context_line":"        else:"},{"line_number":440,"context_line":"            update \u003d self._load_update(update)"},{"line_number":441,"context_line":"            if update is None:"},{"line_number":442,"context_line":"                return"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":"        def do_update():"},{"line_number":445,"context_line":"            successes \u003d update.get(\u0027successes\u0027, [])"}],"source_content_type":"text/x-python","patch_set":1,"id":"cdf69b23_76716390","line":442,"updated":"2021-12-07 23:50:44.000000000","message":"sucks to have to deal with quarantine *again*","commit_id":"36fadb2ac356816736288567e678cc6837fae54f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":49,"context_line":"                        more buckets total the less containers to a bucket"},{"line_number":50,"context_line":"                        (once a busy container slows down a bucket the whole"},{"line_number":51,"context_line":"                        bucket starts skipping)"},{"line_number":52,"context_line":"    :param max_elements_per_group_per_second: tunable, when skipping kicks in"},{"line_number":53,"context_line":"    \"\"\""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def __init__(self, update_iterable, num_buckets,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9e6ac7ab_5a99be4f","line":52,"updated":"2021-12-09 15:29:25.000000000","message":"skip_counting_f is missing\n\nbut it might be better named \"on_skip\" or \"skip_callback_fn\" because the fact that it is \"counting\" is the concern of the function, not this class.","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            try:"},{"line_number":80,"context_line":"                bucket_key \u003d self._bucket_key(update_ctx[\u0027update\u0027])"},{"line_number":81,"context_line":"            except KeyError:"},{"line_number":82,"context_line":"                # this is mostly for tests"},{"line_number":83,"context_line":"                return update_ctx"},{"line_number":84,"context_line":"            now \u003d time.time()"},{"line_number":85,"context_line":"            if self.next_update[bucket_key] \u003e now:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1530103b_48f46312","line":82,"updated":"2021-12-09 15:29:25.000000000","message":"tests fixed here https://review.opendev.org/c/openstack/swift/+/821216","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":167,"context_line":"        self.max_objects_per_second \u003d \\"},{"line_number":168,"context_line":"            float(conf.get(\u0027objects_per_second\u0027,"},{"line_number":169,"context_line":"                           objects_per_second))"},{"line_number":170,"context_line":"        self.max_objects_per_container_per_second \u003d float("},{"line_number":171,"context_line":"            conf.get(\u0027max_objects_per_container_per_second\u0027, 0))"},{"line_number":172,"context_line":"        self.per_container_ratelimit_buckets \u003d int("},{"line_number":173,"context_line":"            conf.get(\u0027per_container_ratelimit_buckets\u0027, 1000))"}],"source_content_type":"text/x-python","patch_set":2,"id":"858853e1_76492d43","line":170,"range":{"start_line":170,"start_character":25,"end_line":170,"end_character":38},"updated":"2021-12-09 15:29:25.000000000","message":"as per conf comment - maybe use per_bucket in the names?\n\nalthough, for some bucket \u003d\u003d container :/","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":true,"context_lines":[{"line_number":373,"context_line":"                        if update is not None:"},{"line_number":374,"context_line":"                            yield {\u0027device\u0027: device,"},{"line_number":375,"context_line":"                                   \u0027policy\u0027: policy,"},{"line_number":376,"context_line":"                                   \u0027update_path\u0027: update_path,"},{"line_number":377,"context_line":"                                   \u0027obj_hash\u0027: obj_hash,"},{"line_number":378,"context_line":"                                   \u0027timestamp\u0027: timestamp,"},{"line_number":379,"context_line":"                                   \u0027update\u0027: update}"}],"source_content_type":"text/x-python","patch_set":2,"id":"aa48271f_b2ef2a44","line":376,"updated":"2021-12-08 22:33:30.000000000","message":"s/path/update_path fixes the signature in proccess_object_update","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":true,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        def skip_counting_f():"},{"line_number":395,"context_line":"            self.stats.skips +\u003d 1"},{"line_number":396,"context_line":"            self.logger.increment(\"skips\")"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        ap_iter \u003d RateLimitedIterator("},{"line_number":399,"context_line":"            self._iter_async_pendings(device),"}],"source_content_type":"text/x-python","patch_set":2,"id":"056c03d3_c3354d1e","line":396,"updated":"2021-12-08 22:33:30.000000000","message":"we could pass the stats \u0026 logger object in - or make this a method instead of a closure; not sure","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":true,"context_lines":[{"line_number":401,"context_line":"        ap_iter \u003d BucketizedUpdateSkippingLimiter("},{"line_number":402,"context_line":"            ap_iter, self.per_container_ratelimit_buckets,"},{"line_number":403,"context_line":"            self.max_objects_per_container_per_second,"},{"line_number":404,"context_line":"            skip_counting_f\u003dskip_counting_f)"},{"line_number":405,"context_line":"        with ContextPool(self.concurrency) as pool:"},{"line_number":406,"context_line":"            for update_ctx in ap_iter:"},{"line_number":407,"context_line":"                pool.spawn(self.process_object_update, **update_ctx)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a6d0cf2_01a06996","line":404,"updated":"2021-12-08 22:33:30.000000000","message":"I think somebody argued not wrapping this when `max_objects_per_container_per_second` was zero.\n\nI *think* the argument given was \"efficiency\", but I don\u0027t think that\u0027s a good argument.  The highest value I\u0027m going for is \"obviously correct\"\n\nIt\u0027s possible the argument was: it\u0027s \"less risk\" if \"turning it off\" cuts out the new code *entirely* (that argument could probably sway me), but my immediate counter posistion is \"doing it this way means a lot of the zero case is covered by existing unittests\" and maybe that helps make the change more obvious.  Dunno.","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":true,"context_lines":[{"line_number":404,"context_line":"            skip_counting_f\u003dskip_counting_f)"},{"line_number":405,"context_line":"        with ContextPool(self.concurrency) as pool:"},{"line_number":406,"context_line":"            for update_ctx in ap_iter:"},{"line_number":407,"context_line":"                pool.spawn(self.process_object_update, **update_ctx)"},{"line_number":408,"context_line":"                now \u003d time.time()"},{"line_number":409,"context_line":"                if now - last_status_update \u003e\u003d self.report_interval:"},{"line_number":410,"context_line":"                    this_sweep \u003d self.stats.since(start_stats)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3230555a_5dee16c8","line":407,"updated":"2021-12-08 22:33:30.000000000","message":"i learned this trick about how to do required named kwargs - i hope it\u0027s obvious enough","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":404,"context_line":"            skip_counting_f\u003dskip_counting_f)"},{"line_number":405,"context_line":"        with ContextPool(self.concurrency) as pool:"},{"line_number":406,"context_line":"            for update_ctx in ap_iter:"},{"line_number":407,"context_line":"                pool.spawn(self.process_object_update, **update_ctx)"},{"line_number":408,"context_line":"                now \u003d time.time()"},{"line_number":409,"context_line":"                if now - last_status_update \u003e\u003d self.report_interval:"},{"line_number":410,"context_line":"                    this_sweep \u003d self.stats.since(start_stats)"}],"source_content_type":"text/x-python","patch_set":2,"id":"94078496_a133b20a","line":407,"in_reply_to":"3230555a_5dee16c8","updated":"2021-12-09 15:29:25.000000000","message":"+1 that\u0027s a lot nicer than passing a dict and then unpacking it in process_update","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"396f3997dad4ec74a3b15393a773b836d7feb1e7","unresolved":true,"context_lines":[{"line_number":440,"context_line":"             \u0027redirects\u0027: sweep_totals.redirects,"},{"line_number":441,"context_line":"             \u0027skips\u0027: sweep_totals.skips})"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    def process_object_update(self, update_path, device, policy, update,"},{"line_number":444,"context_line":"                              **kwargs):"},{"line_number":445,"context_line":"        \"\"\""},{"line_number":446,"context_line":"        Process the object information to be updated and update."}],"source_content_type":"text/x-python","patch_set":2,"id":"3055df77_ee1959c9","line":443,"updated":"2021-12-09 15:29:25.000000000","message":"nit/pre-existing: usually we see device as first arg, and that\u0027s what we have with _load_update\n\narguably ( device, policy, update_path, update ) might be more \"normal\"","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a2ba771eb2841f638537c421a139c601b52ec01a","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def __init__(self, update_iterable, num_buckets,"},{"line_number":56,"context_line":"                 max_elements_per_group_per_second,"},{"line_number":57,"context_line":"                 skip_f\u003dlambda: None):"},{"line_number":58,"context_line":"        self.iterator \u003d iter(update_iterable)"},{"line_number":59,"context_line":"        # if we want a smaller \"blast radius\" we could make this number bigger"},{"line_number":60,"context_line":"        self.num_buckets \u003d num_buckets"}],"source_content_type":"text/x-python","patch_set":3,"id":"3b663f1f_9a0ba246","line":57,"range":{"start_line":57,"start_character":17,"end_line":57,"end_character":36},"updated":"2021-12-10 14:05:41.000000000","message":"this has wrong signature w.r.t. call site","commit_id":"fdd83a1f6ec621614a8eb0001994706b4c041d0a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a2ba771eb2841f638537c421a139c601b52ec01a","unresolved":true,"context_lines":[{"line_number":181,"context_line":"        self.max_objects_per_container_per_second \u003d float("},{"line_number":182,"context_line":"            conf.get(\u0027max_objects_per_container_per_second\u0027, 0))"},{"line_number":183,"context_line":"        self.per_container_ratelimit_buckets \u003d int("},{"line_number":184,"context_line":"            conf.get(\u0027per_container_ratelimit_buckets\u0027, 1000))"},{"line_number":185,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 10))"},{"line_number":186,"context_line":"        self.conn_timeout \u003d float(conf.get(\u0027conn_timeout\u0027, 0.5))"},{"line_number":187,"context_line":"        self.report_interval \u003d float(conf.get(\u0027report_interval\u0027, 300))"}],"source_content_type":"text/x-python","patch_set":3,"id":"b382cefc_f443b710","line":184,"updated":"2021-12-10 14:05:41.000000000","message":"we have helpers to constrain these to be non-negative","commit_id":"fdd83a1f6ec621614a8eb0001994706b4c041d0a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"047f35927a55344afc6260b6f3cea49996177434","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        self.max_objects_per_container_per_second \u003d float("},{"line_number":182,"context_line":"            conf.get(\u0027max_objects_per_container_per_second\u0027, 0))"},{"line_number":183,"context_line":"        self.per_container_ratelimit_buckets \u003d int("},{"line_number":184,"context_line":"            conf.get(\u0027per_container_ratelimit_buckets\u0027, 1000))"},{"line_number":185,"context_line":"        self.node_timeout \u003d float(conf.get(\u0027node_timeout\u0027, 10))"},{"line_number":186,"context_line":"        self.conn_timeout \u003d float(conf.get(\u0027conn_timeout\u0027, 0.5))"},{"line_number":187,"context_line":"        self.report_interval \u003d float(conf.get(\u0027report_interval\u0027, 300))"}],"source_content_type":"text/x-python","patch_set":3,"id":"2c929002_9b3489d1","line":184,"in_reply_to":"b382cefc_f443b710","updated":"2021-12-10 20:13:15.000000000","message":"+1, and now done.","commit_id":"fdd83a1f6ec621614a8eb0001994706b4c041d0a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"047f35927a55344afc6260b6f3cea49996177434","unresolved":true,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def __init__(self, update_iterable, num_buckets,"},{"line_number":57,"context_line":"                 max_elements_per_group_per_second,"},{"line_number":58,"context_line":"                 skip_f\u003dlambda *args, **kwargs: None):"},{"line_number":59,"context_line":"        self.iterator \u003d iter(update_iterable)"},{"line_number":60,"context_line":"        # if we want a smaller \"blast radius\" we could make this number bigger"},{"line_number":61,"context_line":"        self.num_buckets \u003d max(num_buckets, 1)"}],"source_content_type":"text/x-python","patch_set":4,"id":"244929fb_2c314781","line":58,"updated":"2021-12-10 20:13:15.000000000","message":"Better as\n\n skip_f\u003dlambda update_ctx: None\n\nso we give callers a better idea of what the signature looks like.","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3cfb9148450daeb20b1bfa856a2421958799c034","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def __init__(self, update_iterable, num_buckets,"},{"line_number":57,"context_line":"                 max_elements_per_group_per_second,"},{"line_number":58,"context_line":"                 skip_f\u003dlambda *args, **kwargs: None):"},{"line_number":59,"context_line":"        self.iterator \u003d iter(update_iterable)"},{"line_number":60,"context_line":"        # if we want a smaller \"blast radius\" we could make this number bigger"},{"line_number":61,"context_line":"        self.num_buckets \u003d max(num_buckets, 1)"}],"source_content_type":"text/x-python","patch_set":4,"id":"71931c1d_da96ef24","line":58,"in_reply_to":"244929fb_2c314781","updated":"2021-12-10 21:42:10.000000000","message":"Done","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"047f35927a55344afc6260b6f3cea49996177434","unresolved":true,"context_lines":[{"line_number":68,"context_line":"        self.skip_f \u003d skip_f"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def __iter__(self):"},{"line_number":71,"context_line":"        return self"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _bucket_key(self, update):"},{"line_number":74,"context_line":"        hash_path \u003d \u0027%s/%s\u0027 % split_update_path(update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"6dd6b13c_8eb4aa79","line":71,"updated":"2021-12-10 20:13:15.000000000","message":"Out of curiosity: why not just have __iter__() be next() with yields and no StopIteration, and drop next/__next__?","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9f7fcbca2b0ec13c8583fd0bd892d8f961b2cbdc","unresolved":true,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _bucket_key(self, update):"},{"line_number":74,"context_line":"        hash_path \u003d \u0027%s/%s\u0027 % split_update_path(update)"},{"line_number":75,"context_line":"        key \u003d md5(hash_path.encode(\u0027utf-8\u0027), usedforsecurity\u003dFalse).hexdigest()"},{"line_number":76,"context_line":"        return int(key, 16) % self.num_buckets"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def next(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"b366e4e9_61dc64eb","line":75,"updated":"2021-12-10 21:21:10.000000000","message":"This has troubles on py2, where hash_path is already bytes. I\u0027d rephrase it all as something like\n\n hash_path \u003d \u0027%s/%s\u0027 % split_update_path(update)\n if isinstance(hash_path, bytes):\n     hash_path \u003d hash_path.encode(\u0027utf-8\u0027)\n key \u003d md5(hash_path, usedforsecurity\u003dFalse).hexdigest()","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3cfb9148450daeb20b1bfa856a2421958799c034","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _bucket_key(self, update):"},{"line_number":74,"context_line":"        hash_path \u003d \u0027%s/%s\u0027 % split_update_path(update)"},{"line_number":75,"context_line":"        key \u003d md5(hash_path.encode(\u0027utf-8\u0027), usedforsecurity\u003dFalse).hexdigest()"},{"line_number":76,"context_line":"        return int(key, 16) % self.num_buckets"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def next(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"388287a8_abbe0be2","line":75,"in_reply_to":"b366e4e9_61dc64eb","updated":"2021-12-10 21:42:10.000000000","message":"OK, the recommendation was off, but it should be fixed now. ;-)","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"047f35927a55344afc6260b6f3cea49996177434","unresolved":true,"context_lines":[{"line_number":94,"context_line":"    Stats bucket for an update sweep"},{"line_number":95,"context_line":"    \"\"\""},{"line_number":96,"context_line":"    def __init__(self, errors\u003d0, failures\u003d0, quarantines\u003d0, successes\u003d0,"},{"line_number":97,"context_line":"                 unlinks\u003d0, redirects\u003d0, skips\u003d0):"},{"line_number":98,"context_line":"        self.errors \u003d errors"},{"line_number":99,"context_line":"        self.failures \u003d failures"},{"line_number":100,"context_line":"        self.quarantines \u003d quarantines"}],"source_content_type":"text/x-python","patch_set":4,"id":"277b3e5b_273b6e1c","line":97,"updated":"2021-12-10 20:13:15.000000000","message":"+1, I\u0027d forgotten all about the logged stats.","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"047f35927a55344afc6260b6f3cea49996177434","unresolved":true,"context_lines":[{"line_number":403,"context_line":"        self.logger.info(\"Object update sweep starting on %s (pid: %d)\","},{"line_number":404,"context_line":"                         device, my_pid)"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"        def skip_counting_f(update_ctx):"},{"line_number":407,"context_line":"            # in the future we could defer update_ctx"},{"line_number":408,"context_line":"            self.stats.skips +\u003d 1"},{"line_number":409,"context_line":"            self.logger.increment(\"skips\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"38c29b23_0bd7e88b","line":406,"updated":"2021-12-10 20:13:15.000000000","message":"I might even recommend pulling this up to be its own method.","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3cfb9148450daeb20b1bfa856a2421958799c034","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        self.logger.info(\"Object update sweep starting on %s (pid: %d)\","},{"line_number":404,"context_line":"                         device, my_pid)"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"        def skip_counting_f(update_ctx):"},{"line_number":407,"context_line":"            # in the future we could defer update_ctx"},{"line_number":408,"context_line":"            self.stats.skips +\u003d 1"},{"line_number":409,"context_line":"            self.logger.increment(\"skips\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"72367dad_640ce7c4","line":406,"in_reply_to":"38c29b23_0bd7e88b","updated":"2021-12-10 21:42:10.000000000","message":"Oh, I see -- the follow-up does it. Sure.","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"047f35927a55344afc6260b6f3cea49996177434","unresolved":true,"context_lines":[{"line_number":411,"context_line":"        ap_iter \u003d RateLimitedIterator("},{"line_number":412,"context_line":"            self._iter_async_pendings(device),"},{"line_number":413,"context_line":"            elements_per_second\u003dself.max_objects_per_second)"},{"line_number":414,"context_line":"        ap_iter \u003d BucketizedUpdateSkippingLimiter("},{"line_number":415,"context_line":"            ap_iter, self.per_container_ratelimit_buckets,"},{"line_number":416,"context_line":"            self.max_objects_per_container_per_second,"},{"line_number":417,"context_line":"            skip_f\u003dskip_counting_f)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a23714e_4463437d","line":414,"updated":"2021-12-10 20:13:15.000000000","message":"Still think it\u0027s worth only instantiating the BucketizedUpdateSkippingLimiter and doing all this extra book-keeping every iteration if max_objects_per_container_per_second is non-zero; something like\n\n if self.max_objects_per_container_per_second:\n     ap_iter \u003d BucketizedUpdateSkippingLimiter(ap_iter, ...)\n\nAlso saves us from needing to catch the ZeroDivisionError.","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3cfb9148450daeb20b1bfa856a2421958799c034","unresolved":false,"context_lines":[{"line_number":411,"context_line":"        ap_iter \u003d RateLimitedIterator("},{"line_number":412,"context_line":"            self._iter_async_pendings(device),"},{"line_number":413,"context_line":"            elements_per_second\u003dself.max_objects_per_second)"},{"line_number":414,"context_line":"        ap_iter \u003d BucketizedUpdateSkippingLimiter("},{"line_number":415,"context_line":"            ap_iter, self.per_container_ratelimit_buckets,"},{"line_number":416,"context_line":"            self.max_objects_per_container_per_second,"},{"line_number":417,"context_line":"            skip_f\u003dskip_counting_f)"}],"source_content_type":"text/x-python","patch_set":4,"id":"f60c60de_4a4fa396","line":414,"in_reply_to":"1a23714e_4463437d","updated":"2021-12-10 21:42:10.000000000","message":"Oh, I guess that would mess with the time points established in test_sweep_logs... IDK that I care *that* much.","commit_id":"db0379712ef9f335f83d758612b32360fcbe27ef"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8f928f1fc1b60920d2f3b872c2347fca5f4a2232","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        if not isinstance(hash_path, bytes):"},{"line_number":76,"context_line":"            hash_path \u003d hash_path.encode(\u0027utf-8\u0027)"},{"line_number":77,"context_line":"        # else, py2"},{"line_number":78,"context_line":"        key \u003d md5(hash_path, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":79,"context_line":"        return int(key, 16) % self.num_buckets"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def next(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"996232b0_e5ff2286","line":78,"updated":"2021-12-17 14:34:28.000000000","message":"this should use some salt; or maybe even piggy back on hash_path/get_part","commit_id":"d777c3aa9b7cb20377c463e4d4d6a6fa0596d5bc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"45a5b2ae92702d13ffc6d5dcdad38a9c55c8106a","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        if not isinstance(hash_path, bytes):"},{"line_number":76,"context_line":"            hash_path \u003d hash_path.encode(\u0027utf-8\u0027)"},{"line_number":77,"context_line":"        # else, py2"},{"line_number":78,"context_line":"        key \u003d md5(hash_path, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":79,"context_line":"        return int(key, 16) % self.num_buckets"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def next(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"69bb1b2a_45c05bd0","line":78,"in_reply_to":"996232b0_e5ff2286","updated":"2022-01-06 00:16:07.000000000","message":"Done","commit_id":"d777c3aa9b7cb20377c463e4d4d6a6fa0596d5bc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c3c6abe8ecca15d42907b0cccaf4a757ad6ce23a","unresolved":true,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _bucket_key(self, update):"},{"line_number":74,"context_line":"        acct, cont \u003d split_update_path(update)"},{"line_number":75,"context_line":"        return int(hash_path(acct, cont), 16) % self.num_buckets"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    def next(self):"},{"line_number":78,"context_line":"        for update_ctx in self.iterator:"}],"source_content_type":"text/x-python","patch_set":6,"id":"2dc15207_55ae42bf","line":75,"updated":"2022-01-06 10:21:43.000000000","message":"this will hash the same containers into the same buckets for every sweep on every node, which is quite unfair on the less busy container that happens to hash into the same bucket as the busy container. How about adding some salt that is randomly generated for each  BucketizedUpdateSkippingLimiter instance? That way, the less busy container may map to a different bucket on different nodes and on different sweeps.","commit_id":"631dc4f0ff98ab82bcca3a05c4849b1f3ba2e703"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0ee363978a3cf3065cae0f255a2306cc02bb05c0","unresolved":true,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _bucket_key(self, update):"},{"line_number":74,"context_line":"        acct, cont \u003d split_update_path(update)"},{"line_number":75,"context_line":"        return int(hash_path(acct, cont), 16) % self.num_buckets"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    def next(self):"},{"line_number":78,"context_line":"        for update_ctx in self.iterator:"}],"source_content_type":"text/x-python","patch_set":6,"id":"aefb443d_9ef3a606","line":75,"in_reply_to":"2dc15207_55ae42bf","updated":"2022-01-06 20:47:27.000000000","message":"Seems fair. Done.","commit_id":"631dc4f0ff98ab82bcca3a05c4849b1f3ba2e703"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3dd78760ae846354d65bd1e8bfbc2b31acea7a0","unresolved":true,"context_lines":[{"line_number":407,"context_line":"        def skip_counting_f(update_ctx):"},{"line_number":408,"context_line":"            # in the future we could defer update_ctx"},{"line_number":409,"context_line":"            self.stats.skips +\u003d 1"},{"line_number":410,"context_line":"            self.logger.increment(\"skips\")"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"        ap_iter \u003d RateLimitedIterator("},{"line_number":413,"context_line":"            self._iter_async_pendings(device),"}],"source_content_type":"text/x-python","patch_set":7,"id":"f40cc9c1_a6d5b6fa","line":410,"updated":"2022-01-10 16:34:13.000000000","message":"maybe we should pull in some of the SweepStats updates to this change if we still want to keep it separate form the deffer updates","commit_id":"de888629817a2a326b6e8dc66edb0ce3168818a7"}],"test/unit/obj/test_updater.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":true,"context_lines":[{"line_number":1233,"context_line":"                \u0027headers\u0027: {"},{"line_number":1234,"context_line":"                    \u0027X-Container-Timestamp\u0027: normalize_timestamp(0),"},{"line_number":1235,"context_line":"                },"},{"line_number":1236,"context_line":"            }, async_pending)"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    def _find_async_pending_files(self):"},{"line_number":1239,"context_line":"        found_files \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"6341525f_09bb156d","line":1236,"updated":"2021-12-08 22:33:30.000000000","message":"i was surprised this helper didn\u0027t seem to exist","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"cbe47a954c3b01e1eb517859a9a162d2e0300ec8","unresolved":true,"context_lines":[{"line_number":1268,"context_line":"        expected_skipped \u003d expected_total - expected_success"},{"line_number":1269,"context_line":"        self.assertEqual(expected_skipped, daemon.stats.skips)"},{"line_number":1270,"context_line":"        self.assertEqual(expected_skipped,"},{"line_number":1271,"context_line":"                         len(self._find_async_pending_files()))"},{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b21a0327_230dec93","line":1271,"updated":"2021-12-08 22:33:30.000000000","message":"my test:code ratio isn\u0027t great yet - I think some of the refactoring bumped my line count up cause I think this is a pretty good test.  Maybe do something with num_buckets \u0026 blast radius?","commit_id":"e02b5c3f737a53c3075f511da0a5381968716b98"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2c42f20a9548bba05980c52691c7272034256671","unresolved":true,"context_lines":[{"line_number":1295,"context_line":"        expected_total \u003d num_c1_files"},{"line_number":1296,"context_line":"        self.assertEqual(expected_total,"},{"line_number":1297,"context_line":"                         len(self._find_async_pending_files()))"},{"line_number":1298,"context_line":"        latencies \u003d [.11, 0, .11, 0]"},{"line_number":1299,"context_line":"        expected_success \u003d 2"},{"line_number":1300,"context_line":"        fake_status_codes \u003d [200] * 3 * expected_success"},{"line_number":1301,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"a602ae22_b51cb301","line":1298,"updated":"2021-12-10 01:08:47.000000000","message":"OK these latencies threw me for a while. Because they happen on spawn we never actually use all 4. \n\nFirst async goes through because Bucket iter starts at 0.0. The async sleeps for 0.11 seconds. The second async is allowed out of the bucket iter because enough time has passed. It sleeps 0 seconds in fake_spawn. So the third async gets dropped because .10 seconds hasn\u0027t passed. The fourth is the same, because again the 0.10 seconds hasn\u0027t passed.\n\nThat\u0027s why we get 2 successes. if you then go look at the latencies left over we still got 2 left.\n\n  # Put this at the end\n  self.assertEqual(len(latencies), 2)","commit_id":"fdd83a1f6ec621614a8eb0001994706b4c041d0a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1bdcf70671b404edb34afb1ac0da35697373971a","unresolved":true,"context_lines":[{"line_number":1295,"context_line":"        expected_total \u003d num_c1_files"},{"line_number":1296,"context_line":"        self.assertEqual(expected_total,"},{"line_number":1297,"context_line":"                         len(self._find_async_pending_files()))"},{"line_number":1298,"context_line":"        latencies \u003d [.11, 0, .11, 0]"},{"line_number":1299,"context_line":"        expected_success \u003d 2"},{"line_number":1300,"context_line":"        fake_status_codes \u003d [200] * 3 * expected_success"},{"line_number":1301,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"f0d25c27_3eae7798","line":1298,"in_reply_to":"a602ae22_b51cb301","updated":"2021-12-10 06:33:19.000000000","message":"reworked the latencies in https://review.opendev.org/c/openstack/swift/+/821326 so it patches the RateLimitIterator before the bucket iterator so we can control timings of each update hitting it. Allowing us to better test it.","commit_id":"fdd83a1f6ec621614a8eb0001994706b4c041d0a"}]}
