Problem

If you are using MyISAM as default database driver and you try to install current 8.x it will fail with a WSOD.

For more information see origin issue: #2270693: Installing Drupal core 8.x causes WSOD after DB credentials.

Proposed resolution

Shorten collection column in the config table and add the limit to the ConfigCollectionInfo object.

Remaining tasks

Review patch.

Reproduce

Have a clean environment with drupal 8.x ready to install.

Open your mysql configuration (/etc/mysql/my.cnf)
and insert the following at [mysqld]

skip-external-locking
skip-innodb
default_storage_engine=MyISAM

restart mysql ($sudo service mysql restart)

Which forces the usage of MyISAM on installation.

Now try to install drupal.

Can currently be reproduced on simplytest.me:
http://simplytest.me/project/drupal/8.x

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

patrickd’s picture

Issue summary: View changes
geerlingguy’s picture

Issue summary: View changes

Updated acronym to be slightly more correct :)

olli’s picture

Status: Active » Needs review
FileSize
576 bytes

#2262861: Add concept of collections to config storages added collection column (varchar 255) to config table primary key.

Eric_A’s picture

With #3 added simplytest.me now succeeds in installing 8.x, but admin/modules/install is broken. Don't know if this is related to MyISAM or if it's the same in HEAD with InnoDB.

xjm’s picture

xjm’s picture

alexpott’s picture

Title: MyISAM support broken » Limit collection name due to MyISAM restrictions and test ConfigCollectionInfo class
Issue summary: View changes
Related issues: -#697220: Run tests on all supported database servers
FileSize
5.82 KB

So lets encode this maximum length for collection names in the system.

alexpott’s picture

@Eric_A admin/modules/install and admin/modules work with MyISAM for me - so that issue is something unrelated to MyISAM and this specific issue with config collection name length.

alexpott’s picture

xjm’s picture

Component: database system » configuration system
Related issues: -#697220: Run tests on all supported database servers
FileSize
6.37 KB
3.32 KB

A few cleanups. The config object name is capped at 250 characters, not 255, so shortened that column and increased the collections column to 80.

xjm’s picture

The last submitted patch, 7: 2270917.4.patch, failed testing.

olli’s picture

Damien Tournoud’s picture

Why would Drupal 8 want or need to support MyISAM?

xjm’s picture

@Damien Tournoud, because there are certainly thousands of Drupal sites that use MyISAM, it's considered better/faster for some operations than InnoDB, and we support it for Drupal 7 and no one has otherwise filed an issue to remove support. We certainly get a bug report for D8 within a week or so every time we break it. Edit: Also http://simplytest.me/ uses MyISAM and TBH that's reason enough for me.

Attached is a silly doc standards fix, plus the test-only patch I meant to include last time.

The last submitted patch, 15: config-2270917-15-FAIL.patch, failed testing.

xjm’s picture

One more docs improvement. Sorry bot.

webchick’s picture

Issue tags: +alpha target

Would really love to get this in for the next alpha, if at all possible. Would highly suck for the alpha just before DrupalCon to not work on simplytest.me. :(

cordoval’s picture

+++ b/core/lib/Drupal/Core/Config/ConfigCollectionInfo.php
@@ -15,6 +17,20 @@
+   * collection collection names at 80 characters.

remove one collection

cordoval’s picture

  1. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigCollectionInfoTest.php
    @@ -0,0 +1,78 @@
    +      'description' => '',
    

    why there is no description of the unit test? sounds like the name should be equal to the description in most cases?

  2. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigCollectionInfoTest.php
    @@ -0,0 +1,78 @@
    +  public function testaddCollection() {
    

    why not testAddCollection?

  3. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigCollectionInfoTest.php
    @@ -0,0 +1,78 @@
    +    $this->assertEquals(array($collection), $collections);
    

    maybe use assertContains?

xjm’s picture

FileSize
6.45 KB
775 bytes

Now with less collection collection. :)

BTW, the patch is pretty easy to test manually... try to install D8 HEAD on simplytest.me, see that it breaks. Click the simplytest.me dreditor button for the patch, see that Drupal installs. Internet is won. ;)

xjm’s picture

Re: #20 point 1. I'm not sure redundancy is any better or worse than a blank string? HEAD does this all over the place for unit tests as far as I can see. For point 2, sure. For point 3, no, because it should be the only element in the list.

xjm’s picture

The last submitted patch, 21: config-2270917-20.patch, failed testing.

Damien Tournoud’s picture

Drupal doesn't support MyISAM for any decent definition of the term. Running Drupal on MyISAM (a non-transactional table engine) is going to result in all sorts of pain and data loss, even on relatively low traffic websites.

I understand the desire of making sure that Drupal works for the largest amount of people possible, but it is ill-minded in that case. We don't do people service by pretending that we support non-transactional table engines.

fgm’s picture

Seconding DamZ here: MyISAM is a massive performance problem beyond single-user sites due to its table-level locking : if we don't want to remove it because it can be marginally faster on strict single-user workloads, it should probably be documented as never to be used outside this case, like SQLite.

xjm’s picture

This is all out of scope here. If you want to propose we remove MyISAM support from Drupal 8, please file a separate issue proposing that. For now, we need to fix this critical regression so hundreds of contributors can use http://simplytest.me to work with Drupal 8 at DrupalCon Austin. This is by no means the first schema we've tweaked for MyISAM.

patrickd’s picture

(simplytest.me will switch to InnoDB before austin if necessary)

Damien Tournoud’s picture

(simplytest.me will switch to InnoDB before austin if necessary)

MyISAM is not actually supported since at least Drupal 7, even if we never documented this. So yes, you should.

patrickd’s picture

(simplytest.me will switch to InnoDB before austin if necessary)

MyISAM is not actually supported since at least Drupal 7, even if we never documented this. So yes, you should.

Wish somebody had told me that earlier ;)

I'll see what I can do; thanks everyone!

xjm’s picture

MyISAM is not actually supported since at least Drupal 7, even if we never documented this. So yes, you should.

There are still plenty of people using it for D7 and D8, so we need an explicit issue to discuss officially dropping support if that's the case.

catch’s picture

patrickd’s picture

(BTW: simplytest.me now uses MariaDB with InnoDB as database driver - drupal 8.x can now be installed without issues)

webchick’s picture

Yay, that's great Patrick! Nevertheless, it'd be great to get this fixed.

xjm’s picture

Status: Needs review » Closed (won't fix)

Discussed with @alexpott. With #2275535: [policy, no patch] Drop support for non-transactional database engines (including MyISAM) this is now a wontfix. Thanks everyone.