Possible bug in disk statistics page generation

[gentoo_matt]

All,

I believe I've found two bugs in the .js code that generates the disk statistics page within the Zimbra Admin. I've not been able to find either of these in the forums or Bugzilla yet, so if they've been addressed, I apologize.

I've recently completed an install of the OSS version of Zimbra (installed: Version 3.1.0_GA_332.DEBIAN3.1 Apr 14, 2006). All seems to be working correctly, except for Disk Statistics within the Admin.

On this particular server, there is one SCSI disk, partitioned and mounted as such:
1. /dev/sda3 - /
2. /dev/sda5 - /data

Under the /opt/zimbra/zimbramon/rrdtool/work/ directory, I have files similar to the following:
1. disk.server.domain.com.hour.Disk_Usage_0.gif
2. disk.server.domain.com.hour.Disk_Usage_1.gif
With the additional .day., .month., and .year. files, of course.

In addition, all of the disk data appears in MySQL as it should.

However, on the disk stats page, only the graph for /dev/sda5 was being displayed. This is nice, but does not help me, since Zimbra is installed on /dev/sda3.

Data from /dev/sda5 is contained in the Disk_Usage_0.gif file, with data from /dev/sda3 in Disk_Usage_1.gif. Digging through the access log for Tomcat, I noticed the following entries:

X.X.X.X - - [05/May/2006:16:13:04 -0500] "GET /service/statsimg/disk.server.domain.com.month_2.gif?nodef=1 HTTP/1.1" 404 997

Bug 1:
Digging through the .js code, I run across the following function in /opt/zimbra/tomcat/webapps/zimbraAdmin/js/zimbraAdmin/statistics/view/ZaServerDiskStatsPage.js:

ZaServerDiskStatsPage.prototype.loadNextImage = function (parent, periodInt, count) {
        // let's stop at some arbitrarily high number, so that we don't get caught for some
        // reason in an infinite loop
        if (count >= 50) {
                return;
        }
        ++count;
        var server = this._server.name;
        var periodString = this._getPeriodString(periodInt);
        var img = Dwt.parseHtmlFragment(AjxBuffer.concat("<img src='/service/statsimg/disk.", server, ".", periodString, "_",                                                                                                          count, ".gif?nodef=1' onload='javascript:ZaServerDiskStatsPage.callMethod(",                                                                                                          this.__internalId,",ZaServerDiskStatsPage.prototype.loadNextImage,",                                                                                                          "[this.parentNode,",periodInt ,",", count, "])'",                                                                                                          "onerror='javascript:ZaServerDiskStatsPage.callMethod(", this.__internalId ,                                                                                                         ",ZaServerDiskStatsPage.prototype.stopLoadingImages,[this,",count,"])'><br>"));

        parent.appendChild(img);
        parent.appendChild(document.createElement('br'));
        parent.appendChild(document.createElement('br'));
};

In addition, the previous function (in this same file) is as follows:

ZaServerDiskStatsPage.prototype.writeImageHtml = function (periodInt) {
        var periodString = "hour";
        var serverName = this._server.name;
        var periodString = this._getPeriodString(periodInt);
        return AjxBuffer.concat("<img src='/service/statsimg/disk." , serverName , ".", periodString,".Disk_Usage_0.gif?nodef=1' onload='javascript:ZaServerDiskStatsPage.callMethod(",                                                         this.__internalId , ",ZaServerDiskStatsPage.prototype.loadNextImage,[this.parentNode," ,                                                        periodInt , ", 0])' onerror='javascript:AjxCore.objectWithId(", this.__internalId , ").stopLoadingImages(this,0)'><br>");
};

It quickly became obvious that this was the source of consternation with displaying the additional disk partitions. The hard coded ".Disk_Usage_0.gif..." string within the ZaServerDiskStatsPage.prototype.writeImageHtml function will properly display the first disk graph, but the function ZaServerDiskStatsPage.prototype.loadNextImage will fail to properly write the file name for the remaining disk graph image files.

In order to fix this bug, I performed the following:

At line 91 in file /opt/zimbra/tomcat/webapps/zimbraAdmin/js/zimbraAdmin/statistics/view/ZaServerDiskStatsPage.js:

Original:

var img = Dwt.parseHtmlFragment(AjxBuffer.concat("<img src='/service/statsimg/disk.", server, ".", periodString, "_",

New:

var img = Dwt.parseHtmlFragment(AjxBuffer.concat("<img src='/service/statsimg/disk.", server, ".", periodString, ".Disk_Usage_",

This will allow this .js script to properly write the file name for additional disks beyond the first one.

The Fix:
To implement this fix, I did the following (Note: my knowledge of Tomcat and Javascript could probably barely fill a coffee cup. Use with caution!):

1. Modify the file, as mentioned above.
2. Modify the same variable declaration on line 33210 in file /opt/zimbra/tomcat/webapps/zimbraAdmin/js/ZimbraAdmin_all.js
3. Execute the following command: gunzip -S zgz ZimbraAdmin_all.js.zgz.
4. Modify the same variable declaration on line 2927 in file /opt/zimbra/tomcat/webapps/zimbraAdmin/js/ZimbraAdmin_all.js. (note the period at the end of the file). See the forum thread here for more info on why this is necessary.
5. Restart Tomcat.
6. Clear browser cache and cookies.

Bug 2:
After completing all of the above, I was now getting the following entries in my Tomcat access logs, immediately upon logging into the Admin and viewing disk stats:

X.X.X.X - - [05/May/2006:16:17:23 -0500] "GET /service/statsimg/disk.server.domain.com.day.Disk_Usage_0.gif?nodef= 1 HTTP/1.1" 200 11388
X.X.X.X - - [05/May/2006:16:17:23 -0500] "GET /service/statsimg/disk.server.domain.com.day.Disk_Usage_2.gif?nodef= 1 HTTP/1.1" 404 997

Again, going back to ZaServerDiskStatsPage.js, we see the following in function ZaServerDiskStatsPage.prototype.writeImageHtml:

return AjxBuffer.concat("<img src='/service/statsimg/disk." , serverName ,
                                                        ".", periodString,".Disk_Usage_0.gif?nodef=1' onload='javascript:ZaServerDiskStatsPage.callMethod(",
                                                        this.__internalId , ",ZaServerDiskStatsPage.prototype.loadNextImage,[this.parentNode," ,
                                                        periodInt , ", 1])' onerror='javascript:AjxCore.objectWithId(", this.__internalId ,
                                                        ").stopLoadingImages(this,0)'><br>");

Function ZaServerDiskStatsPage.prototype.loadNextImage is being called with arguments "[this.parentNode," ,periodInt , ", 1]".

This would be all well and good, except for the following:

Function ZaServerDiskStatsPage.prototype.loadNextImage initializes the third argument as variable "count". There is a check to make sure count is not greater than 50, then count is incremented before writing out the .gif image file name:

ZaServerDiskStatsPage.prototype.loadNextImage = function (parent, periodInt, count) {
        // let's stop at some arbitrarily high number, so that we don't get caught for some
        // reason in an infinite loop
        if (count >= 50) {
                return;
        }
        ++count;
        var server = this._server.name;
        var periodString = this._getPeriodString(periodInt);
        var img = Dwt.parseHtmlFragment(AjxBuffer.concat("<img src='/service/statsimg/disk.", server, ".", periodString, "_",....

If the value 1 is passed as count, with Disk_Usage_0.gif hard coded in the calling function, then Disk_Usage_1.gif gets skipped.

In order to fix this bug, I performed the following:

Original (line 78 in file ZaServerDiskStatsPage.js):

periodInt , ", 0])' onerror='javascript:AjxCore.objectWithId(", this.__intern

New:

 periodInt , ", 1])' onerror='javascript:AjxCore.objectWithId(", this.__intern

The Fix:
You'll need to repeat steps 1-6 of the previous fix, substituting line 78 in file ZaServerDiskStatsPage.js, line 33197 in file ZimbraAdmin_all.js, and line 2926 in file ZimbraAdmin_all.js. (after gunzip'ing).

After completing these steps, disk statistics works as it should, and all is well in the world ;-)

If these are indeed bugs, please let me know and I will file them appropriately in Bugzilla.

BTW, thanks for putting out such an awesome product. Keep up the good work!

- Gentoo Matt

[KevinH]

Nice post Matt. Might want to drop these in bugzilla so we can track them and get them fixed. You should take a peak at this bug first:

http://bugzilla.zimbra.com/show_bug.cgi?id=7072

It may be related to what you are seeing.

[gentoo_matt]

Kevin,

Wow! Thanks for the quick response! It does look like the bugs I referenced could be causing the condition described in bug 7072.

Question - do you want me to post a followup to this bug? I just want to make sure I'm following the proper Bugzilla procedures, since I'm new to the community. Don't want to step on anybody's toes

Thanks!

- Gentoo Matt

[KevinH]

Yes, please add your details to the bug. It will help us narrow down the issue and a fix.

[Greg]

Yes, please add your details to the bug. It will help us narrow down the issue and a fix.

Thanks a lot for spotting this. Just fixed it.