-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase default maxAllowedPacket size. #1411
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
FWIW we should have kept the default to be the same and let users change it via the query param. Changing the default to be higher is a breaking change for customers who have not changed their server side setting. Please consider reverting this to be the original value of 4MB. This is causing a breaking change from our side: mattermost/mattermost#23394 (comment) |
I'm sorry about changing it in 1.7.1. I had to wait 1.8. Please advocate to use larger max_allowed_packet when user might send more than 4MB blob. |
Keeping the default to be the same, and letting users "fix" the issue by changing the query param would have been the best way to keep full backward compatibility and fix the issue as well. And then if you want to change the default, cut a 2.0 release. |
It may break people using v1.7.1 with default parameters.
It is totally nonsense, like this comic. We need to break small part every release, even in bugfix release. We release v2 when we need to change/remove public APIs. If you can not allow such a small behavior change, you must pin the exact version. |
I don't see how. If the parameter is unset, it would still use the original value. |
I don't understand what you're saying... |
@agnivade : Also FWIW as I brought this up a 4MB query size is tiny. The original problem was the fact that the go driver error message said the server's setting was wrong (it wasn't) and also the 4 MB default size is tiny. This has nothing to do with blobs, just large queries can easily grow over this size.
|
Description
Fixes #1355.
Checklist