MySQL Bugs: #31516: Upgrade fails if graph settings changed in 1.0.0 or 1.0.1

Bug #31516	Upgrade fails if graph settings changed in 1.0.0 or 1.0.1
Submitted:	10 Oct 2007 19:50	Modified:	6 Nov 2007 15:14
Reporter:	Andy Bang	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Enterprise Monitor: Server	Severity:	S1 (Critical)
Version:	1.2.0.7657	OS:	Any
Assigned to:	Darren Oldag	CPU Architecture:	Any

Description:
If you change the graph interval in 1.0.0 or 1.0.1 (say to 24 hours), it writes out an integer ("24") rather than an interval ("24:00:00") to the preferences table.  Later, when you try to upgrade to 1.2.0, the migration code chokes on this.  I'll post related emails below on this.  Also see https://support.mysql.com/view.php?id=19951.

How to repeat:
1) Install 1.0.0.
2) Change the graph interval to 24 hours.
3) Upgrade to 1.2.0.
4) When you try to login you'll see the following errors:

java.lang.ArrayIndexOutOfBoundsException: 1
com.mysql.merlin.server.Schema.splitIntervalPreference(Schema.java:674)
com.mysql.merlin.server.Schema.migrateSchema38(Schema.java:652)
com.mysql.merlin.server.Schema.migrate(Schema.java:95)
com.mysql.merlin.server.Schema.migrate(Schema.java:38)
com.mysql.merlin.server.ServiceMgr.initDatabase(ServiceMgr.java:206)
com.mysql.merlin.server.ServiceMgr.<init>(ServiceMgr.java:140)

Suggested fix:
Handle integers for intervals.

From Eric Herman:

Okay, I finally have the data loaded.

> This customer is having a problem upgrading from 1.1.1 to 1.2-beta.  Here are the details:

> java.lang.ArrayIndexOutOfBoundsException: 1
> com.mysql.merlin.server.Schema.splitIntervalPreference(Schema.java:674)
> com.mysql.merlin.server.Schema.migrateSchema38(Schema.java:652)

And here is what I see:

mysql> select * from preferences;
+---------+----------------------------+-------------+
| user_id | property                   | value       |
+---------+----------------------------+-------------+
|       1 | graph.default.interval     | 24          |
|       1 | graph.full.height.default  | 350         |
|       1 | graph.full.width.default   | 350         |
|       1 | graph.thumb.height.default | 100         |
|       1 | graph.thumb.width.default  | 200         |
|       1 | user.locale                | english-usa |
|       1 | user.timezone              | US/Eastern  |
|       3 | user.locale                | english-usa |
|       3 | user.timezone              | US/Eastern  |
|       5 | user.locale                | english-usa |
|       5 | user.timezone              | US/Eastern  |
+---------+----------------------------+-------------+
11 rows in set (0.01 sec)

mysql>

Or, to emulate the query exercised by the migration:

mysql> select user_id, value from preferences where property = 
'graph.default.interval';
+---------+-------+
| user_id | value |
+---------+-------+
|       1 | 24    |
+---------+-------+
1 row in set (0.00 sec)

thus, for 'graph.default.interval' a '24' gets fed to String.split in 
Schema.java lines 671-674:

   String dashboardInterval = each.getValue();
   String[] dashboardIntervalSplit = dashboardInterval.split(":");
   String dashboardIntervalHours = dashboardIntervalSplit[0];
   String dashboardIntervalMinutes = dashboardIntervalSplit[1];

and I believe since "24" does not contain a colon (":"), the 
dashboardIntervalHours will be set to 24 and the 
dashboardIntervalMinutes will NPE.

So, I think that we can change that value from "24" to "0:24" or "24:0" 
it will complete the migration. I will test that in a moment.

Josh: is your best guess that is is supposed to be 24 hours or 24 minutes?

Oldag, Team: what do you think about this: In the mean time, I think we 
can prevent the code from having a problem with this on a future 
releases with something like this:

   String dashboardInterval = each.getValue();
   String[] dashboardIntervalSplit = dashboardInterval.split(":");
   String dashboardIntervalHours = dashboardIntervalSplit[0];
   String dashboardIntervalMinutes = "0";
   if (dashboardIntervalSplit.length > 1) {
       dashboardIntervalSplit[1];
   }

That seems low impact and could be easily bundled in the next 1.2.x release.

Alternatively, maybe we can probably create instructions or an SQL for 
setting the preferences so they will not have these sorts of values.

Anyone have a guess as to how likely our 1.1.x install-base is to 
encounter this problem?

Cheers,
  -Eric

From Eric Herman:

UPDATE preferences \
    SET value='24:0' \
  WHERE property='graph.default.interval';

Allows the migration to finish, so that should be a work-around for this 
customer in the immediate time-frame.

We should consider our longer-term plan.

Cheers,
  --Eric

From Josh:

So, it looks as though there was both an inconsistency and a bug in the 1.0 release handling this interval. It appears that the default was 24, the value gets saved by the user as a full interval (see line 275 in /gui/php/lib/Merlin/Controller/Graphs.php and line 86 of /gui/php/lib/Merlin/Model/Graph.php), and the value was treated as an integer for hours in the displaying action (see line 338 in /gui/php/lib/Merlin/Controller/Dashboard.php). Unless I'm missing something, this should break consistently.

In any case, Eric's solution looks right-ish moving forward. It just looks like he forgot to make the assignment. I would just replace the one line setting the minutes with:

String dashboardIntervalMinutes = dashboardIntervalSplit.length > 1 ? dashboardIntervalSplit[1] : "00";

Based on my reading of the code, eric's update statement should resolve the upgrade issue and this customer should not experience further issues with this.

Josh

From Andy:

Thanks to Josh and Bill we've found the culprit. If you install 1.0.0 and change the interval (on the Settings page in that release), it writes out "24" rather than "24:00:00". I checked the download logs and Revolution Health (the customer for this issue) originally downloaded 1.0.0, then download the update installers for 1.0.1, 1.1.0, and 1.2.0.
 
Bill has confirmed that it writes out "24" with 1.0.1, too.
 
I installed 1.1.0.4876 (the build they said they were upgrading from), and changed the interval to 24 and it wrote out "24:00:00", so the problem was fixed between 1.0.1 and 1.1.0.
 
So it seems like anyone upgrading from an original 1.0.0 install (or 1.0.1), and who changed the interval in that version, but did *not* change it again in a later version, will be affected by this bug.
 
The workaround is to apply Eric SQL statement patch until we get a real fix into the Merlin migration code.
 
Andy

Patch committed to trunk; requested review for merge into br_6559_1.2.0

svn commit -m "BUG#31516, CSC#19951 - Fixes Migration ArrayIndexOutOfBoundsException"
Sending        server/merlin/WEB-INF/src/com/mysql/merlin/server/Schema.java
Sending        server/merlin/WEB-INF/test/com/mysql/merlin/server/SchemaTest.java
Transmitting file data ..
Committed revision 7754.

patch Committed revision 7757 to branch br_6559_1.2.0

not in official build

Verified fixed in version 1.2.0.7879.

Added to the changelog.