)]}'
{"Engine/agent/storlet_daemon_factory/daemon_factory.py":[{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"8c3744df8d3ae02af2409a7882e1d48469378bbb","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            try:"},{"line_number":391,"context_line":"                self.process_kill(storlet_name)"},{"line_number":392,"context_line":"            except SDaemonError:"},{"line_number":393,"context_line":"                # TODO(takashi): Can we really pass here?"},{"line_number":394,"context_line":"                pass"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    def shutdown_all_processes(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_534df5c2","line":393,"updated":"2016-08-23 07:36:28.000000000","message":"Thinking...\nWe get here if we\n\n(1) os.kill / os.wait failed.\nos.kill may fail if  we have a permission problem, we specified a bad signal number (can\u0027t be) or the pid does not exist. If there is a permission problem we would not get here anyway. if the pid does not exist, then our did is done and we do not need to worry about it.\nos.wait will fail if\n(A) there is no child process or the process does not exist. In which case we can safely ignore the error\n(B) whooping was not specified - but we do specify it.\n\n(2) did not find a pid in the storlet-\u003epid dict.\nThis means that we have a bug where we have somehow deleted the entry from and still trying to kill the process. Ideally we would probably want to surface such a bug, but I am not sure how important this is...\n\nBottom line: I would add a TODO: raise if we did not find the pid in the storlet name -\u003e pid mapping.","commit_id":"f8f280c4453f9449bec2c713a1a04f23fb0acb46"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"1af711f91b7da5473849b1e6a86f76403ad9d7cf","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            try:"},{"line_number":391,"context_line":"                self.process_kill(storlet_name)"},{"line_number":392,"context_line":"            except SDaemonError:"},{"line_number":393,"context_line":"                # TODO(takashi): Can we really pass here?"},{"line_number":394,"context_line":"                pass"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    def shutdown_all_processes(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_57dde0de","line":393,"in_reply_to":"1ac06dbe_534df5c2","updated":"2016-08-23 07:51:49.000000000","message":"(1) os.kill / os.wait failed.\n\u003e os.kill may fail if  we have a permission problem, we specified a bad signal number (can\u0027t be) or the pid does not exist.\n\n\u003e If there is a permission problem we would not get here anyway.\n\nThis is not true. process_kill raises StorletDaemonError, and we get here.\n(No changes from the previous behavior)\nThis error can happen in the following situation.\n\n * started a storlet daemon\n * the storlet daemon terminates, and another process is comming with the same pid\n * storlet_daemon tries to get the status of the storlet daemon\n\n\u003e if the pid does not exist, then our did is done and we do not need to worry about it.\nagree.\n\n\u003e os.wait will fail if\n\u003e (A) there is no child process or the process does not exist. In which case we can safely ignore the error\n\nagree.\n\n\u003e (B) whooping was not specified - but we do specify it.\n\nThis can not happen, as you mentioned.\n\n\u003e (2) did not find a pid in the storlet-\u003epid dict.\n\u003e This means that we have a bug where we have somehow deleted the entry from and still trying to kill the process. Ideally we would probably want to surface such a bug, but I am not sure how important this is...\n\nCurrently daemon_factory works in a single process, and we can ensure its atomicity about pid handling. So if we face this error, there are unexpected bug about pid handling\nI agree with adding some logic to detect the bug phenometa, while I also think it is not so very important.","commit_id":"f8f280c4453f9449bec2c713a1a04f23fb0acb46"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"2b4d80bea9fcf1c9a6af1bb9fc62b13b5be94b1a","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            try:"},{"line_number":391,"context_line":"                self.process_kill(storlet_name)"},{"line_number":392,"context_line":"            except SDaemonError:"},{"line_number":393,"context_line":"                # TODO(takashi): Can we really pass here?"},{"line_number":394,"context_line":"                pass"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    def shutdown_all_processes(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_b41fa0b5","line":393,"in_reply_to":"1ac06dbe_57dde0de","updated":"2016-08-24 07:56:00.000000000","message":"I guess what I meant to say with \"we would not get here anyway\" is not getting to line 393, but rather if we have a permission problem we would probably won\u0027t be able to establish the connection via the named pipe and the execution will not get to call process_kill_all from the first place...","commit_id":"f8f280c4453f9449bec2c713a1a04f23fb0acb46"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"22a0890da4dbacb5ad53cc2ce4139a13fa0a0e68","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            try:"},{"line_number":391,"context_line":"                self.process_kill(storlet_name)"},{"line_number":392,"context_line":"            except SDaemonError:"},{"line_number":393,"context_line":"                # TODO(takashi): Can we really pass here?"},{"line_number":394,"context_line":"                pass"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    def shutdown_all_processes(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_42af772b","line":393,"in_reply_to":"1ac06dbe_b41fa0b5","updated":"2016-08-24 14:14:01.000000000","message":"Oh, sorry for my mistake. Anyway, as far as we currently guess, this exception rarely happens in real cases, without any bug phenomena. I\u0027ll submit a follow-up patch, to stop eating up exceptions here, to enable us to find easily that bug happens in the environment,","commit_id":"f8f280c4453f9449bec2c713a1a04f23fb0acb46"}]}