cmonitor
cmonitor copied to clipboard
CMonitor : Added inotify support to cmonitor.
CmonitorLauncher :
It will perform following steps:
-
- Watch all files below a directory and notify an event for changes.
-
- Retrieves all the process and extract the process name "/proc/
/stat.
- Retrieves all the process and extract the process name "/proc/
-
- check the process name against the white-list given in the filter list.
-
- Execute command to launch CMonitor if the process name matches with the filter.
Hi @satyabratabharati ,
I think this MR still needs some work. Major points to raise are:
- log file: we need this utility to write a log file using Python logging module so that we have each event time-stamped and available for debugging inside a logfile (--log CLI option could be used to define logging destination)
- unit tests: it should be possible to write some unit tests for CgroupWatcher asking a test instance to watch under /tmp/cmonitor_unit_test folder and then manipulating the filesystem there (with os.mkdir and writing a "tasks" file)... check out examples under e.g. tools/filter/tests
- clear sleep policy: each thread pool (the "watchers" and the "launchers") need to have a clear and well defined sleep policy: they need to drain their input queues (either the Inotify list of pending events or the list of items enqueued on "queue" ) as fast as possible and then return to sleep when all inputs have been processed
- function-level documentation, using Python triple quotes embedded docs, at least on public methods of the classes (all non public methods should start with double underscores)
- a README / documentation about the whole utility, with usage examples; I typically like to embed usage example in the --help output of each utility, see e.g. cmonitor_filter.py parse_command_line() function
- packaging: we need the Makefile "install" target to be able to install this new utility so it will be part of "cmonitor-tools" RPM
Generally speaking I think the design is good anyway: I like the idea of using ThreadPoolExecutor to watch on a folder and then launch (synchronoulsy) cmonitor instances. I admit however that's I should study how ThreadPoolExecutor actually works because I have a lot of doubts: a) the thread running CgroupWatcher in my opinion should be just 1: it's doing very small amount of work (using inotify recursively on a folder) and filtering out non-interesting events. Having more than 1 also is tricky: how do we ensure we never watch the same event (cgroup creation) twice? b) is the "queue" object thread-safe (it should be since Python is using GIL but again I'm not sure) c) why did you use "max_workers=5" ? How does ThreadPoolExecutor decide how many threads to spawn? d) where/how do we set the limit to the max instances of "cmonitor" can be running simultaneously?
Hi @satyabratabharati ,
I think this MR still needs some work. Major points to raise are:
- log file: we need this utility to write a log file using Python logging module so that we have each event time-stamped and available for debugging inside a logfile (--log CLI option could be used to define logging destination)
- unit tests: it should be possible to write some unit tests for CgroupWatcher asking a test instance to watch under /tmp/cmonitor_unit_test folder and then manipulating the filesystem there (with os.mkdir and writing a "tasks" file)... check out examples under e.g. tools/filter/tests
- clear sleep policy: each thread pool (the "watchers" and the "launchers") need to have a clear and well defined sleep policy: they need to drain their input queues (either the Inotify list of pending events or the list of items enqueued on "queue" ) as fast as possible and then return to sleep when all inputs have been processed
- function-level documentation, using Python triple quotes embedded docs, at least on public methods of the classes (all non public methods should start with double underscores)
- a README / documentation about the whole utility, with usage examples; I typically like to embed usage example in the --help output of each utility, see e.g. cmonitor_filter.py parse_command_line() function
- packaging: we need the Makefile "install" target to be able to install this new utility so it will be part of "cmonitor-tools" RPM
Generally speaking I think the design is good anyway: I like the idea of using ThreadPoolExecutor to watch on a folder and then launch (synchronoulsy) cmonitor instances. I admit however that's I should study how ThreadPoolExecutor actually works because I have a lot of doubts: a) the thread running CgroupWatcher in my opinion should be just 1: it's doing very small amount of work (using inotify recursively on a folder) and filtering out non-interesting events. Having more than 1 also is tricky: how do we ensure we never watch the same event (cgroup creation) twice? b) is the "queue" object thread-safe (it should be since Python is using GIL but again I'm not sure) c) why did you use "max_workers=5" ? How does ThreadPoolExecutor decide how many threads to spawn? d) where/how do we set the limit to the max instances of "cmonitor" can be running simultaneously?
Hi @f18m ,
I tried to implement all the points what you have suggested. Except for the unit test case from the above list. I will work on it.
Following points have been incorporated :
- logger for the logging events. example.log
- Done.
- regarding sleep policy, added sleep when the Queue is empty and there is no events to process. The sleep parameter can be provided now from the command line as a parameter. The default is set to 20 sec. The 20 sec sleep also used before processing the files ( meaning before applying whilte-list filter logic) , just to allow the task files to be created before applying the filter logic and store it to the Queue.
- Added function level comments as well for the classes.
- Added as suggested.
- Done.
Thank You.