)]}'
{"storlets/agent/daemon/manager.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d4af9388104555c722482b7ee52b6afc51176de1","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.pool_size \u003d pool_size"},{"line_number":63,"context_line":"        self.task_id_to_pid \u003d {}"},{"line_number":64,"context_line":"        self.chunk_size \u003d 16"},{"line_number":65,"context_line":"        self.shutdown_timeout \u003d 30"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _cleanup_pids(self):"},{"line_number":68,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_ea76ddd5","line":65,"range":{"start_line":65,"start_character":32,"end_line":65,"end_character":34},"updated":"2016-12-15 07:36:35.000000000","message":"Please let me make sure why this shutdown_timeout is hard-coded instead of using storlets_timeout?\n\nIf i understand correctly, that terminate is designed to be called at the end of dispatch command[1] (sorry, it\u0027s master not dependency patch). However, if we could give up waiting the children and no errors occurs due to exception\u003dFalse, we can never control the storlet app which can be running more than 30 seconds, right?\n\n\n1: https://github.com/openstack/storlets/blob/master/storlets/agent/daemon/manager.py#L327","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c360e54e30671d3d871c40a6ec3fa16ea23504a2","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.pool_size \u003d pool_size"},{"line_number":63,"context_line":"        self.task_id_to_pid \u003d {}"},{"line_number":64,"context_line":"        self.chunk_size \u003d 16"},{"line_number":65,"context_line":"        self.shutdown_timeout \u003d 30"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _cleanup_pids(self):"},{"line_number":68,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_05c3de89","line":65,"range":{"start_line":65,"start_character":32,"end_line":65,"end_character":34},"in_reply_to":"1a6eadb0_ea76ddd5","updated":"2016-12-16 01:39:36.000000000","message":"Currently we don\u0027t have any other mechanism to set configurable parameters about daemon factory and storlet daemon, than passing command line parameters.\nIMO it is better way to add some config files defining config parameters like shutdown_timeout, chunk_size and so on, than adding more and more command line parameters, but currently I leave it for future.\n\n\u003e we can never control the storlet app which can be running more than 30 seconds, right?\nYes, in storlet daemon case.\nIn that case, read timeout occurs in gateway side.","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d4af9388104555c722482b7ee52b6afc51176de1","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        terminated \u003d []"},{"line_number":72,"context_line":"        for task_id, daemon_pid in self.task_id_to_pid.iteritems():"},{"line_number":73,"context_line":"            try:"},{"line_number":74,"context_line":"                pid, status \u003d green_os.waitpid(daemon_pid, os.WNOHANG)"},{"line_number":75,"context_line":"                if os.WIFEXITED(status) or os.WIFSIGNALED(status):"},{"line_number":76,"context_line":"                    terminated.append(task_id)"},{"line_number":77,"context_line":"            except OSError as err:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_0fda4b17","line":74,"updated":"2016-12-15 07:36:35.000000000","message":"Looking at eventlet[1], we don\u0027t have to use eventlet.green.io here because we set os.WNOHUNG as option and it means no behavior difference from os.waitpid().\n\n\n1: https://github.com/eventlet/eventlet/blob/master/eventlet/green/os.py#L88-L89","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c360e54e30671d3d871c40a6ec3fa16ea23504a2","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        terminated \u003d []"},{"line_number":72,"context_line":"        for task_id, daemon_pid in self.task_id_to_pid.iteritems():"},{"line_number":73,"context_line":"            try:"},{"line_number":74,"context_line":"                pid, status \u003d green_os.waitpid(daemon_pid, os.WNOHANG)"},{"line_number":75,"context_line":"                if os.WIFEXITED(status) or os.WIFSIGNALED(status):"},{"line_number":76,"context_line":"                    terminated.append(task_id)"},{"line_number":77,"context_line":"            except OSError as err:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_8588ce72","line":74,"in_reply_to":"1a6eadb0_0fda4b17","updated":"2016-12-16 01:39:36.000000000","message":"Thank you for your information.\n\nIMO, using green_os for all places makes codes easy to read, without any deep checking eventlet spec, even if it is not useful here.","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d4af9388104555c722482b7ee52b6afc51176de1","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            return"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        try:"},{"line_number":118,"context_line":"            pid, status \u003d green_os.wait()"},{"line_number":119,"context_line":"            if os.WIFEXITED(status) or os.WIFSIGNALED(status):"},{"line_number":120,"context_line":"                self._remove_pid(pid)"},{"line_number":121,"context_line":"        except OSError as err:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_6f19ffd6","line":118,"updated":"2016-12-15 07:36:35.000000000","message":"(continue from previous comment)However, this wait is affected with eventlet.green.os. The diff is we can trampoline the thread if waiting the signal, right?\n\nAnd not sure if it works well the green_os if it\u0027s out of eventlet threads.","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c360e54e30671d3d871c40a6ec3fa16ea23504a2","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            return"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        try:"},{"line_number":118,"context_line":"            pid, status \u003d green_os.wait()"},{"line_number":119,"context_line":"            if os.WIFEXITED(status) or os.WIFSIGNALED(status):"},{"line_number":120,"context_line":"                self._remove_pid(pid)"},{"line_number":121,"context_line":"        except OSError as err:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_e54ee239","line":118,"in_reply_to":"1a6eadb0_6f19ffd6","updated":"2016-12-16 01:39:36.000000000","message":"As far as I understand, if we don\u0027t use green_os here, eventlet.Timeout never works because normal os.wait never trampolines.","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d4af9388104555c722482b7ee52b6afc51176de1","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"    def _terminate(self):"},{"line_number":276,"context_line":"        with Timeout(self.shutdown_timeout, exception\u003dFalse):"},{"line_number":277,"context_line":"            self._wait_all_child_processes()"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"def usage():"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_1eafdae2","line":277,"updated":"2016-12-15 07:36:35.000000000","message":"If we got timeout (absolutely optional from operators or users), should we kill all child processes?","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c360e54e30671d3d871c40a6ec3fa16ea23504a2","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"    def _terminate(self):"},{"line_number":276,"context_line":"        with Timeout(self.shutdown_timeout, exception\u003dFalse):"},{"line_number":277,"context_line":"            self._wait_all_child_processes()"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"def usage():"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_e5fc22ba","line":277,"in_reply_to":"1a6eadb0_1eafdae2","updated":"2016-12-16 01:39:36.000000000","message":"It seems more safe to me, even if child processes are automatically killed when its parent process is killed. will fix, thanks!","commit_id":"9e2ecd7f01598a83500f0866bc6f18ce481ec972"}]}