307602 - Smooth attach_data upgrade for sites with huge attachment tables

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, enhancement, P3)

Description

[Joel Peshkin]

For sites with giant attachment tables, the upgrade path to use attach_data is
much smoother if checksetup uses INSERT .. SELECT instead of copying each row in
the client.

Also, sites that have needed to overrid the 4G MySQL table size limit will need
to have the limit raised for the new attach_data table. This should not hurt
smaller sites at all.

Comment 1

[Joel Peshkin]

Attachment: Initial patch for comment

Please see the attached patch.	There are a few issues I have with it....

1) It has an arbitrary limit of 100GB for the attach_data table.  While that is
much better than the default of 4GB, it is still arbitrary.

2) It only applies the higher limit to upgrades.  New installations will get
the standard 4GB limit from mysql.  This is OK in both situations, but it might
be better to apply a higher limit when the attach_data table is created on new
installs as well.

Comment 2

[Max Kanat-Alexander]

Comment on attachment 195350 [details] [diff] [review]
Initial patch for comment

>+    $dbh->do("ALTER TABLE attach_data 
>+              AVG_ROW_LENGTH=1000000, 
>+              MAX_ROWS=100000") if ($my_db_driver eq "mysql");

  Put this in Bugzilla::DB::Mysql::bz_setup_database at some point after the
SUPER::bz_setup_database call.

>+    $dbh->do("INSERT INTO attach_data (id, thedata) 
>+                   SELECT attach_id, thedata FROM attachments");

  I *believe* that this also works fine on Pg, so that looks good to me. :-)

Comment 3

[Max Kanat-Alexander]

Yeah, this seems like something that would be good to get into the branch.

Comment 4

[Joel Peshkin]

Attachment: v2 - Works on all installations if default was left in effect

OK...  I reorganized this as suggested.  It is now conditional on the default
MAX_ROWS still being in effect, so it will work on new and upgraded
installations, but it will not second-guess someone who has already overriddent
the 4G default.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 197258 [details] [diff] [review]
v2 - Works on all installations if default was left in effect

>+    $sth = $self->prepare("SHOW TABLE STATUS like 'attach_data'");

  Don't re-use $sth, just make a new one.

>+        print "Converting attach_data maximum zise to 100G..\n";

  Typo and not enough periods. :-)

  Everything else looks good.

Comment 6

[Joel Peshkin]

Attachment: v3 with nits fixed

Comment 7

[Max Kanat-Alexander]

Comment on attachment 197269 [details] [diff] [review]
v3 with nits fixed

Looks good to me! :-)

I like the selectrow much better. :-)

Comment 8

[Max Kanat-Alexander]

Because of the large amount of automated testing that checksetup gets, I think
it's safe to put this in 2.20 final.

Comment 9

[Frédéric Buclin]

attach_data doesn't exist on 2.20

Comment 10

[Joel Peshkin]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.439; previous revision: 1.438
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v  <--  Mysql.pm
new revision: 1.29; previous revision: 1.28
done