https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/engine.py File plaso/multi_processing/engine.py (right): https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/engine... plaso/multi_processing/engine.py:32: # TODO: change to runtime configurable setting. Lets do this in this CL, I can see this being something users might need to tinker with, especially if there are some large files and custom parsers (not in the plaso source tree) that people are using. https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/engine... plaso/multi_processing/engine.py:139: os.kill(pid, signal.SIGKILL) Replace with a call to _KillProcess. Please add some logging around this situation. Devs and users will want to know when this happens, how often it's happening etc. https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/psort.py File plaso/multi_processing/psort.py (right): https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/psort.... plaso/multi_processing/psort.py:29: # TODO: change to runtime configurable setting. As before, lets add this in this CL. https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/psort.... plaso/multi_processing/psort.py:202: os.kill(pid, signal.SIGKILL) Use _KillProcess instead.
Code updated.
https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/engine.py File plaso/multi_processing/engine.py (right): https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/engine... plaso/multi_processing/engine.py:32: # TODO: change to runtime configurable setting. As discussed this will need the config changes CL then first. https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/engine... plaso/multi_processing/engine.py:139: os.kill(pid, signal.SIGKILL) good point. https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/psort.py File plaso/multi_processing/psort.py (right): https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/psort.... plaso/multi_processing/psort.py:29: # TODO: change to runtime configurable setting. On 2017/01/13 12:45:49, onager wrote: > As before, lets add this in this CL. Acknowledged. https://codereview.appspot.com/318260043/diff/1/plaso/multi_processing/psort.... plaso/multi_processing/psort.py:202: os.kill(pid, signal.SIGKILL) On 2017/01/13 12:45:49, onager wrote: > Use _KillProcess instead. Done.
Couple of docstring nits, LGTM https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/en... File plaso/multi_processing/engine.py (right): https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/en... plaso/multi_processing/engine.py:115: logging.debug(( I don't know that debug is the right level for this message, I think we want users to notice this. It is an error, after all. https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ps... File plaso/multi_processing/psort.py (right): https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ps... plaso/multi_processing/psort.py:194: logging.debug(( See previously regarding verbosity level here. https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ps... plaso/multi_processing/psort.py:566: allowed to consume, where None represents 2 GiB. Represents the default memory limit, not 2 GiB. https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ta... File plaso/multi_processing/task_engine.py (right): https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ta... plaso/multi_processing/task_engine.py:683: allowed to consume, where None represents 2 GiB. -2GiB + the default worker memory limit Or something similar.
https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/en... File plaso/multi_processing/engine.py (right): https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/en... plaso/multi_processing/engine.py:115: logging.debug(( users will notice a worker getting killed in the top view. I'll change it to logging.warning but that is not very noticeable either. https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ps... File plaso/multi_processing/psort.py (right): https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ps... plaso/multi_processing/psort.py:194: logging.debug(( On 2017/01/18 07:14:32, onager wrote: > See previously regarding verbosity level here. Acknowledged. https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ps... plaso/multi_processing/psort.py:566: allowed to consume, where None represents 2 GiB. On 2017/01/18 07:14:32, onager wrote: > Represents the default memory limit, not 2 GiB. Done. https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ta... File plaso/multi_processing/task_engine.py (right): https://codereview.appspot.com/318260043/diff/80001/plaso/multi_processing/ta... plaso/multi_processing/task_engine.py:683: allowed to consume, where None represents 2 GiB. On 2017/01/18 07:14:32, onager wrote: > -2GiB + the default worker memory limit > > Or something similar. Done.
Changes have been merged with master branch. To close the review and clean up the feature branch you can run: python ./utils/review.py close memlimit
