817486 - _sync_fulltext always updates bugs_fulltext.short_desc, even if it wasn't changed

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[:glob ✱]

_sync_fulltext always updates bugs_fulltext.short_desc, even if it wasn't changed.

this is problematic on mysql because the bugs_fulltext table is MyISAM, which has very broad locks (no row-level locks).

by updating the short_desc field each time a comment is added (even if it hasn't changed), we're taking a broad lock for no reason, which can cause locking problems on high volume sites.

Comment 1

[:glob ✱]

Attachment: patch v1

this patch reworks sync_fulltext to ensure we update the bugs_fulltext table only once for each update (currently the code performs two separate actions).

Comment 2

[:glob ✱]

Attachment: patch v2

i missed a few calls to _sync_fulltext, which were being made outside the package.

Comment 3

[Frédéric Buclin]

Comment on attachment 688250 [details] [diff] [review]
patch v2

>=== modified file 'Bugzilla/Bug.pm'

>+    $self->_sync_fulltext(
>+        update_short_desc => $changes->{short_desc},
>+        update_comments   => $self->{added_comments} || $self->{comment_isprivate}
>+    ) if $self->{added_comments} || $changes->{short_desc}
>+         || $self->{comment_isprivate};

Please remove the |if ...| part. Not only this looks ugly, but _sync_fulltext() won't do anything if both update_short_desc and update_comments are false. So this test is useless. r=LpSolit with this fix on checkin.

Comment 4

[Frédéric Buclin]

Looks safe for 4.4.

Comment 5

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified importxml.pl
modified sanitycheck.cgi
modified Bugzilla/Bug.pm
modified Bugzilla/Comment.pm
modified Bugzilla/Migrate.pm
Committed revision 8542.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified importxml.pl
modified sanitycheck.cgi
modified Bugzilla/Bug.pm
modified Bugzilla/Comment.pm
modified Bugzilla/Migrate.pm
Committed revision 8495.