----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57344/#review168289 -----------------------------------------------------------
Fix it, then Ship it! ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 47 (patched) <https://reviews.apache.org/r/57344/#comment240485> Is this eagerly created `Process` used anywhere? Could it be replaced with `None`? ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 142 (patched) <https://reviews.apache.org/r/57344/#comment240486> Won't this set `timedOutEvent` even if out queue loop was terminated due to `mustDieEvent`? ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 236-237 (patched) <https://reviews.apache.org/r/57344/#comment240484> Could this be simplified by putting `not self.worker_process` as 2nd and `not self.worker_process.is_alive()` as 3rd OR condition, this way omitting `self.worker_process and`? ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 268-272 (patched) <https://reviews.apache.org/r/57344/#comment240482> The comment and log message state the same (ie. "died gracefully"), but they are on different branches of the `if`. I guess `process_logs()` and the commend belong to the `else` branch instead. - Attila Doroszlai On March 7, 2017, 4:41 p.m., Eugene Chekanskiy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57344/ > ----------------------------------------------------------- > > (Updated March 7, 2017, 4:41 p.m.) > > > Review request for Ambari, Attila Doroszlai, Andrew Onischuk, Dmytro Sen, > Robert Levas, and Sebastian Toader. > > > Bugs: AMBARI-20323 > https://issues.apache.org/jira/browse/AMBARI-20323 > > > Repository: ambari > > > Description > ------- > > All status command execution logic moved under one file. > > * added protection for hardkilling(now process tries to die gracefully) > * added protection for queue internal logic blocking(underlying pipes hard > closed) > * added lock around queues that will prevent from using queues before new one > will be created in case of recreating executor process > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/ActionQueue.py 5300b52 > ambari-agent/src/main/python/ambari_agent/Controller.py 61a74e6 > ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 2f15770 > ambari-agent/src/main/python/ambari_agent/main.py 3f333c4 > ambari-agent/src/test/python/ambari_agent/TestActionQueue.py 9fefefb > ambari-agent/src/test/python/ambari_agent/TestController.py 663e215 > ambari-agent/src/test/python/ambari_agent/TestMain.py 97c448b > > > Diff: https://reviews.apache.org/r/57344/diff/4/ > > > Testing > ------- > > mvn clean test, manual testing > > > Thanks, > > Eugene Chekanskiy > >
