[policy, no patch] Drop support for non-transactional database engines (including MyISAM) [#2275535]

Opening this following discussion in #2270917: Limit collection name due to MyISAM restrictions and test ConfigCollectionInfo class.

Damien Tournoud pointed out that we don't really support MyISAM any more (i.e. we expect transaction support in lots of core code such as field/entity database storage. xjm pointed out that we claim to support MyISAM so people still can and do run it.

Given we've had numerous patches recently that broke specifically on MyISAM, we should fix it and add it to #697220: Run tests on all supported database servers as a release blocker if we're serious about supporting it.

If we're not going to do that, then we should explicitly remove support for it (instead of pretending that we support it when we really don't). This would have been very difficult before migrate in core, but we at least don't have to deal with trying to automatically upgrade database tables...

This would also mean removing https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Database%... and etc.

Beta blocker because an 8.x site installed on MyISAM has nowhere to go if we rip out support. Unlike sqlite or postgresql, this can't be moved to contrib, we'll just be saying you can't use a non-transactional database with the core database API at all.

One advantage of dropping support would be we could use longer indexes if we need to - although need to check limits on other databases.

Proposed resolution

Decide in this issue to stop supporting MyISAM as the primary storage engine for a Drupal install. This needs to be documented in a change notice + update drupal.org/requirements
Follow-up (major) patch to add a warning/error to the installer if MySQL is chosen as the default storage engine, and nothing in SHOW ENGINES reports as InnoDB, but need to be careful with this since we don't want false positives.
Follow-up (normal) issue to consider dropping Connection::supportsTransactions() altogether since it's not even accurate without manual intervention on a per-site basis anyway

Comments

Comment #1

catch

he/him

English

commented 28 May 2014 at 11:38

Issue summary:

View changes

Comment #2

xjm

she/her

English

commented 28 May 2014 at 11:42

Parent issue:

» #2152443: Allow settings.php to specify the table engine

#2152443: Allow settings.php to specify the table engine would be wontfix if this were agreed. Edit: Actually I think it maybe just becomes a little feature request; catch pointed out that someone might want to use it to pick a different MySQL engine.

Comment #3

xjm

she/her

English

commented 28 May 2014 at 11:43

Parent issue:	#2152443: Allow settings.php to specify the table engine	»
Related issues:		+#2152443: Allow settings.php to specify the table engine

Comment #4

damien tournoud commented 28 May 2014 at 11:55

There are other engines that might be useful in specific situations (for example NDB), so I don't see #2152443: Allow settings.php to specify the table engine as specifically related to MyISAM.

Comment #5

xjm

she/her

English

commented 28 May 2014 at 12:10

Well, that's the reason it was initially filed. At least, we would need to follow up there to use an engine other than MyISAM in the example documentation.

@catch pointed to #2230091-17: Route rebuilding is not guaranteed to finish in time for the next request.

Comment #6

ianthomas_uk

he/him

English

Brighton, UK

commented 28 May 2014 at 12:57

Would this include the semaphore table (assuming that still exists on D8)?
See #1898204: Do not use InnoDB for the semaphore table, use Memory

Comment #7

xjm

she/her

English

commented 29 May 2014 at 15:49

Title:

Drop MyISAM support

» Drop support for non-transactional database engines (including MyISAM)

Comment #8

mikeytown2 commented 29 May 2014 at 21:57

The only issue I can potentially see with this is the fact that InnoDB only recently supported full-text search. That's the only real reason I can think of why someone would switch engines. Then there are the odd ball cases like this as to why doing this might not be a good idea: http://drupal.stackexchange.com/questions/114656/drupal-pdoexception-whe...

Edit: GIS is another reason for someone to not pick InnoDB, as indexes can not be used with InnoDB.
http://dev.mysql.com/doc/refman/5.6/en/creating-spatial-indexes.html

Comment #9

Crell commented 29 May 2014 at 23:44

Have I mentioned that I hate MySQL?

It is difficult to keep code that works close enough to the same both with and without transactions. Simplifying that is fine by me.

How often do we use fulltext search? If I need to do anything like that these days I use Solr, ElasticSearch, or some other for-realsies search indexer. Honestly I don't remember the last time I even used core search. :-)

The saving grace is that in D8, a lot less is (or should be) hard-coded tables. We can think less globally. So, eg, the MEMORY engine is not transaction-safe (http://dev.mysql.com/doc/refman/5.1/en/memory-storage-engine.html), but we can still use it for #1898204: Do not use InnoDB for the semaphore table, use Memory as we don't need transactions on that. Cache tables are now built on the fly (I think that got in), which means they can create whatever tables they want. (Maybe those should be memory, too?) There's work to make the Entity system create its own tables, too. (I can't find the issue right now.) So... maybe this is a per-subsystem thing? Entity API only does transactional engines, but Lock or Cache or State could support others?

That may be over-complicating things as I don't know how to really communicate that... :-)

Comment #10

catch

he/him

English

commented 30 May 2014 at 09:38

Thinking about this some more.

The only issue we have that breaks core explicitly is index length. So we can say that on MySQL something compatible with InnoDB has to be available. That's easy at http://drupal.org/requirements.

For transactions I'd forgotten that core assumes transaction support anyway - the only way that supportsTransactions() returns FALSE is if you explicitly set that as a connection option in settings.php, so we really have no way to support non-transactional engines, but equally no way currently to require transaction support at a code level.

On MySQL at least, I think it would be possible to check that at least one storage engine supports transactions in the installer. SHOW ENGINES provides that information since 5.1. So using that we can check for InnoDB support and/or transaction support, and possibly warn if we don't find it. At the moment MariaDB reports xtradb and similar as InnoDB, but can't guarantee that'll last forever (or even that SHOW ENGINES across the various MySQL variants will stay consistent).

But not sure it's worth the effort trying to enforce this at all - especially given we're close to the point where you can run Drupal without touching an SQL database anyway.

Additionally it's completely fine to use something like MEMORY as the storage engine for cache tables. What happens then is those won't respect transactions, which means you run into issues like #1679344: Race condition in node_save() when not using DB for cache_field, but that's an issue with any alternative cache storage too. MySQL doesn't care at all if you run queries on a non-transactional storage engine within a transaction.

So what I think should happen:

Policy decision to drop MyISAM as the default storage engine in this issue - change notice + update drupal.org/requirements
Follow-up (major) patch to add a warning/error to the installer if MySQL is chosen as the default storage engine, and nothing in SHOW ENGINES reports as InnoDB, but need to be careful with this since we don't want false positives
Follow-up (normal) issue to consider dropping Connection::supportsTransactions() altogether since it's not even accurate without manual intervention on a per-site basis anyway

Comment #11

Crell commented 30 May 2014 at 19:30

MyISAM isn't the default engine already. Drupal will try to use InnoDB by default if you don't try to tell it otherwise. The only way to get all-MyISAM is to not have InnoDB installed, IIRC.

So it's more like "policy decision that we're not going to bother with a MyISAM testbot", followed by catch's points 2 and 3.

(As a side note, I don't know what the install process is for going totally non-SQL. Not relevant to this thread, but it's something we may want to think about sometime.)

Comment #12

catch

he/him

English

commented 30 May 2014 at 19:38

Sorry #1 should have been 'drop support for MyISAM as the main storage engine' for an install not the default. Although it's likely it will also be the default on installs that don't have InnoDB (or something reporting as InnoDB).

Non-SQL install process we'll need to get rid of all core hook_schema() implementations before that's realistic, but that's getting less distant so worth opening an issue for.

Comment #13

catch

he/him

English

commented 30 May 2014 at 20:14

Issue summary:

View changes

Comment #14

catch

he/him

English

commented 30 May 2014 at 20:15

Title:	Drop support for non-transactional database engines (including MyISAM)	» [policy, no patch] Drop support for non-transactional database engines (including MyISAM)
Status:	Active	» Needs review

Put a proposed resolution in the issue summary. Marking CNR.

Comment #15

Crell commented 30 May 2014 at 22:45

Added proposed change notice: https://drupal.org/node/2277327

Comment #16

chx commented 31 May 2014 at 03:29

I see no problems here. It is trivial to null implement the transaction methods in a driver if someone needs such (because the underlying database / engine doesn't support it).

Comment #17

chx commented 31 May 2014 at 03:30

Comment #18

andypost

he/him

Russian

commented 31 May 2014 at 11:48

memory engine can't be used for cache because can't have blob/text columns, so change notice should be updated

Comment #19

chx commented 31 May 2014 at 14:25

Also

Only the InnoDB engine (or compatible engines such as NDB) are supported.

There is nothing "compatible" between NDB and InnoDB such a thing is not even defined; you could happily use for example TokuDB as well. It is not even clear what do we mean by 'supported'. I'd recommend

Drupal core is tested with InnoDB only.

Because that's the honest truth. Beyond that, you are on your own.

Comment #20

Crell commented 1 June 2014 at 01:24

Updated change notice per #19: "Only the InnoDB engine is tested and considered fully supported."

Comment #21

catch

he/him

English

commented 1 June 2014 at 11:04

Status:

Needs review

» Reviewed & tested by the community

OK I've discussed this with the other committers already and I think everyone was OK with it.

However since I opened the issue and now RTBCing, it'd be good to get confirmation on the issue.

Comment #22

alexpott

he/they

English

🇪🇺🌍

commented 1 June 2014 at 15:07

Removed the mention of putting cache tables in memory since this is not possible on a mysql type db unless you're running a percona server - afaik neither MySQL or MariaDB support blobs in memory tables.

Comment #23

cweagans

He/Him

English

Boise, ID, USA

commented 1 June 2014 at 19:30

I agree with the decision and it seems that pretty much everyone else here does too. There's a change notice ready to roll. Seems like this is done, or am I missing something?

Per https://drupal.org/node/2275535#comment-8827351, there are two actionable followups:

Add a warning/error to the installer if MySQL is chosen as the default storage engine, and nothing in SHOW ENGINES reports as InnoDB
Consider dropping Connection::supportsTransactions() altogether since it's not even accurate without manual intervention on a per-site basis anyway

Comment #24

chx commented 1 June 2014 at 19:46

That warning I am extremely strongly against. Drupal 8 is going to be out there for 5-6 years how do you know what awesome engines will show up and take the world by storm?

Comment #25

catch

he/him

English

commented 2 June 2014 at 09:20

@chx we'd only show that warning if:

1. MyISAM is the default storage engine

2. InnoDB (nor any of the current alternatives that report themselves as it) is not available

In that situation, with the current code in core, the tables will be created with MyISAM. And that's the only situation we'd want to warn about.

If you're using a new fancy storage engine, then to get tables created with that, you'd have to set it as the default or otherwise configure it so that Drupal uses it.

Comment #26

xjm

she/her

English

commented 2 June 2014 at 16:15

Assigned:

Unassigned

» xjm

Update https://drupal.org/requirements/database
create cahnge record

Comment #27

webchick

she/they

English

Vancouver 🇨🇦

commented 2 June 2014 at 19:42

Do we have any data on what InnoDB support is like across hosts? We tend to look at this kind of thing prior to making decisions like this, or at least polling g.d.o/core.

Comment #28

cweagans

He/Him

English

Boise, ID, USA

commented 2 June 2014 at 20:00

Pretty sure InnoDB is almost as widely supported as PHP. The absolute bottom of the barrel in my opinion is GoDaddy shared hosting, and they do support InnoDB. Also supported by hostgator and site5 and bluehost.

Also, this has been mentioned in person at the sprint table a few times, but I don't know that it's been brought up in this issue. Drupal 8 won't install at all on MyISAM right now and nobody has said anything, so this issue is also about deciding if we want to invest the time/resources in fixing the problems that already exist.

Comment #29

xjm

she/her

English

commented 2 June 2014 at 23:00

@webchick: I asked the same question of @alexpott, @Dries, and @effulgentsia, and they recommended against a g.d.o/core post here.

In general, the likelihood of a host supporting PHP 5.4 (released Mar. 2012) but not InnoDB is slim, as MySQL itself has defaulted to InnoDB since 5.5 (released Dec. 2010). In both the case of PHP and the database, sites wishing to use Drupal 8 will need to pick (or migrate to) a host that supports these requirements.

Thanks @cweagans for checking the specific hosting platforms!

Comment #30

xjm

she/her

English

commented 2 June 2014 at 23:22

I made the following handbook updates:
https://drupal.org/node/270/revisions/view/7094195/7306523
https://drupal.org/node/1783060/revisions/view/7305331/7306511

Edit: I improved both docs to be more explicit that we don't support MyISAM as the primary storage engine. If someone knows what they're doing and chooses to convert a table to MyISAM for an edgecase reason, they still can.

Comment #31

xjm

she/her

English

commented 2 June 2014 at 23:36

And a draft change record:
https://drupal.org/node/2278745

Comment #32

xjm

she/her

English

commented 2 June 2014 at 23:37

Assigned:

xjm

» dries

Assigning to Dries for final sign-off on the requirements change.

Meanwhile, can we get the followups filed? (See issue summary.)

Comment #33

catch

he/him

English

commented 3 June 2014 at 10:43

I did a google search for innodb not supported and the first page of results was mainly people getting warnings from Magento's installer or similar about it not being supported on obscure shared hosting plans in 2008/9.