)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e8857c591a09eb734e1171d8aa2b3effd88b8188","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"* Stop logging in the signal handler. Logging may cause us to block on"},{"line_number":10,"context_line":"  the shared handler lock; if whoever was holding it was blocked waiting"},{"line_number":11,"context_line":"  on another lock held by the main thread, we deadlock."},{"line_number":12,"context_line":"* In cleanup, signal all children at approximately the same time with"},{"line_number":13,"context_line":"  `os.kill(p, signal.SIGTERM)` instead of looping through children"},{"line_number":14,"context_line":"  individually. This has also ensures that grand-child processes (such"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3b836a18_469b2e81","line":11,"updated":"2022-05-02 06:24:00.000000000","message":"IDK -- raising some special error feels suspect, too. What about any finally blocks we go blasting through? What if *they* log?\n\nMaybe we need to at least fire the killpg() inside the handler? I\u0027m a little worried about the cross-process properties of the PipeMutex, though... what happens if we kill children while one of them holds the lock?","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ccf8e2b2b6d060d49867fa9747238ef4eb6bd603","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5d650297_4d8a0fb3","updated":"2022-05-04 06:48:00.000000000","message":"There are some pep issues. But other then that looks pretty cool. I might spawn up some workers and have a play, will get back the this patch afterwards.","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"}],"swift/common/daemon.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4d49b51694c88325c6d072e1f642d08bd5b0f68c","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        utils.capture_stdio(self.logger, **kwargs)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        def kill_children(signum, frame):"},{"line_number":151,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_IGN)"},{"line_number":152,"context_line":"            raise DaemonExit(signum)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        signal.signal(signal.SIGTERM, kill_children)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6dd8595c_28ef986b","line":151,"updated":"2022-05-03 17:02:37.000000000","message":"Maybe ought to still call killpg() here -- who knows how \u0026 where we might get stuck between wherever the signal\u0027s caught and the main run loop. At least the signal will have gone out.","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ccf8e2b2b6d060d49867fa9747238ef4eb6bd603","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        utils.capture_stdio(self.logger, **kwargs)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        def kill_children(signum, frame):"},{"line_number":151,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_IGN)"},{"line_number":152,"context_line":"            raise DaemonExit(signum)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        signal.signal(signal.SIGTERM, kill_children)"}],"source_content_type":"text/x-python","patch_set":1,"id":"93929e1a_87e7b14d","line":151,"in_reply_to":"6dd8595c_28ef986b","updated":"2022-05-04 06:48:00.000000000","message":"Yeah, interesting thought. I see you add a pgkill down in cleanup, so already doing it in the normal case, adding it here will only do it when the parent process gets the SIGTERM, so thinking yeah it makes sense to me. But I might need to re-grok signal handling some more before I\u0027m happy to give a big +2.\n\nDoes calling a pgkill here trigger itself or only it\u0027s children, and is there any other cleanup itself need to do. I assume note seeing as we used to run os._exit which avoids any finallys etc.","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4d49b51694c88325c6d072e1f642d08bd5b0f68c","unresolved":true,"context_lines":[{"line_number":235,"context_line":"    def _run(self, once, **kwargs):"},{"line_number":236,"context_line":"        self.ask_daemon_to_prepare_workers(once, **kwargs)"},{"line_number":237,"context_line":"        if not self.unspawned_worker_options:"},{"line_number":238,"context_line":"            return self._run_inline(once, **kwargs)"},{"line_number":239,"context_line":"        for per_worker_options in self.iter_unspawned_workers():"},{"line_number":240,"context_line":"            if self._fork(once, **per_worker_options) \u003d\u003d 0:"},{"line_number":241,"context_line":"                return 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"bb0ca537_9e8b81b6","line":238,"updated":"2022-05-03 17:02:37.000000000","message":"Off-topic: WTF is up with these returns? _run_inline returns None, all the explicit returns here return 0... and as best I can tell, nothing actually *looks at* the return!","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ccf8e2b2b6d060d49867fa9747238ef4eb6bd603","unresolved":true,"context_lines":[{"line_number":235,"context_line":"    def _run(self, once, **kwargs):"},{"line_number":236,"context_line":"        self.ask_daemon_to_prepare_workers(once, **kwargs)"},{"line_number":237,"context_line":"        if not self.unspawned_worker_options:"},{"line_number":238,"context_line":"            return self._run_inline(once, **kwargs)"},{"line_number":239,"context_line":"        for per_worker_options in self.iter_unspawned_workers():"},{"line_number":240,"context_line":"            if self._fork(once, **per_worker_options) \u003d\u003d 0:"},{"line_number":241,"context_line":"                return 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c66e4ee_a23764fb","line":238,"in_reply_to":"bb0ca537_9e8b81b6","updated":"2022-05-04 06:48:00.000000000","message":"Lol, yeah, that does sound kinda nuts. We should maybe return what the daemon wants. like:\n\n  return self.daemon.run(..)","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4d49b51694c88325c6d072e1f642d08bd5b0f68c","unresolved":true,"context_lines":[{"line_number":256,"context_line":"        return 0"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    def cleanup(self):"},{"line_number":259,"context_line":"        if not self.spawned_pids():"},{"line_number":260,"context_line":"            return"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        def timeout(signum, frame):"}],"source_content_type":"text/x-python","patch_set":1,"id":"80e6712a_3cedf5ab","line":259,"updated":"2022-05-03 17:02:37.000000000","message":"If we include the killpg() in the signal handler, we probably want a check_on_all_running_workers() before this early exit.","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4d49b51694c88325c6d072e1f642d08bd5b0f68c","unresolved":true,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        signal.signal(signal.SIGTERM, signal.SIG_IGN)"},{"line_number":266,"context_line":"        os.killpg(0, signal.SIGTERM)"},{"line_number":267,"context_line":"        signal.signal(signal.SIGALRM, timeout)"},{"line_number":268,"context_line":"        signal.setitimer(signal.ITIMER_REAL, self.daemon.CLEANUP_TIMEOUT)"},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            while True:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a9e2a65e_f86d9c0a","line":267,"updated":"2022-05-03 17:02:37.000000000","message":"Should I be using signals, or eventlet timeouts?","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ccf8e2b2b6d060d49867fa9747238ef4eb6bd603","unresolved":true,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        signal.signal(signal.SIGTERM, signal.SIG_IGN)"},{"line_number":266,"context_line":"        os.killpg(0, signal.SIGTERM)"},{"line_number":267,"context_line":"        signal.signal(signal.SIGALRM, timeout)"},{"line_number":268,"context_line":"        signal.setitimer(signal.ITIMER_REAL, self.daemon.CLEANUP_TIMEOUT)"},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            while True:"}],"source_content_type":"text/x-python","patch_set":1,"id":"08eaab2a_fccb5f57","line":267,"in_reply_to":"a9e2a65e_f86d9c0a","updated":"2022-05-04 06:48:00.000000000","message":"good question, maybe I feel burnt so I\u0027d like to say avoid eventlet :P","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"}],"test/unit/common/test_daemon.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4d49b51694c88325c6d072e1f642d08bd5b0f68c","unresolved":true,"context_lines":[{"line_number":423,"context_line":"            (\u0027mock-pid-1\u0027, signal.SIGKILL),"},{"line_number":424,"context_line":"            (\u0027mock-pid-2\u0027, signal.SIGKILL),"},{"line_number":425,"context_line":"        ]))"},{"line_number":426,"context_line":"        self.assertEqual(strategy.unspawned_worker_options, ["},{"line_number":427,"context_line":"            {\u0027mock_options\u0027: True},"},{"line_number":428,"context_line":"            {\u0027mock_options\u0027: False}])"},{"line_number":429,"context_line":"        self.assertEqual(strategy.options_by_pid, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"942877e4_9a2d08f9","line":426,"updated":"2022-05-03 17:02:37.000000000","message":"Need to compare sets here.","commit_id":"299ab0ce63c362954bea5c21d34b7c20b22d9b65"}]}