Zimbra offers Open Source email server software and shared calendar for Linux and the Mac
Go Back   Zimbra :: Forums > Zimbra Collaboration Suite > Developers

Welcome to the Zimbra :: Forums!
Welcome, if you would like to post a comment please register. We also encourage you to explore all things Zimbra with our team and members of the community.

Reply
 
LinkBack Thread Tools Search this Thread Display Modes
  #1 (permalink)  
Old 05-05-2006, 07:37 PM
New Member
 
Posts: 3
Default Possible bug in disk statistics page generation

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:

Code:
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:

Code:
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:
Code:
var img = Dwt.parseHtmlFragment(AjxBuffer.concat("<img src='/service/statsimg/disk.", server, ".", periodString, "_",
New:
Code:
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:

Code:
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:

Code:
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):
Code:
periodInt , ", 0])' onerror='javascript:AjxCore.objectWithId(", this.__intern
New:
Code:
 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
Reply With Quote
  #2 (permalink)  
Old 05-05-2006, 08:00 PM
Zimbra Employee
 
Posts: 4,792
Default

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.
__________________
Bugzilla - Wiki - Downloads - Offline Client
Reply With Quote
  #3 (permalink)  
Old 05-05-2006, 08:08 PM
New Member
 
Posts: 3
Default

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
Reply With Quote
  #4 (permalink)  
Old 05-08-2006, 11:18 AM
Zimbra Employee
 
Posts: 4,792
Default

Yes, please add your details to the bug. It will help us narrow down the issue and a fix.
__________________
Bugzilla - Wiki - Downloads - Offline Client
Reply With Quote
  #5 (permalink)  
Old 05-08-2006, 03:41 PM
Zimbra Employee
 
Posts: 127
Default

Quote:
Originally Posted by KevinH
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.
__________________
Bugzilla - Wiki - Downloads - Before posting... Search!
P.S.: don't forget to vote on this bug
add Samba LDAP entries to Exchange Migration Tool
Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes


Similar Threads

Why Join?

Registering let's you ask questions, makes it easier to search, displays any files attached to posts, and notifies you about replies.

blog.zimbra.com




 

SEO by vBSEO ©2011, Crawlability, Inc.