----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57344/#review168101 -----------------------------------------------------------
ambari-agent/src/main/python/ambari_agent/Controller.py Line 208 (original), 205 (patched) <https://reviews.apache.org/r/57344/#comment240233> "INITIAL_START" might be confusing as the server may request the agent to re-register. Eg. the server was restarted, or agent lost heartbeat etc. ambari-agent/src/main/python/ambari_agent/Controller.py Lines 479-480 (patched) <https://reviews.apache.org/r/57344/#comment240225> Isn't this the same as ```self.statusCommandsExecutor.relaunch("INITIAL_START")``` in ```def registerWithServer(self):```? Would it make sense to create ```StatusCommandsExecutor``` in ```def registerWithServer(self):``` just before calling ```self.statusCommandsExecutor.relaunch("INITIAL_START")``` if hasn't been instantiated yet? ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Line 43 (original), 40 (patched) <https://reviews.apache.org/r/57344/#comment240226> Comment what this lock is used for. ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 51 (patched) <https://reviews.apache.org/r/57344/#comment240237> This doesn't seem to be used? ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 60 (patched) <https://reviews.apache.org/r/57344/#comment240227> Messages needs to be logged to ambari-agent.log as there are environments logs are collected from ambari-agent.log and published into a central log collector. Perhaps the parent process could read the out file and channel the content into ambari-agent.log. ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 94 (patched) <https://reviews.apache.org/r/57344/#comment240235> You could use the timed_out property. ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py Lines 158 (patched) <https://reviews.apache.org/r/57344/#comment240238> Could this deadlock in case the worker process is killed from outside thus being shut down without properly closing the mp_result_queue? ambari-agent/src/main/python/ambari_agent/main.py Lines 317-318 (original), 318-319 (patched) <https://reviews.apache.org/r/57344/#comment240236> How this ensures that status command executor is re-launched in case is killedd from oputside? - Sebastian Toader On March 6, 2017, 6:45 p.m., Eugene Chekanskiy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57344/ > ----------------------------------------------------------- > > (Updated March 6, 2017, 6:45 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/1/ > > > Testing > ------- > > mvn clean test, manual testing > > > Thanks, > > Eugene Chekanskiy > >
