)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ab8e61c013d67bb105be98a006697e2b0da689a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8b17fe50_b17db81d","updated":"2025-11-10 16:47:08.000000000","message":"please update the example config for the container-sharder to demonsrate that it exposes/uses this new common.db_replicator config option (you can probably copy from the container-replicator)\n\nA couple of tests that validate ContainerSharder config-parsing/setup would be a welcome addition - it\u0027s a small value, but hopefully also low cost.\n\nA happy-path test that sets up multiple brokers - and verifies they finish as completed (particularly one with a slow update root, or replicate_object call or something) might be helpful too - it\u0027s never been super clear to me where actually expect most of the cooperative concurrency to come from in a db replicator; and we tend to run it pretty small in prod:\n\n```\n[root@s8k-sjc11-f13-ac-02 ~]# /opt/ss/bin/swift-config /etc/swift/container-server/2.conf -s container-replicator | grep concurrency\nconcurrency \u003d 8\n```","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"}],"swift/container/sharder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ab8e61c013d67bb105be98a006697e2b0da689a1","unresolved":true,"context_lines":[{"line_number":2622,"context_line":"        if not dirs:"},{"line_number":2623,"context_line":"            self.logger.info(\u0027Found no containers directories\u0027)"},{"line_number":2624,"context_line":"        for part, path, node in self.roundrobin_datadirs(dirs):"},{"line_number":2625,"context_line":"            self.cpool.spawn_n(self._process_broker_worker, part, path, node)"},{"line_number":2626,"context_line":"        self.cpool.waitall()"},{"line_number":2627,"context_line":"        stats.kill()"},{"line_number":2628,"context_line":"        self._report_stats()"}],"source_content_type":"text/x-python","patch_set":1,"id":"46d14689_75e9fdc7","line":2625,"updated":"2025-11-10 16:47:08.000000000","message":"wow, this `self.cpool` is inherited from way over in `common.db_replicator`\n\n```\nclass Replicator(Daemon):\n    \"\"\"\n    Implements the logic for directing db replication.\n    \"\"\"\n\n    def __init__(self, conf, logger\u003dNone):\n        self.conf \u003d conf\n        self.logger \u003d logger or get_logger(conf, log_route\u003d\u0027replicator\u0027)\n        self.root \u003d conf.get(\u0027devices\u0027, \u0027/srv/node\u0027)\n        self.mount_check \u003d config_true_value(conf.get(\u0027mount_check\u0027, \u0027true\u0027))\n        self.bind_ip \u003d conf.get(\u0027bind_ip\u0027, \u00270.0.0.0\u0027)\n        self.port \u003d int(conf.get(\u0027bind_port\u0027, self.default_port))\n        concurrency \u003d int(conf.get(\u0027concurrency\u0027, 8))\n        self.cpool \u003d GreenPool(size\u003dconcurrency)\n```","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"}],"test/unit/container/test_sharder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ab8e61c013d67bb105be98a006697e2b0da689a1","unresolved":true,"context_lines":[{"line_number":594,"context_line":"                    if secs is not sharder.stats_interval:"},{"line_number":595,"context_line":"                        return sleep(secs)"},{"line_number":596,"context_line":"                    can_do_stats.wait()"},{"line_number":597,"context_line":"                    can_do_stats.clear()"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"                with mock.patch("},{"line_number":600,"context_line":"                        \u0027swift.container.sharder.time.sleep\u0027) as mock_sleep:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a75849c_55b24980","line":597,"updated":"2025-11-10 16:47:08.000000000","message":"well, kudos for finding something that will work, but I\u0027m finding this a bit hard to follow.  There\u0027s a couple of smells:\n\n1) `sleep` is a non-obvious place to coordinate multiple actors: Who is calling sleep?\n2) how is there no *behavior* between wait and clear - what are we \"waiting to do\"?","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"0e074ff603648becfae27b42c8dcec759858957e","unresolved":true,"context_lines":[{"line_number":594,"context_line":"                    if secs is not sharder.stats_interval:"},{"line_number":595,"context_line":"                        return sleep(secs)"},{"line_number":596,"context_line":"                    can_do_stats.wait()"},{"line_number":597,"context_line":"                    can_do_stats.clear()"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"                with mock.patch("},{"line_number":600,"context_line":"                        \u0027swift.container.sharder.time.sleep\u0027) as mock_sleep:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5356518d_0d4ac3b7","line":597,"in_reply_to":"9a75849c_55b24980","updated":"2025-11-10 19:01:08.000000000","message":"Thanks for pointing that out, I checked again and since the only place we really call (eventlet) sleep in the sharder is during heartbeat, the non-stats_interval sleep check doesn\u0027t actually get called. The reconstructor is where the check is actually useful since eventlet.sleep has more uses outside of the heartbeat there.\n\n\nWhat I understand about the time between wait() and clear(), though, is that when wait\u0027s called it just makes the heartbeat thread to stall at that wait call until we call set() on the thread on the third broker, which gives the heartbeat thread a chance to continue and report stats on the third broker. then we clear() to reset it for future calls","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ab8e61c013d67bb105be98a006697e2b0da689a1","unresolved":true,"context_lines":[{"line_number":605,"context_line":"                                \u0027swift.container.sharder.dump_recon_cache\u0027,"},{"line_number":606,"context_line":"                                mock_dump_recon_cache):"},{"line_number":607,"context_line":"                            with mock.patch(\u0027swift.container.sharder.sleep\u0027,"},{"line_number":608,"context_line":"                                            fake_eventlet_sleep):"},{"line_number":609,"context_line":"                                fake_time.return_value \u003d ("},{"line_number":610,"context_line":"                                    time_progression[0])"},{"line_number":611,"context_line":"                                sharder._process_broker \u003d ("}],"source_content_type":"text/x-python","patch_set":1,"id":"775627d6_f960406f","line":608,"updated":"2025-11-10 16:47:08.000000000","message":"this is not idiomatic spelling of multiple nested `with` statements, please prefer:\n\n```\nwith A() as a, B() as b:\n    SUITE\n```\n\nhttps://docs.python.org/3/reference/compound_stmts.html#the-with-statement","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"0e074ff603648becfae27b42c8dcec759858957e","unresolved":false,"context_lines":[{"line_number":605,"context_line":"                                \u0027swift.container.sharder.dump_recon_cache\u0027,"},{"line_number":606,"context_line":"                                mock_dump_recon_cache):"},{"line_number":607,"context_line":"                            with mock.patch(\u0027swift.container.sharder.sleep\u0027,"},{"line_number":608,"context_line":"                                            fake_eventlet_sleep):"},{"line_number":609,"context_line":"                                fake_time.return_value \u003d ("},{"line_number":610,"context_line":"                                    time_progression[0])"},{"line_number":611,"context_line":"                                sharder._process_broker \u003d ("}],"source_content_type":"text/x-python","patch_set":1,"id":"52e24c83_2352ca08","line":608,"in_reply_to":"775627d6_f960406f","updated":"2025-11-10 19:01:08.000000000","message":"Acknowledged","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ab8e61c013d67bb105be98a006697e2b0da689a1","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            check_recon(recon_data[0], sum(time_progression[1:3]),"},{"line_number":695,"context_line":"                        sum(time_progression[:3]), fake_stats)"},{"line_number":696,"context_line":"            # periodic stats report after first broker has been visited during"},{"line_number":697,"context_line":"            # second cycle - one candidate identified so far this cycle"},{"line_number":698,"context_line":"            fake_stats.update({\u0027visited\u0027: {\u0027attempted\u0027: 1, \u0027skipped\u0027: 0,"},{"line_number":699,"context_line":"                                           \u0027success\u0027: 1, \u0027failure\u0027: 0,"},{"line_number":700,"context_line":"                                           \u0027completed\u0027: 0}})"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ceb4349_ce99269f","line":697,"updated":"2025-11-10 16:47:08.000000000","message":"so the test is trying to orchestrate that the 2nd cycle is the only one \"long\" enough to generate periodic stats\n\nI feel like there should be an easier way to spell this, sort of like:\n\n```\nif len(mock_process_calls) \u003d\u003d when_we_want_to_do_periodic_stats:\n    sleeping_stats_event.set()\n```\n\ngiven\n```\n            # four cycles are started, two brokers visited per cycle, but\n            # fourth never completes\n            self.assertEqual(8, len(fake_process_broker_calls))\n```\nI sort of imagine `when_we_want_to_do_periodic_stats \u003d 3`","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"0e074ff603648becfae27b42c8dcec759858957e","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            check_recon(recon_data[0], sum(time_progression[1:3]),"},{"line_number":695,"context_line":"                        sum(time_progression[:3]), fake_stats)"},{"line_number":696,"context_line":"            # periodic stats report after first broker has been visited during"},{"line_number":697,"context_line":"            # second cycle - one candidate identified so far this cycle"},{"line_number":698,"context_line":"            fake_stats.update({\u0027visited\u0027: {\u0027attempted\u0027: 1, \u0027skipped\u0027: 0,"},{"line_number":699,"context_line":"                                           \u0027success\u0027: 1, \u0027failure\u0027: 0,"},{"line_number":700,"context_line":"                                           \u0027completed\u0027: 0}})"}],"source_content_type":"text/x-python","patch_set":1,"id":"c3cc1c85_afc7109c","line":697,"in_reply_to":"1ceb4349_ce99269f","updated":"2025-11-10 19:01:08.000000000","message":"I had actually originally put it together that way but wanted to be a bit more verbose with how it functioned based on elapsed time; corrected!","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ab8e61c013d67bb105be98a006697e2b0da689a1","unresolved":true,"context_lines":[{"line_number":885,"context_line":"            # container c1 always came first in the list (n\u003d20) reruns"},{"line_number":886,"context_line":"            # but now that it\u0027s concurrent I feel like this should be tested"},{"line_number":887,"context_line":"            # d9fferently than this is currently... race conditions"},{"line_number":888,"context_line":"            # and all that."},{"line_number":889,"context_line":"            # also not certain why none of the other in_progress_stats need"},{"line_number":890,"context_line":"            # changing in the test given that this one oes"},{"line_number":891,"context_line":"            expected_in_progress_stats \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"2e42d4c0_addef422","line":888,"updated":"2025-11-10 16:47:08.000000000","message":"cooperative concurrency can be more deterministic than you might expect, the \"error\" container probably doesn\u0027t get yield as much b/c it raises an exception","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"},{"author":{"_account_id":38368,"name":"Christian Ohanaja","display_name":"Christian Ohanaja","email":"cohanaja@nvidia.com","username":"cohanaja"},"change_message_id":"178c2f58314f6bbe680d0670493edbffe50ad854","unresolved":false,"context_lines":[{"line_number":885,"context_line":"            # container c1 always came first in the list (n\u003d20) reruns"},{"line_number":886,"context_line":"            # but now that it\u0027s concurrent I feel like this should be tested"},{"line_number":887,"context_line":"            # d9fferently than this is currently... race conditions"},{"line_number":888,"context_line":"            # and all that."},{"line_number":889,"context_line":"            # also not certain why none of the other in_progress_stats need"},{"line_number":890,"context_line":"            # changing in the test given that this one oes"},{"line_number":891,"context_line":"            expected_in_progress_stats \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"3757db34_50bf4fcc","line":888,"in_reply_to":"2e42d4c0_addef422","updated":"2025-11-10 19:03:42.000000000","message":"Acknowledged","commit_id":"cea5dba958294124ab5d7c9573699d985048cbc8"}]}