Re: [collectd] New aggregator plugin basic_aggregator (#136)
On 10/13/12 04:27 AM, Sebastian Harl wrote: Ack. I'm not sure, though, if it's a good idea to implement that before most/many/all parts of collectd actually support reconfigure as people might expect too much from SIGHUP else Just to toss this out there... as I mentioned on a different thread, collectd is already (and frequently) calling each plugin's init() function. It does this every time update_kstat() is called. In fact, I think I've just run into another crash in cpu.c where numcpu is being messed about with inside init() while cpu_read() is using it for a calculation. So if it influences the debate any, it might be good to push the reconfigure notion, even if some plugins get left behind... because to a certain extent, it's happening already and some of the plugins don't deal well with it at all. Might as well get the benefit along with the pain. ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
Re: [collectd] New aggregator plugin basic_aggregator (#136)
On Sat, Oct 13, 2012 at 09:27:25AM +0200, Sebastian Harl wrote: most/many/all parts of collectd actually support reconfigure as people might expect too much from SIGHUP else. agreed Just to be sure: the main collectd thread has no control over its child threads in the current implementation? ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
Re: [collectd] New aggregator plugin basic_aggregator (#136)
Hi, On Sat, Oct 13, 2012 at 10:51:34AM +0200, Fabien Wernli wrote: On Sat, Oct 13, 2012 at 09:27:25AM +0200, Sebastian Harl wrote: most/many/all parts of collectd actually support reconfigure as people might expect too much from SIGHUP else. agreed Just to be sure: the main collectd thread has no control over its child threads in the current implementation? As mentioned on IRC: the main collectd process controls the child threads by means of a pthread conditional variable and a boolean variable indicating whether the main process is still in its main loop. The former may be used to signal the child threads to re-check the read loop condition. HTH, Sebastian -- Sebastian tokkee Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/ Those who would give up Essential Liberty to purchase a little Temporary Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin signature.asc Description: Digital signature ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
Re: [collectd] New aggregator plugin basic_aggregator (#136)
On 11 October 2012 18:11, Sebastian Harl s...@tokkee.org wrote: Now, my idea is to introduce a reconfigure callback that allows to reconfigure a single plugin. This could then be exposed, for example, through the 'unixsock' plugin. Example: /* plugin.h */ int plugin_register_reconfig (const char *name, int (*callback) (oconfig_item_t *)); /* UNIXSOCK */ RECONFIGURE PluginName Obviously, reconfigure will fail if the specified plugin did not register a reconfig callback. In case, the plugin did not specify a config callback either, reconfigure could be a no-op (no error). Internally, the callback would trigger re-parsing the whole collectd.conf file. Then, the appropriate config-block would be dispatched to the registered reconfig callback just in the same way that the original configuration was dispatched to the config callback. As a second step we could then think about also implementing a reload action. This would mean unloading and reloading the shared object of a plugin and then doing a reconfigure. This approach will allow us to introduce a global reload operation step by step. Also, if we decide never to implement a global operation, some plugins (like the aggregator) will still be able to benefit from the infrastructure. The implication here is that a reload wouldn't work unless all plugins implement a reconfigure callback. Unless a cookie cutter approach can be devised, it would take a fair amount of development effort to implement the callback across all the collectd plugins. This would limit the usefulness of the reload command. Maybe as a stepping stone to having the reconfigure callback implemented everywhere, the reload command could just skip over plugins that don't have the callback, and output a list of plugins that have actually been reconfigured. The operator would then have some insight into which plugins have been reconfigured, and why they may not see their configuration change take effect. What do others think? I think the idea is great. A reload command would certainly help when you've got big installations with lots of plugins loaded, or plugins that lag when stopping and starting (such as the RRDtool plugin). Cheers, Lindsay -- w: http://holmwood.id.au/~lindsay/ t: @auxesis ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
Re: [collectd] New aggregator plugin basic_aggregator (#136)
Hi, Adding a configuration reload is an excellent idea, and is one of the big missing features of collectd IMHO. I quite agree with Lindsay's remarks: there should simply be a warning for the plugins that don't support reconfiguration. On the other hand, maybe there should be the option to restart the thread, if it doesn't register the cb. just my 2 euros Cheers ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
Re: [collectd] New aggregator plugin basic_aggregator (#136)
Hi, On Wed, Oct 10, 2012 at 11:28:05PM -0700, ymettier wrote: After some use of the aggregator, I noticed that when you change something in the configuration of the aggregator (add,remove a hostname, type...), you have to restart all collectd to take the change into account. This new commit introduce a new feature : there is a distinct configuration file for the aggregator and when it changes, the aggregator notices it and updates its configuration. No more need to restart anything. Hrm, from a first glance, I don't really like this idea. The following are a couple of random ideas / notes; maybe we can get to a better (more generic) solution from there. Also, I've Cc'ed the mailing list. Imho, the discussion should happen there. Background: currently, collectd does not support reloading any configuration at run-time. This is due mostly to the rather generic configuration approach. Support for config reloading would probably require modifying each single plugin. Now, my idea is to introduce a reconfigure callback that allows to reconfigure a single plugin. This could then be exposed, for example, through the 'unixsock' plugin. Example: /* plugin.h */ int plugin_register_reconfig (const char *name, int (*callback) (oconfig_item_t *)); /* UNIXSOCK */ RECONFIGURE PluginName Obviously, reconfigure will fail if the specified plugin did not register a reconfig callback. In case, the plugin did not specify a config callback either, reconfigure could be a no-op (no error). Internally, the callback would trigger re-parsing the whole collectd.conf file. Then, the appropriate config-block would be dispatched to the registered reconfig callback just in the same way that the original configuration was dispatched to the config callback. As a second step we could then think about also implementing a reload action. This would mean unloading and reloading the shared object of a plugin and then doing a reconfigure. This approach will allow us to introduce a global reload operation step by step. Also, if we decide never to implement a global operation, some plugins (like the aggregator) will still be able to benefit from the infrastructure. What do others think? Cheers, Sebastian -- Sebastian tokkee Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/ Those who would give up Essential Liberty to purchase a little Temporary Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
