-
Notifications
You must be signed in to change notification settings - Fork 824
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
UTF8MB4 Breaks cron tasks #9811
Comments
As per multiple comments in the Slack workspace, this is not just cronjobs, but it has deeper impacts on multiple users. In my opinion, this is quite high impact, given at least 10 people (if my count is correct, and it's not including those that just don't complain), it breaks quite a few websites. I think, the immediate solution is reverting to UTF8 (non-MB4), until a working solution has been found. |
Can you describe the issue in more detail? Code samples/examples of errors would probably help here 👍 |
builds will break if the DB is not the latest version of MySQL. For example, MariaDB will break on indexes |
I cannot replicate this issue using framework 4.7.x-dev or 4.x-dev and crontask 2.1.3, using mariadb 10.5.2 and php 7.1.33 Could you please provide further replication steps? Test run and code below:
CronTest.php <?php
use SilverStripe\CronTask\Interfaces\CronTask;
class TestCron implements CronTask
{
public function getSchedule()
{
return "* * * * *";
}
public function process()
{
echo __CLASS__ . '::' . __FUNCTION__ . chr(10);
echo MyModel::get()->first()->MyTitle . chr(10);
}
} MyModel.php <?php
use SilverStripe\ORM\DataObject;
class MyModel extends DataObject
{
private static $db = [
'MyTitle' => 'Varchar'
];
private static $indexes = [
'MyTitle' => true
];
public function requireDefaultRecords()
{
parent::requireDefaultRecords();
if (self::get()->count() == 0) {
$record = self::create();
$record->MyTitle = 'my-title';
$record->write();
}
}
} |
Issue is on Ubuntu 18.04, which is at MariaDB 10.1, also, check the Slack Silverstripe Users workspace for details, as it's been mentioned there. This is a crucial difference in the Ubuntu supported MariaDB version for the used server. |
Furthermore, indexes of 255 characters, will break the build. MariaDB's support for longer indexes or unique, do not work. |
The example you created, will throw the following error: |
I tried dropping mariadb down to 10.1.10 (docker container uses ubuntu 18.04 as far as I'm aware) though I am still unable to replicate this issue using the code sample above. I'm not sure what further I can do here without further replication steps. |
Full stack trace from one of my machines:
Details:
This is with UTF8MB4 enabled. Switching back to UTF8 fixes it. |
Related code:
Relevant part of the Message class:
|
The release notes https://docs.silverstripe.org/en/4/changelogs/4.7.0/#default-mysql-collation-updated say that only new sites installed via silverstripe/installer should have ut8mb4 enabled unless you choose to switch in over yourself manually. Was this an existing site with content already in the database that was on 4.6.x or less and was upgraded to 4.7.0 and the database collation automatically changed without you intending it to? Or was this a brand new site? |
Both. |
So, by both does that mean it was a newly deployed site on 4.7.0 installed with silverstripe/installer, and, with a database snapshot restored from a different website that was on 4.6.x or less? |
The change is literally a breaking change, for multiple people. Yes, a clean installation does break. "YoU sHoUlD uPdAtE yOuR dAtAbAsE". Insert Spongebob meme here. This change is a breaking change. If you can't replicate it, then, that's not my problem. It is not my problem, to solve your problems, so to speak. @chillu , @unclecheese , @bergice , I would appreciate your input and more thorough testing. |
This comment has been minimized.
This comment has been minimized.
With all due respect, I don't think you would. This thread evidences someone clearly taking a lot to time to understand your issue, and the level of "appreciation" you've shown is sarcastic barbs and a public indictment of his job title. We all have a common interest to make the ecosystem healthier and more stable overall, but a lot of open source is about community and relationships, and the way you're interacting with others here leaves little incentive for anyone to offer up their time on your behalf. Please afford some civility and respect. Not every issue can be understood and actioned immediately. |
This comment has been minimized.
This comment has been minimized.
Trying to catch up on this. Reopening this, to signal our willingness to support this process. The issue has been observed by Simon on MariaDB 10.1.47. I'm assuming that Simon opted into the change for an existing project when upgrading to 4.7. The issue has also been observed by We currently state support for MySQL 5.6+. While MariaDB isn't explicitly supported, it's assumed to be a limited drop in replacement starting from MariaDB 10.0 for MySQL 5.6. 4.7 introduced an opt-in change for existing projects (which are perfectly fine under semver even if they turn out to be breaking changes). Nobody is forced to use it, but if there's known caveats we should document these. 4.7 sets utf8mb4 a new default for new projects (via the installer). New projects should work with supported database versions (so MySQL 5.6+). If they don't, that might be considered a breaking change. Apparently there's more impacted users on Slack, but it's hard to find out details. We need to be able to reproduce to fix the issue, and that requires patience and input from the people experiencing the issues - that's pretty uncontroversial, right? But it sounds like Steve is stuck at the moment and needs help in being able to reproduce this. Maybe we need more isolation, e.g. running the same Docker image with a default codebase plus the specific code mentioned above? It'll get complicated if the issue can only be reproduced on an existing database snapshot. P.S.: Unless it's related to the failures, I propose that we discuss the issue about MariaDB index sizes on a separate Github issue - this one is already complicated enough :) |
We've determined that due to the collation adjustment being introduced in a separate config file, it is currently being installed in projects during upgrades, making this an opt-out change as of Recipe 4.7.0. I've raised two PRs to address this:
Sincere apologies for the confusion, @Firesphere - this was always seen and intended as an opt-in change. |
I closed the issue out of pure frustration. I openly admit that. The change to switch to MB4 is understandable, but I just wanted a breaking change to be fixed, and all I got initially was "I can't reproduce". I hope you understand that it's a frustrating thing, which, in a way, is similar to walking in to a wall.
I did not opt in, it was "forced" upon me. If it wasn't for me noticing the Unrelated, I prefer to be called Firesphere on GitHub/GitLab etc. ;)
It was not just me, multiple people have observed the issue. Sadly, despite my efforts to have them chip in, nobody actually did. This may or may not (spoiler alert, it is), a reason why people don't chip in or open PR's or issues. As they don't feel taken seriously (as I do), which is why I resorted to sarcasm. I have nothing against the team, but if I keep running in to a wall.... something's gotta give.
Despite best efforts on your side, it was, and is, an opt-out, not an opt-in, at the moment. This caused quite some issues which are, as you say, per semver, not to be expected. I do not mean anything bad, but I do stand with my frustration.
This is an issue I'm not sure about. Some MariaDB instances of 10.1+ seem to work, others don't, despite the large index setting. Right now, I feel it's more random. For example my local Vagrant machine on Ubuntu 18.04-latest works just fine, but one of my clients machines on production, doesn't. With exactly the same config files, except for the memory limit. It's confusing me at best.
I expected that was the case. But it caused a lot of headache for quite a few people. Which is a shame. May I suggest, checking if the DB engine supports the change, for such impactful systems in the future. E.g. if "Ubuntu.OS <= 18.04 { Don't do anything }; @Cheddam Cheers! I'm closing this issue, again, because it has taken up a lot of my time, which I should not have spent at it, in the end. |
As Firesphere doesn’t want to continue discussion here, I’ve opened a new issue for tracking this: #9822 |
@Firesphere I appreciate that you've invested a lot of time and are keen to move on from this, but I've done some further testing against the same distro and MariaDB version you reported, and I have a theory about what the issue is. When running the SQL query listed in the debug output you provided directly via the MariaDB CLI client, I get the same syntax error you encountered. The reason I get this is that the default In 4.7.0, alongside the collation change, My theory about the cause of your troubles is that the configuration cache used in the CLI environment of your servers needs to be flushed in order to populate the If you have any further time to spare, I'd really appreciate if you could try running a |
Hi @kinglozzer It's not about not wanting to discuss, but now that it's properly opt-in in the next release, and I don't feel like I have the time or energy to discuss further. I've given every bit of information I have, and I left it at that. Discussion should continue on the PR made by @Cheddam , not here, in my opinion. You're free to disagree, of course. @Cheddam My default set-up for this client flushes when updating the code twice, before and after the pull from the repository. And only if those are successful, runs a |
FWIW, running a |
Affected Version
4.x latest
Description
Using MariaDB on Ubuntu 18.04, the
"
quoted queries ran from a cron job, throw an errorSteps to Reproduce
Run a cron job that requires querying the database
The text was updated successfully, but these errors were encountered: