764457 - Add a primary key to the bugs_activity table

Status: RESOLVED FIXED

(Bugzilla :: Database, enhancement)

Description

[David Lawrence [:dkl]]

Due to recent database issues and also being good practice to have a primary key on tables due to replication, I will be attaching a patch to add a primary key to the bugs_activity table.

dkl

Comment 1

[David Lawrence [:dkl]]

Attachment: Patch to add incrementing primary key to bugs_activity table (v1)

Comment 2

[Frédéric Buclin]

I don't think we need a PK for everything. An index marked as UNIQUE will do it.

Comment 3

[Sheeri Cabral [:sheeri]]

If you don't specify a primary key for InnoDB, InnoDB will make one internally, that we as end users cannot access. If the table isn't InnoDB, a UNIQUE index will do it.

The reason InnoDB makes this hidden primary key is that InnoDB stores a primary key with every secondary index (unique or regular index). So there is also motivation not to make a very large PRIMARY key (e.g., a primary key of (bug_id, bug_when, who, what) would be pretty "big").

My advice is for an auto-increment primary key. How does this affect non-MySQL databases?

Comment 4

[:glob ✱]

Comment on attachment 632749 [details] [diff] [review]
Patch to add incrementing primary key to bugs_activity table (v1)

r=glob

Comment 5

[Frédéric Buclin]

Hum no, wait. Do we have any DB column within Bugzilla which isn't used at all? I don't think that's the case. Is there no code which could use it? Else there is no problem leaving InnoDB tables create their own internal PK.

Comment 6

[Sheeri Cabral [:sheeri]]

Well, we need a way to identify rows in case of data duplication, which actually did happen, and is why this ticket was created. so there *is* a use case for a primary key......if the unique index is very large, it makes more sense to use a primary key.

Comment 7

[Frédéric Buclin]

I found a place where this new primary key will be useful: in contrib/recode.pl. On checkin, add a comment in SPECIAL_KEYS right above

  bugs_activity     => 'bug_id,bug_when,fieldid'

that the PK only exists since 4.4 (else someone is going to accidentally remove this entry).

Comment 8

[David Lawrence [:dkl]]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified contrib/recode.pl
modified Bugzilla/Install/DB.pm
modified Bugzilla/DB/Schema.pm
Committed revision 8290.

Comment 9

[Simon Green]

12:57 <%reed> simon: can you comment in the bug?
13:06 <%reed> simon: reopen it, too

It is my suggestion that the column type should be changed from a mediumserial to an intserial. The limit for a signed medium serial value is 8388607 (source: http://dev.mysql.com/doc/refman/5.0/en/numeric-type-overview.html )

At Red Hat, we have 8033780 rows in this table (and over 8.38 million on our dev server). Even if bmo isn't close to the 8.38 million limit, other installations (I'm looking at Yahoo! here) may easily be over that limit.

Comment 10

[:glob ✱]

on our 4.2 staging environment we have 8652904 rows in bugs_activity.

Adding new column 'id' to the 'bugs_activity' table...
DBD::mysql::db do failed: ALTER TABLE causes auto_increment resequencing, resulting in duplicate entry '8388607' for key 'PRIMARY' [for Statement "ALTER TABLE bugs_activity ADD COLUMN id mediumint auto_increment PRIMARY KEY NOT NULL"] at Bugzilla/DB.pm line 606.


nice catch simon!

Comment 11

[Frédéric Buclin]

For comparison, how many rows do you have in the longdescs table? Its comment_id column is also of type MEDIUMSERIAL.

I'm a bit surprised that this problem hasn't been caught by dkl and glob before being checked in as they wanted it for bmo specifically. I thought this patch was already applied to bmo?

Comment 12

[Simon Green]

(In reply to Frédéric Buclin from comment #11)
> For comparison, how many rows do you have in the longdescs table? Its
> comment_id column is also of type MEDIUMSERIAL

mysql> SELECT COUNT(*) FROM longdescs;
+----------+
| COUNT(*) |
+----------+
|  4733147 |
+----------+

For brc, we are only half way to hitting the 8.3 million rows issue.

Comment 13

[:glob ✱]

(In reply to Frédéric Buclin from comment #11)
> For comparison, how many rows do you have in the longdescs table? Its
> comment_id column is also of type MEDIUMSERIAL

6063162 in the sanitized dump (prod will have more of course).

> I'm a bit surprised that this problem hasn't been caught by dkl and glob
> before being checked in as they wanted it for bmo specifically.

i didn't hit the limit in my dev env, and there's a bug in our update script on our 4.2 env which was masking this :(

> I thought this patch was already applied to bmo?

it is not.

Comment 14

[Frédéric Buclin]

(In reply to Byron Jones ‹:glob› from comment #13)
> > For comparison, how many rows do you have in the longdescs table? Its
> > comment_id column is also of type MEDIUMSERIAL
> 
> 6063162 in the sanitized dump (prod will have more of course).

This means that bmo is very close to the 8.38M limit and should reach it in the coming years. Maybe both columns should be changed to INTSERIAL.

Comment 15

[:glob ✱]

(In reply to Frédéric Buclin from comment #14)
> This means that bmo is very close to the 8.38M limit and should reach it in
> the coming years. Maybe both columns should be changed to INTSERIAL.

that sounds sane.

Comment 16

[Frédéric Buclin]

Closing this bug again as bug 776972 has been filed about this problem.