95890 - Changing a keyword to a keyword+ fails to record in bugmail/activity log

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Dave Miller [:justdave]]

This happened in a number of bugs, but I reproduced it and you can examine the
activity log on bug 95854.

If you have a standard keyword (say nsenterprise) and you change that keyword to
one with a plus on the end (say nsenterprise+), it records the regular one being
removed, but does not record the addition of the + keyword, even though it happens.

Comment 1

[Dave Miller [:justdave]]

Myk, this is apparently causing triage problems.  Can you get this reviewed and
updated to b.m.o ASAP?  (fix my patch if it's broke, I don't have an easy way to
test it, and I'm too tired to think about it now)  I *think* it's being caused
by DiffStrings() failing to escape special characters in the regexp...  patch to
follow

Comment 2

[Dave Miller [:justdave]]

Attachment: Patch v1

Comment 3

[Dave Miller [:justdave]]

I see a bug in the patch already, needs a g on the end of the s/// (s///g)

Comment 4

[Dave Miller [:justdave]]

Ah, crap....   Myk/Jake:  This exact same routine was used to convert the
activity table to the new format in checksetup.pl.  Which means all history of
keyword to keyword+ changes and vice versa got toasted in the conversion.

Anyone feel like writing a quick script to restore that table from the backup,
reconvert it, then reapply any changes to it made since the conversion?  Ewwwww

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: avoids regexp entirely

Comment 6

[Myk Melez [:myk] [@mykmelez]]

The \Q...\E metasymbols escape regular expression meta-characters and can
accomplish this task more easily than specifying each meta-character in a
separate regexp.

In this particular situation, however, a regular expression is unnecessary since
we are checking two strings for equivalence.  The attached patch uses the test
"$_ eq $value" within the grep calls to match values that appear in the arrays.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Patch applied to b.m.o. installation.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

The plan for recovering data lost before the upgrade is as follows:

---
Create a database called bugs_old and populate it with data from the pre-upgrade
backup file.
Run a script to convert the bugs_old.bugs_activity table to the new format.
Shut down b.m.o.
Delete keyword records from the bugs.bugs_activity table.
Copy keyword records from the bugs_old.bugs_activity table to the
bugs.bugs_activity table.
Sync the shadow database to the bugs database.
Start up b.m.o. and review the database to make sure the data has been recovered.
Delete the bugs_old database.
---

Comment 9

[Myk Melez [:myk] [@mykmelez]]

To clarify:

---
The plan for recovering data lost before the upgrade
---
... should read...

---
The plan for recovering data that was lost during the upgrade
---

Comment 10

[Myk Melez [:myk] [@mykmelez]]

It looks like there are only about 15-25 bugs_activity records that were
corrupted on Friday because of this bug, and most if not all of these records
can be fixed by looking at the current value of the keywords field and deducing
from it what activity took place.

---

Note: if, as Dave says in bug 95889, the bugs_activity table has additionally
been corrupted with spurious records due to incorrect splitting of records
across commas, then it might make more sense to re-convert all the old records
to the new format rather than just the keyword records.

Comment 11

[Jacob Steenhagen]

I didn't realize you could grep for $_ eq $value... this same thing should be
done in checksetup.pl.

Comment 12

[Dave Miller [:justdave]]

As noted, this same change needs to be made to the conversion routine in
checksetup.pl as well (it doesn't load globals.pl, so the code is duplicated there).

Comment 13

[Dave Miller [:justdave]]

Attachment: Patch v2a Changes to checksetup.pl

Comment 14

[Matthew Tuck [:CodeMachine]]

If you delete all keywords records, won't we lose recent keywords records that
were made since upgrade and didn't suffer this bug?

Comment 15

[Myk Melez [:myk] [@mykmelez]]

No, because I am only going to delete records from before the change.

Comment 16

[Myk Melez [:myk] [@mykmelez]]

More info about data recovery plans:

Saturday evening or Sunday morning I am going to write the script that
re-converts the data that was corrupted during the upgrade.

Sunday afternoon (~2-3pm) or Sunday night (~10pm) I am going to take Bugzilla
down and convert the data per the plan previously posted here.  Then I am going
to go through and correct by hand the records that were created on Friday.

So, by Monday morning all database corruption will be fixed.

Comment 17

[Myk Melez [:myk] [@mykmelez]]

The latest version of the recovery plan:

----------------------------------------
Make sure there is enough space on the /mysql partition for another copy of the
b.m.o. database.  Create more space by gzipping the /mysql/mysqlbackup/bugs file
if necessary (gzip /mysql/mysqlbackup/bugs).

Create the bugs_old database (in mysql: CREATE DATABASE bugs_old;).

Import the backup file into the bugs_old database (gunzip --stdout
/tmp/bugs.pre-update.gz | mysql bugs_old).

Shut down Bugzilla (add text to shutdownhtml parameter).

CVS update Bugzilla (cd /opt/webtools/bugzilla/; ./cvs-update.sh).

Dump post-update bugs_activity data (mysqldump --no-create-info
"--where=bug_when>'2001-08-17 00:00:01'" bugs bugs_activity >
/tmp/bugs_activity.post-update.sql).

Drop the bugs_activity table from the bugs database (in mysql: USE bugs; DROP
table bugs_activity;).

Dump the bugs_activity from the bugs_old database and import it into the bugs
database (mysqldump bugs_old bugs_activity > /tmp/bugs_activity.pre-update.sql;
mysql bugs < /tmp/bugs_activity.pre-update.sql).

Run checksetup.pl (./checksetup.pl), which will re-convert the pre-update data.

Import the post-update bugs_activity data into the bugs database (mysql bugs <
/tmp/bugs_activity.post-update.sql).

Go through corrupted records in the post-update bugs_activity table by hand and
manually fix their problems (SELECT bugs_activity.*, keyworddefs.name AS
currentvalues FROM bugs_activity, keywords, keyworddefs WHERE
bugs_activity.bug_id = keywords.bug_id AND keywords.keywordid = keyworddefs.id
AND bug_when > '2001-08-17 00:10:00' and fieldid = 21 and (removed LIKE
'%nsenterprise%' or removed LIKE '%nsbeta1%' OR added LIKE '%nsenterprise%' OR
added LIKE '%nsbeta1%') AND bugs_activity.bug_id != 95854 AND (keyworddefs.name
LIKE '%nsenterprise%' OR keyworddefs.name LIKE '%nsbeta1%');).

Note: 42 bugs are in this list, about half of which contain corrupted data.

Note: nsdogfood+ and nsCatFood+ keywords also exist, but neither one was added
to or removed from a bug during the post-update data-corruption period.

Sync the shadow database to the main database (./syncshadowdb -syncall).

Bring Bugzilla back up.
----------------------------------------

Comment 18

[Myk Melez [:myk] [@mykmelez]]

Dave's patch looks good.  I am waiting to complete a database import, after
which I will test it against a replica of the pre-update database.

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: displays iteration count

Comment 20

[Dave Miller [:justdave]]

Note that patch v3 does not include patch v2, it only updates patch v2a.  So for
a complete checkin, as of this comment, you need both v2 and v3 (but not v2a).

Comment 21

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v5: the full monty

Comment 22

[Myk Melez [:myk] [@mykmelez]]

I can get the upgrade down to under an hour with the following plan, which
allows me to do some time-consuming tasks on the list before taking Bugzilla down:

1. Create the bugs_old database on another computer.

2. Import the pre-update backup file into the bugs_old database.

3. Run checksetup.pl against the bugs_old database to re-convert the pre-update
bugs_activity data.

4. Go through corrupted records in the post-update bugs_activity table in the
bugs database by hand and manually fix their problems.

5. Dump the bugs_activity table from the bugs_old database and copy the dump
file to the b.m.o. computer.

6. Shut down Bugzilla (add text to shutdownhtml parameter).

7. Dump the post-update bugs_activity table from the bugs database.

8. CVS update Bugzilla.

9. Drop the bugs_activity table from the bugs database.

10. Import the re-converted pre-update bugs_activity dump file into the bugs
database.

11. Import the post-update bugs_activity dump file into the bugs database.

12. Sync the shadow database to the main database.

13. Bring Bugzilla back up.

14. Party.

Comment 23

[Myk Melez [:myk] [@mykmelez]]

Dave's patch correctly converted my replica database, r=myk.  Dave gave an
r=justdave in IRC last night for my portion of the patch; checked in and resolved.

Comment 24

[Myk Melez [:myk] [@mykmelez]]

Triaged 43 bugs_activity table rows, 15 of which contained corrupted data that
was repaired via the following SQL statements:

----------
UPDATE bugs_activity SET added='nsbeta1+' WHERE bug_id=20943 AND who=16513 AND
bug_when='2001-08-17 15:42:38' AND fieldid=21;
UPDATE bugs_activity SET added='nsbeta1+' WHERE bug_id=26543 AND who=16513 AND
bug_when='2001-08-17 15:27:36' AND fieldid=21;
UPDATE bugs_activity SET removed='nsenterprise' , added='' WHERE bug_id=40823
AND who=1896 AND bug_when='2001-08-17 14:32:03' AND fieldid=21;
UPDATE bugs_activity SET added='nsenterprise+, nsBranch' WHERE bug_id=40867 AND
who=1896 AND bug_when='2001-08-17 15:13:03' AND fieldid=21;
UPDATE bugs_activity SET added='nsenterprise+' WHERE bug_id=62178 AND who=1896
AND bug_when='2001-08-17 14:24:55' AND fieldid=21;
UPDATE bugs_activity SET removed='nsenterprise' , added='' WHERE bug_id=64371
AND who=1896 AND bug_when='2001-08-17 14:46:49' AND fieldid=21;
UPDATE bugs_activity SET removed='nsenterprise' , added='nsenterprise+,
nsBranch' WHERE bug_id=65569 AND who=1896 AND bug_when='2001-08-17 14:59:12' AND
fieldid=21;
UPDATE bugs_activity SET added='nsenterprise+' WHERE bug_id=76248 AND who=1896
AND bug_when='2001-08-17 14:29:09' AND fieldid=21;
UPDATE bugs_activity SET added='nsenterprise+' WHERE bug_id=83555 AND who=1896
AND bug_when='2001-08-17 14:14:52' AND fieldid=21;
UPDATE bugs_activity SET removed='helpwanted' , added='patch, review' WHERE
bug_id=90567 AND who=25000 AND bug_when='2001-08-17 13:25:59' AND fieldid=21;
UPDATE bugs_activity SET added='nsenterprise+' WHERE bug_id=91352 AND who=1896
AND bug_when='2001-08-17 15:26:49' AND fieldid=21;
UPDATE bugs_activity SET removed='' , added='review' WHERE bug_id=91407 AND
who=21768 AND bug_when='2001-08-17 07:33:18' AND fieldid=21;
UPDATE bugs_activity SET removed='' , added='nsenterprise+' WHERE bug_id=91726
AND who=28283 AND bug_when='2001-08-17 12:28:19' AND fieldid=21;
UPDATE bugs_activity SET removed='nsenterprise' , added='nsenterprise+,
nsBranch' WHERE bug_id=92135 AND who=1896 AND bug_when='2001-08-17 15:30:42' AND
fieldid=21;
UPDATE bugs_activity SET removed='vtrunk' , added='' WHERE bug_id=93972 AND
who=29264 AND bug_when='2001-08-17 10:52:44' AND fieldid=21;
----------

Comment 25

[Myk Melez [:myk] [@mykmelez]]

The corruption recovery began 1:05pm PDT Sunday, August 19 and finished
successfully at 2:10pm that same day.  As far as I can tell, all database
corruption has been corrected.  If you notice any more corruption, reopen this
bug or file a new one.

Comment 26

[Dave Miller [:justdave]]

Moving to Bugzilla product