)]}'
{"zuul/cmd/executor.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"93b16f277dcec6134830e2415f107147a7574056","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        while True:"},{"line_number":46,"context_line":"            time.sleep(5)"},{"line_number":47,"context_line":"            pid, rc \u003d os.waitpid(self._log_streamer_pid, os.WNOHANG)"},{"line_number":48,"context_line":"            if pid \u003d\u003d self._log_streamer_pid:"},{"line_number":49,"context_line":"                # Exit and force system level service management to restart"},{"line_number":50,"context_line":"                # the service."},{"line_number":51,"context_line":"                self.log.error(\"Log streaming proxy died. Stopping Executor.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_a9002302","line":48,"updated":"2019-08-23 07:04:50.000000000","message":"does this need to be sort of registered for the shutdown path (in executor/server.py)?  otherwise maybe this jumps in an kills the executor when it\u0027s shutting down?","commit_id":"e725af152631483db14c8d146108161f95770681"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e57f4170778c49a8b62bec5d2f4ae2a141a8fc1f","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        while True:"},{"line_number":46,"context_line":"            time.sleep(5)"},{"line_number":47,"context_line":"            pid, rc \u003d os.waitpid(self._log_streamer_pid, os.WNOHANG)"},{"line_number":48,"context_line":"            if pid \u003d\u003d self._log_streamer_pid:"},{"line_number":49,"context_line":"                # Exit and force system level service management to restart"},{"line_number":50,"context_line":"                # the service."},{"line_number":51,"context_line":"                self.log.error(\"Log streaming proxy died. Stopping Executor.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_ed485d8b","line":48,"in_reply_to":"7faddb67_544fc3bd","updated":"2019-08-23 18:06:22.000000000","message":"The way the fork is done below that race should be avoided. The child log streamer process waits for the parent\u0027s pipe to close then shuts itself down. That should avoid this race. However, if we want belts and suspenders for this we can add a stop flag that the shutdown of the executor sets.","commit_id":"e725af152631483db14c8d146108161f95770681"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"32a396790c8155b472cc1675258e678b4eea82fd","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        while True:"},{"line_number":46,"context_line":"            time.sleep(5)"},{"line_number":47,"context_line":"            pid, rc \u003d os.waitpid(self._log_streamer_pid, os.WNOHANG)"},{"line_number":48,"context_line":"            if pid \u003d\u003d self._log_streamer_pid:"},{"line_number":49,"context_line":"                # Exit and force system level service management to restart"},{"line_number":50,"context_line":"                # the service."},{"line_number":51,"context_line":"                self.log.error(\"Log streaming proxy died. Stopping Executor.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_544fc3bd","line":48,"in_reply_to":"7faddb67_a9002302","updated":"2019-08-23 12:54:45.000000000","message":"I agree that we should include stopping this into the shutdown procedure. Otherwise there could be a race where this kicks in and terminates the executor non-gracefully.","commit_id":"e725af152631483db14c8d146108161f95770681"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"32a396790c8155b472cc1675258e678b4eea82fd","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                # the service."},{"line_number":51,"context_line":"                self.log.error(\"Log streaming proxy died. Stopping Executor.\")"},{"line_number":52,"context_line":"                # TODO does this shutdown need to be more graceful?"},{"line_number":53,"context_line":"                os._exit(2)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _start_log_streamer(self):"},{"line_number":56,"context_line":"        pipe_read, pipe_write \u003d os.pipe()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_74cd1f2f","line":53,"range":{"start_line":53,"start_character":16,"end_line":53,"end_character":27},"updated":"2019-08-23 12:54:45.000000000","message":"I think we should try to gracefully stop the executor to give it a chance to abort all jobs. As a fallback we can still directly exit after a grace period.","commit_id":"e725af152631483db14c8d146108161f95770681"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"58b92e6d613634f436d65a98eabc2bc0f9bbbdd3","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        while not self.stop_event.is_set():"},{"line_number":51,"context_line":"            if not self.stop_event.wait(5):"},{"line_number":52,"context_line":"                # Check that we havne\u0027t stopped yet"},{"line_number":53,"context_line":"                pid, rc \u003d os.waitpid(self._log_streamer_pid, os.WNOHANG)"},{"line_number":54,"context_line":"                if pid \u003d\u003d self._log_streamer_pid:"},{"line_number":55,"context_line":"                    # Exit and force system level service management to"},{"line_number":56,"context_line":"                    # restart the service."}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_3420e0f8","line":53,"updated":"2019-08-27 00:42:28.000000000","message":"It feels a bit like we\u0027re mixing ideas here ... the log streaming process is sitting in a read() on the pipe to notice if the parent dies.  but here we\u0027re checking with a waitpid() and sleep...\n\nif the child didn\u0027t close pipe_write, and we just did pipe_write.read() here for the blocking?  when that returns, the same checks for a shutdown situation v unexpected can occur?","commit_id":"609923e14f2e632def21cd249bfb883aeb9ddb61"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"58b92e6d613634f436d65a98eabc2bc0f9bbbdd3","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                    self.executor.stop()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _start_log_streamer(self):"},{"line_number":62,"context_line":"        pipe_read, pipe_write \u003d os.pipe()"},{"line_number":63,"context_line":"        child_pid \u003d os.fork()"},{"line_number":64,"context_line":"        if child_pid \u003d\u003d 0:"},{"line_number":65,"context_line":"            os.close(pipe_write)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_9428b4ee","line":62,"updated":"2019-08-27 00:42:28.000000000","message":"I know this is working with what we have ... I hesitate to descend into bikeshedding but this now does seem like we\u0027ve replicated what the multiprocessing package provides?  with the shutdown changes here, it seems like with multiprocessing you could start the child and then just join() it, then once that join() returns check the shutdown flag and if it\u0027s not set, die (unexpected) or if it is, just ignore?","commit_id":"609923e14f2e632def21cd249bfb883aeb9ddb61"}]}