Closed
Bug 141951
Opened 22 years ago
Closed 16 years ago
Set the max_packet_size for attachments (and bugs_fulltext) when connecting to MySQL
Categories
(Bugzilla :: Installation & Upgrading, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.2
People
(Reporter: CodeMachine, Assigned: mkanat)
References
Details
Attachments
(1 file, 1 obsolete file)
4.46 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
If the max attachment size in the system parameters is larger than the maximum size MySQL allows, obscure MySQL error messages can get spewed out to the user if their attachment is bigger than the MySQL size. We could possibly use the SQL "SHOW VARIABLES" and read off the "max_allowed_packet" variables. From there we could force the administration page to make the max attachment size smaller than this. I figure there's probably some overhead in the packet besides the attachment, so it would actually have to be some appropriately smaller size.
Reporter | ||
Updated•22 years ago
|
Priority: -- → P4
Target Milestone: --- → Bugzilla 2.20
Comment 1•21 years ago
|
||
Relevant information can be found at: http://www.mysql.com/doc/en/Packet_too_large.html
Comment 2•20 years ago
|
||
This has come up enough times to actually annoy me.
Assignee: justdave → kiko
Comment 3•20 years ago
|
||
*** Bug 263908 has been marked as a duplicate of this bug. ***
Comment 4•20 years ago
|
||
*** Bug 275821 has been marked as a duplicate of this bug. ***
Comment 5•19 years ago
|
||
*** Bug 285116 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 6•19 years ago
|
||
This would be really easy now that we have the Bugzilla::DB::Mysql module. You can put the check inside Bugzilla::DB::Mysql::new, and then it will also get checked inside of checksetup.
Comment 7•19 years ago
|
||
*** Bug 307704 has been marked as a duplicate of this bug. ***
Comment 8•18 years ago
|
||
*** Bug 329532 has been marked as a duplicate of this bug. ***
Comment 9•18 years ago
|
||
*** Bug 331643 has been marked as a duplicate of this bug. ***
Comment 10•18 years ago
|
||
*** Bug 339357 has been marked as a duplicate of this bug. ***
Updated•18 years ago
|
QA Contact: mattyt-bugzilla → default-qa
Updated•18 years ago
|
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Comment 11•18 years ago
|
||
ACK. This should be done within the database module. May be the DB module should provide an function to get such db backend limitations. Other RDBMs don't have such low limitations (ie. postgresql allows 1 GB per row) BTW: large attachements could be stored as blobs.
Comment 12•18 years ago
|
||
I (and other people, according to the dupes) encountered this same problem, but instead of spewing SQL errors, the upload _appears_ to work fine, and gives the message 'Attachment #0 to Bug #612 Created', which is of course untrue as attachment #0 cannot exist. It also successfully obsoletes other uploads and lodges the comment, linked to bug #0, which is clearly incorrect behaviour if the upload failed! The max_allowed_packet MySQL variable indeed solves the problem, and I agree that the DB module should handle this somehow, but in the meantime, how about catching this error? Currently I have a bug with 5 comments referring to the same 'attachment #0', where a user has repeatedly attempted to upload a large file and been baffled as to why it didn't work. I am using 2.20.2, so apologies if this error is being handled better in later versions.
Comment 13•18 years ago
|
||
kiko hasn't touched this in 2 years, and I suspect Max is in a better position to do something about it nowadays. Per comment 6 this probably belongs in Installation/Upgrading since the check should be done in checksetup.pl and/or editparams.cgi
Assignee: kiko → installation
Component: Attachments & Requests → Installation & Upgrading
Comment 14•18 years ago
|
||
I would suggest that the check is also done on the upload page, as it is possible for this value to change after installation - that would make it more bullet-proof.
Assignee | ||
Comment 15•18 years ago
|
||
It may also be possible to change max_allowed_packet with a SET SESSION in Bugzilla::DB::Mysql::new(). I'll have to look into that.
Assignee | ||
Comment 16•18 years ago
|
||
The number of dups filed indicates to me that this is a higher-priority support issue than it is currently filed as, so I'm upgrading the priority and severity.
Assignee: installation → mkanat
Severity: minor → normal
Priority: P4 → P1
Comment 17•16 years ago
|
||
(In reply to comment #0) > If the max attachment size in the system parameters is larger than the maximum > size MySQL allows, obscure MySQL error messages can get spewed out to the user > if their attachment is bigger than the MySQL size. > We could possibly use the SQL "SHOW VARIABLES" and read off the > "max_allowed_packet" variables. From there we could force the administration > page to make the max attachment size smaller than this. > I figure there's probably some overhead in the packet besides the attachment, so > it would actually have to be some appropriately smaller size. yes (In reply to comment #1) > Relevant information can be found at: > http://www.mysql.com/doc/en/Packet_too_large.html (In reply to comment #0) > If the max attachment size in the system parameters is larger than the maximum > size MySQL allows, obscure MySQL error messages can get spewed out to the user > if their attachment is bigger than the MySQL size. > We could possibly use the SQL "SHOW VARIABLES" and read off the > "max_allowed_packet" variables. From there we could force the administration > page to make the max attachment size smaller than this. > I figure there's probably some overhead in the packet besides the attachment, so > it would actually have to be some appropriately smaller size. (In reply to comment #0) > If the max attachment size in the system parameters is larger than the maximum > size MySQL allows, obscure MySQL error messages can get spewed out to the user > if their attachment is bigger than the MySQL size. > We could possibly use the SQL "SHOW VARIABLES" and read off the > "max_allowed_packet" variables. From there we could force the administration > page to make the max attachment size smaller than this. > I figure there's probably some overhead in the packet besides the attachment, so > it would actually have to be some appropriately smaller size.
Comment 18•16 years ago
|
||
very good
Comment 19•16 years ago
|
||
The 2.22 branch is restricted to security bugs -> 3.0
Target Milestone: Bugzilla 2.22 → Bugzilla 3.0
Assignee | ||
Comment 21•16 years ago
|
||
Note that we also need this now for bugs_fulltext, and it's not very hard to fix. (If your comments all combined are larger than max_allowed_packet, they'll get truncated or there will be an error when you update a bug.)
Severity: normal → major
Summary: Make sure max attachment size is small enough to not cause MySQL errors. → Set the max_packet_size for attachments (and bugs_fulltext) when connecting to MySQL
Assignee | ||
Updated•16 years ago
|
Flags: blocking3.2+
Comment 22•16 years ago
|
||
Will this check be run during checksetup and edit_params? Probably the best two places to do it.
Comment 23•16 years ago
|
||
Here is how to get the value: my ($max_pack_size) = @{$self->selectcol_arrayref( q{SHOW VARIABLES LIKE 'max\_allowed\_packet'}, {Columns=>[2]})}; Looks like the best place to do this is in the middle of the "update_params" sub. http://mxr.mozilla.org/mozilla/source/webtools/bugzilla/Bugzilla/Config.pm#113
Assignee | ||
Comment 24•16 years ago
|
||
It's going to be *set*, not checked.
Assignee | ||
Comment 25•16 years ago
|
||
Attachment #344540 -
Flags: review?(LpSolit)
Comment 26•16 years ago
|
||
Comment on attachment 344540 [details] [diff] [review] v1 >Index: Bugzilla/DB/Mysql.pm >+ my $min_max_allowed_packet = MAX_COMMENTS * MAX_COMMENT_LENGTH; >+ my $max_allowed_packet = max($min_max_allowed_packet, >+ Bugzilla->params->{'maxattachmentsize'}); >+ $self->do("SET SESSION max_allowed_packet = $max_allowed_packet"); One thing bothers me with your patch: if max_allowed_packet is already set to a larger value in my.cnf, you completely ignore it. I can imagine some cases where this could be a problem (e.g. in bugs with several tens or hundreds of long comments such as stack trace). Why not taking the max between $max_allowed_packet and "SHOW VARIABLES LIKE 'max_allowed_packet'"?
Attachment #344540 -
Flags: review?(LpSolit) → review-
Assignee | ||
Comment 27•16 years ago
|
||
Yeah, fair enough. :-) Here we go.
Attachment #344540 -
Attachment is obsolete: true
Attachment #344830 -
Flags: review?(LpSolit)
Comment 28•16 years ago
|
||
Comment on attachment 344830 [details] [diff] [review] v2 Looks good. Note that your patch doesn't apply cleanly on the 3.2 branch.
Attachment #344830 -
Flags: review?(LpSolit) → review+
Assignee | ||
Comment 29•16 years ago
|
||
OK, I'll fix the patch for 3.2 when I check it in.
Flags: approval3.2+
Flags: approval+
Assignee | ||
Comment 30•16 years ago
|
||
tip: Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.266; previous revision: 1.265 done Checking in Bugzilla/Constants.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v <-- Constants.pm new revision: 1.99; previous revision: 1.98 done Checking in Bugzilla/DB/Mysql.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v <-- Mysql.pm new revision: 1.67; previous revision: 1.66 done Checking in docs/en/xml/installation.xml; /cvsroot/mozilla/webtools/bugzilla/docs/en/xml/installation.xml,v <-- installation.xml new revision: 1.162; previous revision: 1.161 done 3.2: Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.241.2.16; previous revision: 1.241.2.15 done Checking in Bugzilla/Constants.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v <-- Constants.pm new revision: 1.92.2.4; previous revision: 1.92.2.3 done Checking in Bugzilla/DB/Mysql.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v <-- Mysql.pm new revision: 1.60.2.4; previous revision: 1.60.2.3 done Checking in docs/en/xml/installation.xml; /cvsroot/mozilla/webtools/bugzilla/docs/en/xml/installation.xml,v <-- installation.xml new revision: 1.157.2.4; previous revision: 1.157.2.3 done
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: relnote
Resolution: --- → FIXED
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Comment 32•15 years ago
|
||
Is it possible that this setting wasn't set when upgrading from 3.0.6 to 3.2? Because I got this error and had to modify my /etc/my.cnf Also, the release notes don't explain what needs to be redone if restoring from a dump; wouldn't checksetup.pl fix this automagically?
Assignee | ||
Comment 33•15 years ago
|
||
(In reply to comment #32) > Is it possible that this setting wasn't set when upgrading from 3.0.6 to 3.2? > Because I got this error and had to modify my /etc/my.cnf I don't think so, no, unless your maxattachmentsize parameter is actually smaller than some of the attachments currently in your DB. > Also, the release notes don't explain what needs to be redone if restoring from > a dump; wouldn't checksetup.pl fix this automagically? Ah, yeah, that's something that we should probably explain somewhere, because you do have to do that now. You can file a bug for that, if you want.
Comment 34•15 years ago
|
||
(In reply to comment #33) > (In reply to comment #32) > > Is it possible that this setting wasn't set when upgrading from 3.0.6 to 3.2? > > Because I got this error and had to modify my /etc/my.cnf > > I don't think so, no, unless your maxattachmentsize parameter is actually > smaller than some of the attachments currently in your DB. No, but it was smaller than the value in my data/params. :-/ I thought this bug was about making them match so we wouldn't get this error? > > Also, the release notes don't explain what needs to be redone if restoring from > > a dump; wouldn't checksetup.pl fix this automagically? > > Ah, yeah, that's something that we should probably explain somewhere, because > you do have to do that now. You can file a bug for that, if you want. Added bug 476905
Comment 35•15 years ago
|
||
Details: Bugzilla 3.2 /etc/my.conf had max_packet_size = 1M maxattachmentsize was set to 2000 I got the infamous mysql error message. I expected it to either say the attachment was too big or to adjust the max_packet_size to 2000 for the SQL connection.
Assignee | ||
Comment 36•15 years ago
|
||
(In reply to comment #34) > No, but it was smaller than the value in my data/params. :-/ I thought this > bug was about making them match so we wouldn't get this error? It makes them match when it connects, so it's only effective for Bugzilla code. If you have somehow customized your code, it may not be effective for your customizations.
Comment 37•15 years ago
|
||
(In reply to comment #36) > It makes them match when it connects, so it's only effective for Bugzilla > code. If you have somehow customized your code, it may not be effective for > your customizations. Hmm... Nothing should be customized in the process.cgi or attachment uploading stuff... I have customizations on the edit templates... but it goes to the default attachment upload template first. Maybe it's broken or it doesn't have permission to set max_packet_size or something? Should I file a new bug, then? I can recreate the situation on off hours, if need be.
Assignee | ||
Comment 38•15 years ago
|
||
(In reply to comment #37) > Should I file a new bug, then? You could, but it's almost certainly a local problem, since I tested this fairly thoroughly when I wrote the patch, I believe. However, if we're not throwing an error message when we can possibly do so (say, that something is denying us access to setting this configuration var), then that would be bug-worthy.
Comment 39•15 years ago
|
||
Is there someway I can tell if something is denying access to the configuration var?
You need to log in
before you can comment on or make changes to this bug.
Description
•