>>> On 12/7/2008 at 1:19 AM, in message <[EMAIL PROTECTED]>,
Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]> wrote:
> On Mon, Dec 01, 2008 at 12:19:25PM -0700, Brad Nicholes wrote:
>> I don't understand what you are trying to do with this patch.
>
> as explained in the commit message (which I apologize if it wasn't
> clear enough), it is correcting the definition of "modules" so it is
> correctly tagged as showing multiple times in the configuration as per :
>
> http://www.nongnu.org/confuse/manual/confuse_8h.html#a3
>
> of course, I also adjusted all in tree code to use the correct syntax
> to retrieve all module definitions and that way avoid any problems.
>
The documentation that is being referenced does not apply to the design or
functionality that was implemented. The implemented functionality was not
intended to be multiple modules sections. It was designed to be a single
modules section which is exactly how it was coded. As implemented, libconfuse
allows various modules sections to appear in different parts of the
configuration, but since the modules section is not defined as MULTI, it
automatically consolidates the sections. This is exactly the behavior that was
designed for and implemented. Changing the implementation of the section to
MULTI breaks the original design.
>> Once libconfuse has finished reading and parsing the entire
>> configuration file along with all of the individual modules sections,
>> it automatically consolidates them into a single section.
>
> since the "modules" section contains only other subsections (in this case
> the section "module") which is defined as showing multiple times then
> all those subsections will be linked to the first section created, and
> which will be then accessible with a call to cfg_getsec("modules").
>
> the problem with that, of course, is that we are then relying on an
> unintended sideffect of how the configuration structure is being created
> and that will break if another non section configuration is added later.
>
There is nothing that says that section consolidation is an unintended side
effect. In fact I would argue that the consolidation functionality is intended
simple because the end result of a non-multi section is a single consolidated
section which is exactly what a section that is marked as CFG_NONE is intended
to produce. The gmond design for the modules section requires a single
modules section that contains all of the individual module sections. It was
designed that way and implemented according to the design and in accordance
with libconfuse functionality.
> I'd also argue that if all "modules" sections are to be collapsed anyway
> wouldn't be better to get rid of the "modules" configuration and just
> list all modules as part of the "root"?
>
No, a modules section was intended to be a container for individual module
sections. This makes it very natural for an admin to group all of the module
sections together within a single .conf file as well as for included .conf
files. Therefore when anybody else wants to look for the definition of
individual modules, they are more likely to find them all defined within the
modules section of the .conf file. It also makes it very simple for the gmond
code or C based metric modules to reference a single modules section and
iterate through all of the individual modules. The end result is a code
pattern that is very intuitive. There was a lot of thought that went into this
design. If you want to discuss the design, we can certainly do that. But the
implementation of the design is exactly as it should be and altering the
implementation will break the design.
>> There is no need to try to scan individual modules sections.
>
> if the configuration is defined to be shown multiple times, then a call
> to cfg_getsec will only get one of the instances.
>
Which is exactly the intended design and implementation.
>> This code was working correctly as it was. Please revert this patch.
>
> seems it was reverted already in r1931, so added some documentation of
> the latent problems in r1933 until the compatibility issues raised could
> be resolved.
>
> either implementation will work for the current setup but if you are to
> reconsider don't forget to revert r1933 as well.
>
> fixing any external module that would have problems looking at the
> module list (most likely useful for script handlers) shouldn't been
> that difficult IMHO.
>
There is no problem with this code, its implementation, its functionality or
its design. Any change at this point will cause an unnecessary incompatibility
which no matter how difficult to fix, is still an incompatibility. There is no
problem with the code, there is no issue to be resolved. Rather than spending
time discussing a non-issue, lets move on to real problems.
Brad
------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you. Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/
_______________________________________________
Ganglia-developers mailing list
Ganglia-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ganglia-developers
