)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"130970110c04f328a761427e43f3ad2046f41eaa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1dfeb09e_ac92f950","updated":"2021-12-10 06:31:27.000000000","message":"I hope you don\u0027t mind me pushing over. Only skipping if not deferred and if we run out of interval time what\u0027s left on the queue is added to skips.\n\nReworked the tests the latencies are for each iteration and so we have more control.\n\nAdded a test that drops and then reruns it from the deferred queue and is successful. And another to test to show updates stuck in the deferred queue when the interval passes is added to skips.","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e0ad09c8_c909a47d","updated":"2021-12-10 16:50:51.000000000","message":"This worked out well, will push a follow up with some fixups, mostly to the tests","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22825a4b670918cd69bdf0352ac5f5425f43e449","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"24e894ba_78578d2a","updated":"2021-12-10 16:57:02.000000000","message":"fixups here https://review.opendev.org/c/openstack/swift/+/821422","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a1f792641d4cd8cd799b450e5210a1fbbb0934be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"be757ad1_233b8902","updated":"2021-12-10 23:51:51.000000000","message":"i guess this is fine; but I don\u0027t love it.  The tests have helped a lot.\n\nIt *seems* like we should be able to detect when our re-iterate is spinning\n\n    (len \u003d\u003d last_len) ???","commit_id":"af17424f727737bee70f45b1928f83b710970079"}],"swift/obj/updater.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":true,"context_lines":[{"line_number":190,"context_line":"                                         DEFAULT_RECON_CACHE_PATH)"},{"line_number":191,"context_line":"        self.rcache \u003d os.path.join(self.recon_cache_path, RECON_OBJECT_FILE)"},{"line_number":192,"context_line":"        self.stats \u003d SweepStats()"},{"line_number":193,"context_line":"        self.max_deferred_updates \u003d int("},{"line_number":194,"context_line":"            conf.get(\u0027max_deferred_updates\u0027, 10000))"},{"line_number":195,"context_line":"        self.deferred_updates \u003d deque(maxlen\u003dself.max_deferred_updates)"},{"line_number":196,"context_line":"        self.begin \u003d time.time()"},{"line_number":197,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e1c0a414_be6d6d81","line":194,"range":{"start_line":193,"start_character":8,"end_line":194,"end_character":52},"updated":"2021-12-10 16:50:51.000000000","message":"this must not be negative : deque will blow up","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":true,"context_lines":[{"line_number":394,"context_line":"                                   \u0027timestamp\u0027: timestamp,"},{"line_number":395,"context_line":"                                   \u0027update\u0027: update}"},{"line_number":396,"context_line":"        # process any deferred work"},{"line_number":397,"context_line":"        elapsed \u003d time.time() - self.begin"},{"line_number":398,"context_line":"        while elapsed \u003c self.interval:"},{"line_number":399,"context_line":"            try:"},{"line_number":400,"context_line":"                yield self.deferred_updates.popleft()"}],"source_content_type":"text/x-python","patch_set":2,"id":"85bba5fb_70def347","line":397,"range":{"start_line":397,"start_character":18,"end_line":397,"end_character":42},"updated":"2021-12-10 16:50:51.000000000","message":"nit: we could just write\n\n  while time.time() - self.begin \u003c self.interval","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"130970110c04f328a761427e43f3ad2046f41eaa","unresolved":true,"context_lines":[{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            # update the skipped stats with that of the items left in the"},{"line_number":406,"context_line":"            # queue. Also put an IF check in place so we don\u0027t trigger a"},{"line_number":407,"context_line":"            # statsd for no update for no reason."},{"line_number":408,"context_line":"            if self.deferred_updates:"},{"line_number":409,"context_line":"                self.stats.skips +\u003d len(self.deferred_updates)"},{"line_number":410,"context_line":"                self.logger.update_stats(\"skips\", len(self.deferred_updates))"}],"source_content_type":"text/x-python","patch_set":2,"id":"38c8e2ff_4ed41986","line":407,"range":{"start_line":407,"start_character":21,"end_line":407,"end_character":27},"updated":"2021-12-10 06:31:27.000000000","message":"no idea where this came from. sigh.","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            # update the skipped stats with that of the items left in the"},{"line_number":406,"context_line":"            # queue. Also put an IF check in place so we don\u0027t trigger a"},{"line_number":407,"context_line":"            # statsd for no update for no reason."},{"line_number":408,"context_line":"            if self.deferred_updates:"},{"line_number":409,"context_line":"                self.stats.skips +\u003d len(self.deferred_updates)"},{"line_number":410,"context_line":"                self.logger.update_stats(\"skips\", len(self.deferred_updates))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3c5bee65_14b7e964","line":407,"range":{"start_line":407,"start_character":21,"end_line":407,"end_character":27},"in_reply_to":"38c8e2ff_4ed41986","updated":"2021-12-10 16:50:51.000000000","message":"Done","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a34263209f621540fae5484d78838b07b2462781","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        self.rcache \u003d os.path.join(self.recon_cache_path, RECON_OBJECT_FILE)"},{"line_number":193,"context_line":"        self.stats \u003d SweepStats()"},{"line_number":194,"context_line":"        self.max_deferred_updates \u003d int("},{"line_number":195,"context_line":"            conf.get(\u0027max_deferred_updates\u0027, 10000))"},{"line_number":196,"context_line":"        self.deferred_updates \u003d deque(maxlen\u003dself.max_deferred_updates)"},{"line_number":197,"context_line":"        self.begin \u003d time.time()"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"f1c0befd_8f2cb7fa","line":195,"range":{"start_line":195,"start_character":45,"end_line":195,"end_character":50},"updated":"2021-12-10 21:14:59.000000000","message":"I wonder if 15k would be a better default -- that\u0027d be the max you\u0027d need given the defaults for objects_per_second and interval.","commit_id":"78b637c34c0c906987baa72f9db7ee7985dadfac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a34263209f621540fae5484d78838b07b2462781","unresolved":true,"context_lines":[{"line_number":402,"context_line":"            except IndexError:"},{"line_number":403,"context_line":"                break"},{"line_number":404,"context_line":"            elapsed \u003d time.time() - self.begin"},{"line_number":405,"context_line":"        else:"},{"line_number":406,"context_line":"            # update the skipped stats with that of the items left in the"},{"line_number":407,"context_line":"            # queue. Also put an IF check in place so we don\u0027t trigger a"},{"line_number":408,"context_line":"            # statsd for no update for no reason."}],"source_content_type":"text/x-python","patch_set":3,"id":"c17a0a1a_6c035c03","line":405,"range":{"start_line":405,"start_character":8,"end_line":405,"end_character":12},"updated":"2021-12-10 21:14:59.000000000","message":"Not sure this else is really necessary since we\u0027ve got the `if self.deferred_updates` anyway.","commit_id":"78b637c34c0c906987baa72f9db7ee7985dadfac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a34263209f621540fae5484d78838b07b2462781","unresolved":true,"context_lines":[{"line_number":411,"context_line":"                self.logger.update_stats(\"skips\", len(self.deferred_updates))"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"    def skip_update(self, update_ctx):"},{"line_number":414,"context_line":"        if len(self.deferred_updates) \u003d\u003d self.deferred_updates.maxlen:"},{"line_number":415,"context_line":"            self.stats.skips +\u003d 1"},{"line_number":416,"context_line":"            self.logger.increment(\"skips\")"},{"line_number":417,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"495d3bde_7e5966e7","line":414,"updated":"2021-12-10 21:14:59.000000000","message":"Good thing we\u0027re not trying to use *real* threads -- I\u0027d worry about some other thread\u0027s append() sneeking in between this length check and *our* append().\n\nSide note: seems like a missed opportunity that there isn\u0027t some deque.is_full property...","commit_id":"78b637c34c0c906987baa72f9db7ee7985dadfac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a1f792641d4cd8cd799b450e5210a1fbbb0934be","unresolved":true,"context_lines":[{"line_number":401,"context_line":"                yield self.deferred_updates.popleft()"},{"line_number":402,"context_line":"            except IndexError:"},{"line_number":403,"context_line":"                break"},{"line_number":404,"context_line":"            elapsed \u003d time.time() - self.begin"},{"line_number":405,"context_line":"        else:"},{"line_number":406,"context_line":"            # update the skipped stats with count of the items left in the"},{"line_number":407,"context_line":"            # queue, but don\u0027t trigger a statsd if there are no items."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba0ee703_0576c27a","line":404,"updated":"2021-12-10 23:51:51.000000000","message":"I don\u0027t like how this spin here doesn\u0027t sleep - we\u0027re totally dependent on the global ratelimit (which we want to be HIGH) to keep us from just passing our entire defferred list back to ourselves via the skip function OVER AND OVER again until we *finally* catch a break with the per_container ratelimit (which we want to be LOOOOOOW)\n\nCould we somehow detect or validate \"all my deferred updates have the same bucket_key\" \u003d\u003e \"take a bucket_update_delta size nap\"","commit_id":"af17424f727737bee70f45b1928f83b710970079"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a1f792641d4cd8cd799b450e5210a1fbbb0934be","unresolved":true,"context_lines":[{"line_number":415,"context_line":"            self.logger.increment(\"skips\")"},{"line_number":416,"context_line":"        else:"},{"line_number":417,"context_line":"            # There is room on the deferred queue so don\u0027t update stats yet"},{"line_number":418,"context_line":"            self.deferred_updates.append(update_ctx)"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"    def object_sweep(self, device):"},{"line_number":421,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"40a6833c_c5eb15a9","line":418,"updated":"2021-12-10 23:51:51.000000000","message":"this is wrong; we should always append (it automatically pushes the oldest one out)\n\nwe want to do the most recently opened asyncs because on a long cycle (with a long interval) the ones opened most recently from disk have had the least amount of time to become stale from an object-server overwrite\n\nthere\u0027s nothing in the order that makes the later updates older or newer - they just have a higher chance of being more representative of what\u0027s on disk (which is what we\u0027d always processed before)","commit_id":"af17424f727737bee70f45b1928f83b710970079"}],"test/unit/obj/test_updater.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":true,"context_lines":[{"line_number":1306,"context_line":"        fake_status_codes \u003d [200] * 3 * expected_success"},{"line_number":1307,"context_line":""},{"line_number":1308,"context_line":"        # NOTE: We dont need to grab orig_next if we only supported py3, the"},{"line_number":1309,"context_line":"        # mock.patch just works as expected."},{"line_number":1310,"context_line":"        if six.PY3:"},{"line_number":1311,"context_line":"            next_path \u003d \u0027swift.obj.updater.RateLimitedIterator.__next__\u0027"},{"line_number":1312,"context_line":"            orig_next \u003d RateLimitedIterator.__next__"}],"source_content_type":"text/x-python","patch_set":2,"id":"96f48a3f_311cc906","line":1309,"updated":"2021-12-10 16:50:51.000000000","message":"I think we can avoid this by hooking into the ratelimit_if call back of RateLimitedIterator","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":true,"context_lines":[{"line_number":1330,"context_line":"        expected_skipped \u003d expected_total - expected_success"},{"line_number":1331,"context_line":"        self.assertEqual(expected_skipped, daemon.stats.skips)"},{"line_number":1332,"context_line":"        self.assertEqual(expected_skipped,"},{"line_number":1333,"context_line":"                         len(self._find_async_pending_files()))"},{"line_number":1334,"context_line":""},{"line_number":1335,"context_line":"    @mock.patch(\u0027swift.obj.updater.dump_recon_cache\u0027)"},{"line_number":1336,"context_line":"    def test_per_container_rate_limit_drop_2_pickup_1(self, mock_recon):"}],"source_content_type":"text/x-python","patch_set":2,"id":"336dbf56_4c5e559a","line":1333,"updated":"2021-12-10 16:50:51.000000000","message":"thanks Matt - this a much better approach i.e. insert delay into the feeding iter rather than into the responses","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"130970110c04f328a761427e43f3ad2046f41eaa","unresolved":true,"context_lines":[{"line_number":1358,"context_line":"        expected_success \u003d 3"},{"line_number":1359,"context_line":"        fake_status_codes \u003d [200] * 3 * expected_success"},{"line_number":1360,"context_line":""},{"line_number":1361,"context_line":"        if six.PY3:"},{"line_number":1362,"context_line":"            next_path \u003d \u0027swift.obj.updater.RateLimitedIterator.__next__\u0027"},{"line_number":1363,"context_line":"            orig_next \u003d RateLimitedIterator.__next__"},{"line_number":1364,"context_line":"        else:"},{"line_number":1365,"context_line":"            next_path \u003d \u0027swift.obj.updater.RateLimitedIterator.next\u0027"},{"line_number":1366,"context_line":"            orig_next \u003d RateLimitedIterator.next"},{"line_number":1367,"context_line":""},{"line_number":1368,"context_line":"        def fake_next(cls):"},{"line_number":1369,"context_line":"            # make each update delay before the iter being called again"},{"line_number":1370,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"d51a9048_fffa316d","line":1367,"range":{"start_line":1361,"start_character":8,"end_line":1367,"end_character":0},"updated":"2021-12-10 06:31:27.000000000","message":"I can\u0027t wait until we don\u0027t have to do py2 anymore. This is ugly. On py3 the mock.patch just works as expected (and can just do it to the __next__. But on Py2 we need to grab the original next because otherwise we end up in recursion death, I guess patch does too good a job :P","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"130970110c04f328a761427e43f3ad2046f41eaa","unresolved":true,"context_lines":[{"line_number":1375,"context_line":"            return orig_next(cls)"},{"line_number":1376,"context_line":""},{"line_number":1377,"context_line":"        with mocked_http_conn(*fake_status_codes), \\"},{"line_number":1378,"context_line":"                mock.patch(next_path, fake_next):"},{"line_number":1379,"context_line":"            daemon.run_once()"},{"line_number":1380,"context_line":"        self.assertEqual(expected_success, daemon.stats.successes)"},{"line_number":1381,"context_line":"        expected_skipped \u003d expected_total - expected_success"}],"source_content_type":"text/x-python","patch_set":2,"id":"fd316830_bcec64fc","line":1378,"range":{"start_line":1378,"start_character":16,"end_line":1378,"end_character":49},"updated":"2021-12-10 06:31:27.000000000","message":"By mocking the RateLimitIterator rather then the spawn, we can control time before it gets into the bucket ratelimiter. So we get much more control. Each latency is now triggered for each iteration. With spawn it only sleeps updates that aren\u0027t dropped on the floor, so we only ended up using the first 2 latencies.","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e861bb302b765a8aa717dbf818b265a1fc7e77a6","unresolved":true,"context_lines":[{"line_number":1425,"context_line":"            # make each update delay before the iter being called again"},{"line_number":1426,"context_line":"            count[0] +\u003d 1"},{"line_number":1427,"context_line":"            if count[0] \u003d\u003d 8:"},{"line_number":1428,"context_line":"                eventlet.sleep(1)"},{"line_number":1429,"context_line":"                # There should be 3 on the deferred queue"},{"line_number":1430,"context_line":"                self.assertEqual(len(daemon.deferred_updates), 3)"},{"line_number":1431,"context_line":"                self.assertEqual(daemon.stats.skips, 0)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0a239bae_350d1bf8","line":1428,"updated":"2021-12-10 16:50:51.000000000","message":"we can avoid this by setting the daemon.interval to 0 after a number of iterations","commit_id":"1c785e58cae1edcab9e5afe20ef3a327e18b0838"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a1f792641d4cd8cd799b450e5210a1fbbb0934be","unresolved":true,"context_lines":[{"line_number":1467,"context_line":"            [],"},{"line_number":1468,"context_line":"            [],"},{"line_number":1469,"context_line":"            all_objs[1:2],"},{"line_number":1470,"context_line":"            all_objs[1:2],"},{"line_number":1471,"context_line":"            [],"},{"line_number":1472,"context_line":"            []"},{"line_number":1473,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":5,"id":"a51316f6_3bd2b17b","line":1470,"updated":"2021-12-10 23:51:51.000000000","message":"it\u0027s hard for me to remember this is the same as:\n\n  [all_objs[1]],\n\ni.e. we\u0027ve queued the one we skipped","commit_id":"af17424f727737bee70f45b1928f83b710970079"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a1f792641d4cd8cd799b450e5210a1fbbb0934be","unresolved":true,"context_lines":[{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"        def ratelimit_if(value):"},{"line_number":1515,"context_line":"            if len(captured_queues) \u003d\u003d 10:"},{"line_number":1516,"context_line":"                daemon.interval \u003d 0  # force the iter to terminate"},{"line_number":1517,"context_line":"            contexts_fed_in.append(value)"},{"line_number":1518,"context_line":"            captured_queues.append(list(daemon.deferred_updates))"},{"line_number":1519,"context_line":"            captured_skips_stats.append(daemon.stats.skips)"}],"source_content_type":"text/x-python","patch_set":5,"id":"69f9104c_110c7a17","line":1516,"updated":"2021-12-10 23:51:51.000000000","message":"this is cheating!","commit_id":"af17424f727737bee70f45b1928f83b710970079"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a1f792641d4cd8cd799b450e5210a1fbbb0934be","unresolved":true,"context_lines":[{"line_number":1559,"context_line":"            all_objs[4:5] + all_objs[1:2],"},{"line_number":1560,"context_line":"            all_objs[1:2],  # note: one in queue, one in-flight..."},{"line_number":1561,"context_line":"            all_objs[4:5],  # ...and they rotate"},{"line_number":1562,"context_line":"            all_objs[1:2],"},{"line_number":1563,"context_line":"        ]"},{"line_number":1564,"context_line":"        self.assertEqual("},{"line_number":1565,"context_line":"            expected_deferrals,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1acd0654_acb828a0","line":1562,"updated":"2021-12-10 23:51:51.000000000","message":"good thing we popped the interval after 10 iterations; what will this look like on our nodes if we want to tune up interval in order to give the updater a chance to focus on slow root containers? 😮","commit_id":"af17424f727737bee70f45b1928f83b710970079"}]}