On Fri, Sep 12, 2008 at 03:57:37PM -0400, Jesse Becker wrote: > On Fri, Sep 12, 2008 at 13:33, Carlo Marcelo Arenas Belon > > > the following commit (r1754 in ganglia's svn) seems to be patching the fix > > proposed by Jason as part of BUG37 and that was committed in r1595 and has > > been left unconsistent (as not all uses of this feature has been converted > > to use /dev/null). > > Then the other instances should be converted, IMO.
Committed revision 1760. > I looked at this > one specifically since it was causing trouble on a local install > running trunk. Specifically, the error was: > > PHP Notice: Undefined offset: 1 in > /home/beckerjes/ganglia/versions/trunk/monitor-core/web-working/functions.php > on line 266, referer: > http://saturn/ganglia-jb/?m=load_one&r=hour&s=descending&hc=4&mc=2 then the commit message was incorrect, because the workaround wasn't to a problem with rrdtool (which is what was committed in 1585) but to the PHP code that is using exec and failing to handle null output correctly. > After adding a one-line print let me see exactly the rrdtool command > getting called (rrdtool 1.2.26): > > /usr/bin/rrdtool graph '' --start -3600 --end N > DEF:avg='/long/path/to/rrds/__SummaryInfo__/cpu_num.rrd':'sum':AVERAGE > PRINT:avg:AVERAGE:%.2lf > > (Or something to that effect, the test system is at home, and I'm not. > I'm building this on the fly from the code.) > > Sure enough, no output at all, and this causes line 266 to throw the > error. I can reproduce this using rrdtool 1.2.23 and 1.2.26. This > was specifically in the meta-view, but not in the cluster- or > host-views. do you mean there is no output at all in STDOUT?, still can't reproduce it with 1.2.27 or 1.3.2 (both in amd64 linux): $ rrdtool graph '' --start -3600 --end N DEF:avg='/var/lib/ganglia/rrds/__SummaryInfo__/cpu_num.rrd':'sum':AVERAGE PRINT:avg:AVERAGE:%.2lf 0x0 nan > > I had been unable to reproduce any problem with the original patch using > > several different versions of rrdtool, but your comment seems to imply you > > had observed the problem somehow, could you elaborate on that? > > I tested three cases: > rrdtool - > rrdtool '' > rrdtool /dev/null > > The only one that I could get to work consistently was 'rrdtool > /dev/null'. This "trick" of using /dev/null is actually suggested in > a number of rrdtool mailing list threads for situations where you want > rrdtool to calculate a value, but suppress the generation of a graph. as I pointed out was also suggested in some ganglia developer list. > I'd also like to note that there are no difference in syscalls between > using /dev/null and "rrdtool ''", so there should be no additional > cost between the two methods. There are some between using a dash and > the other two, but I think that's because nothing is written to STDOUT > at all. right > Of course, in the case of graph.php, we want 'rrdtool -', since the > graphs are generated in-line. AFAIK there were only 2 candidates in functions.php and there was a suggestion to consolidate them somehow as part of BUG193 that you are now CC on. even better will be to calculate the values as a sideffect of a real graph being generated which will eliminate the need to have this "use rrdgraph as a calculator" functions overall and with it all the problems seen so far. > > but the solution proposed by Jason has the advantage of being cross-platform > > (/dev/null doesn't exist in windows), so if you see no problem with the > > original patch I'd suggest you revert yours. > > As I said, this was the only one of the three that I could get working > consistently. I also happen to think that "rrdtool /dev/null <blah>" > is more obvious than "rrdtool '' <blah>". So there's a minor argument > to be made in favor of that as well. In the case of Windows, there is > an equivalent NUL file that could be used. OK, I personally don't care about windows either, and I really think that whoever is nuts enough to try to run the ganglia web frontend and gmetad in windows when they can just get a quick LAMP setup doing that in spare hardware deserves all the bugs they will find doing so. but intentionally breaking their setup when there is a solution that is crossplatform and that has the same effect than one that is *nix specific will be just too much evilness on our part and there might be good reasons for using windows for this in some environments (luckily not in mine). in this case though I'd expect using /dev/null as an output should work the same for rrdtool than having an invalid filename eitherway and even if I can't reproduce the problem you are reporting if you say it fails for you and this is "fixing" it then I'll go with that. > Regardless, I do *not* think that the patch should simply get > reverted. Arguably, the problem stems from a lack of proper error > handling on the exec() call. This exists in 1753 (and persists 1754, > for that matter), so a simple revert won't help matters any. not visible in my setup with PHP 5.2.6 in either case but maybe you could come out for a solution to that problem instead (or on top of the current one or probably instead) if the problem is that the output is not being handled correctly, adding some more checks around it as it is done in the original find_limits function might do the trick. > Instead, > having code to handle NULL output from exec(), in addition to an > rrdtool that can reliably suppress graph generation but still do the > computations we want. this is an unlikely fix to have (but feel free to push it to the rrdtool developers), in any case, even if there is a version of rrdtool that does that, we will have to support the old behaviour for backward compatibility and unless we start distributing our own version of rddtool or forcing everyone to upgrade to a fixed version that we support, which are also unlikely to happen IMHO Carlo ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Ganglia-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ganglia-developers
