Problem/Motivation

Core's implementation of lock api supposed to use "semaphore" table to implement locking.
This table as all others use InnoDB storage engine which brings serious bug that any lock is released by any transaction.

Proposed resolution

Change storage engine for mysql to Memory

Original report by @mikeytown2

#301362: Default to InnoDB in MySQL was great for D7 but the semaphore table should actually be MEMORY instead of InnoDB.

Problem reported elsewhere with wrong conclusion:
http://www.workhabit.com/blog/drupal-innodb-and-myisam-engine-issues-cac...

Recently reported here with the right conclusion: side 26
http://www.percona.com/resources/technical-presentations/drupal-and-mysq...
Side 26 contents

Memory engine for semaphore table
● Removes InnoDB overhead
● No disk access needed – table in RAM
● Native implementation (no plugin or modules)
● Non-durable
● Negative – table-level locking can reduce concurrency, but this
table is typically small (and hopefully not used often)

Various links on drupal.stackexchange.com where people are complaining about this issue.
http://drupal.stackexchange.com/questions/113042/after-enabling-mysql-re...
http://drupal.stackexchange.com/questions/23500/enabling-modules-my-site...
http://drupal.stackexchange.com/questions/22145/pdo-deadlocks-galore-on-...

Files: 
CommentFileSizeAuthor
#57 semaphore-deadlock-on-dev-site.sql_.txt7.69 KBkwerey
#46 deadlock-march-21.sql_.txt43.11 KBmikeytown2
#46 deadlock-april-9.sql_.txt25.11 KBmikeytown2
#34 drupal-1898204-34-semaphore-memory-table-D8.patch399 bytesmikeytown2
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,628 pass(es).
[ View ]
#25 drupal-1898204-25-semaphore-memory-table.patch381 bytesmikeytown2
PASSED: [[SimpleTest]]: [MySQL] 40,950 pass(es).
[ View ]
#23 drupal-1898204-23-semaphore-memory-table.patch529 bytesmikeytown2
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#4 drupal-1898204-4-convert-tables.patch1.82 KBmikeytown2
PASSED: [[SimpleTest]]: [MySQL] 40,021 pass(es).
[ View ]
#2 drupal-1898204-2-convert-tables.patch1.82 KBmikeytown2
PASSED: [[SimpleTest]]: [MySQL] 39,666 pass(es).
[ View ]

Comments

mikeytown2’s picture

Status:Needs review» Active

Initial work for updating old sites:

<?php
$tables
= array(
 
'cache' => 'MyISAM',
 
'cache_bootstrap' => 'MyISAM',
 
'cache_menu' => 'MyISAM',
 
'semaphore' => 'MyISAM',
 
'variable' => 'MyISAM',
);
system_install_convert_db_engine($tables);

function
system_install_convert_db_engine($tables) {
  static
$engines = array();
 
// Get all Database Engine types.
 
if (empty($engines)) {
   
$engine_query = db_query('SHOW ENGINES');
    foreach (
$engine_query as $engine) {
     
$engines[$engine->Engine] = $engine;
    }
  }

 
// Build SQL query.
 
$sql = "SHOW TABLE STATUS WHERE Name = '" . implode("' OR Name = '", array_keys($tables)) . "'";
 
// Get table info.
 
$db_table_info = array();
 
$table_query = db_query($sql);
  foreach (
$table_query as $table) {
   
$db_table_info[$table->Name] = $table;
  }

 
// Find tables that need to be converted.
 
$tables_to_convert = array();
  foreach (
$tables as $name => $engine) {
    if (   !empty(
$engines[$engine])
        &&
$engines[$engine]->Support != 'NO'
       
&& $db_table_info[$name]->Engine != $engine
         
) {
     
$tables_to_convert[$name] = $engine;
    }
  }

 
// Convert Tables.
 
foreach ($tables_to_convert as $table_name => $engine) {
   
db_query('ALTER TABLE ' . $table_name . ' ENGINE = ' . $engine);
  }
}
?>
mikeytown2’s picture

StatusFileSize
new1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 39,666 pass(es).
[ View ]

Patch to fix fresh installs.

mikeytown2’s picture

Status:Active» Needs review
mikeytown2’s picture

Status:Active» Needs review
StatusFileSize
new1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 40,021 pass(es).
[ View ]

Last patch has a typo in it.

mikeytown2’s picture

Modules that use drupal_get_schema_unprocessed() in order to get cache table schema.
Output from gotta_download_them_all/allmodules$ grep -l -r -i " = drupal_get_schema_unprocessed('system', 'cache');" ./    

This is for 7.x code only

./adbc/adbc.module                                                                                   
./admin_menu/admin_menu.install                                                                      
./advagg/advagg.install                                                                              
./advagg/advagg_css_compress/advagg_css_compress.install                                             
./advagg/advagg_js_compress/advagg_js_compress.install                                               
./amazon_store/amazon_store.install                                                                  
./alfresco/alfresco_browser/alfresco_browser.install                                                 
./apachesolr/apachesolr.install                                                                      
./accuweather/accuweather.install                                                                    
./advanced_blockqueue/advanced_blockqueue.install                                                    
./autolink/autolink.install                                                                          
./biblio/biblio.install                                                                              
./blizzardapi/blizzardapi.install                                                                    
./booking_com_api/booking_com_api.install                                                            
./bookmark/bookmark.install                                                                          
./browscap/browscap.install                                                                          
./bubbletimer/bubbletimer.install                                                                    
./bundle_aggregation/bundle_aggregation.install                                                      
./clients/clients.install                                                                            
./coffee/coffee.install                                                                              
./commerce_cybersource/commerce_cybersource.install                                                  
./commerce_entitycache/commerce_entitycache.install                                                  
./commerce_kiala/commerce_kiala.install                                                              
./commerce_shipping/commerce_shipping.install                                                        
./controls/controls_nav/controls_nav.install                                                         
./controls/controls_cc/controls_cc.install                                                           
./creativecommons/creativecommons.install                                                            
./ctr_field/ctr_field.install                                                                        
./customfilter/customfilter.install                                                                  
./dominion/dominion.install                                                                          
./drd/drd.install                                                                                    
./dynamic_background/modules/dynamic_background_inherit/dynamic_background_inherit.install           
./entitycache/entitycache.install                                                                    
./feedback_reloaded/feedback_reloaded.install                                                        
./finder/finder.install                                                                              
./genes4all/modules/genes4all_explore/genes4all_explore.install                                      
./genes4all/modules/genes4all_nextgen/genes4all_nextgen.install                                      
./govdelivery/govdelivery.install                                                                    
./gravatar/gravatar.install                                                                          
./Guiders-JS/guiders_js.install                                                                      
./hacked/hacked.install                                                                              
./harvest/harvest.install                                                                            
./icontact/icontact.install                                                                          
./l10n_server/l10n_packager/l10n_packager.install                                                    
./l10n_update/l10n_update.install                                                                    
./livethemer/livethemer_inspector.install                                                            
./mailchimp/mailchimp.install                                                                        
./media/media.install                                                                                
./media_mover/media_mover_api.install                                                                
./metatag/metatag.install                                                                            
./microdata/microdata.install                                                                        
./mollom/mollom.install                                                                              
./multilink/multilink.install                                                                        
./navigate/navigate.install                                                                          
./oempro/oempro.install                                                                              
./og/og.install                                                                                      
./chili_highlighter/chili_highlighter.install                                                        
./coder/coder_review/coder_review.install                                                            
./pagepreview/pagepreview.install                                                                    
./panels_hash_cache/panels_hash_cache.install                                                        
./path_breadcrumbs/path_breadcrumbs.install                                                          
./performance_hacks/performance_hacks.install                                                        
./petfinder/petfinder.install                                                                        
./ph/ph_package/ph_package.install                                                                   
./photos/photos.install                                                                              
./picasa_node_album/picasa_node_album.install                                                        
./pmb_connector/pmb/pmb.install
./power_menu/power_menu.install
./proxy/proxy.install
./rest_api_query/rest_api_query.install
./route/route.install
./salsa_entity/salsa_entity.module
./saml_sp/saml_sp.install
./scald/scald.install
./search_api_sphinx/search_api_sphinx.install
./session_cache/session_cache.install
./shadow/shadow.install
./shorten/shorten.install
./simplemeta/simplemeta.install
./skimlinks/everyfeed/everyfeed.install
./smart_breadcrumb/smart_breadcrumb.install
./smart_ip/smart_ip.install
./styles/contrib/file_styles/file_styles.install
./styles/styles.install
./tableofcontents/tableofcontents.install
./TabT/tabt.install
./tdt/thedatatank.install
./tmgmt/tmgmt.install
./token/token.install
./topspin/topspin.install
./twitter_pull/twitter_pull.install
./user_relationship_locator/user_relationship_locator_facebook/user_relationship_locator_facebook.install
./views/views.install
./vimeo/vimeo.install
./walkthru/walkthru.install
./wbapi/wbapi.install
./web_taxonomy/web_taxonomy.install
./widgets/widgets.install
./zend_feed/zend_feed.install
ron_s’s picture

Mike, question about this patch... are you suggesting the cache, cache_bootstrap, cache_menu, semaphore, and variable tables should be MyISAM due to performance benefits for high traffic sites?

From reading the WorkHabit post, I wasn't sure if Nick was suggesting that converting existing MyISAM tables to InnoDB is the problem, or if it is also an issue for sites which were originally built using all InnoDB tables.

Thanks for your insights.

Ron

mikeytown2’s picture

This issue doesn't matter if the tables where always InnoDB or if they were converted from MyISAM.

If the semaphore table is not MyISAM, multiple processes can acquire the same lock; thus it is critical for this table to be MyISAM. This is a bug. All other table changes are for speed.

From what I've seen, the only core cache tables that benefit from InnoDB are cache_form, cache_field, and cache_page. cache_menu is a toss up depending on the site, InnoDB or MyISAM is better; but with what I've seen so far MyISAM performs better.

Writes to the variable table should be rare, and I was seeing issues with queries taking a long time with this table. Once I switched it over to MyISAM, things started to look better.

gielfeldt’s picture

If the semaphore table is not MyISAM, multiple processes can acquire the same lock; thus it is critical for this table to be MyISAM.

Are you sure about this?

This sounds like a bug that would be discovered quite fast, yet I cannot find any information on this except this thread and the WorkHabit blog?

mikeytown2’s picture

The bug would be very hard to reproduce due to 2 inserts hitting the semaphore table at the same time. Once a MySQL database gets enough traffic (baseline - thousands of transactions per second) the bug will happen about once a month. The end result of this is usually a corrupt cache that needs to be flushed and/or the database locking up for multiple seconds (deadlock).

Usually most people switch to memcache once issues like this arise, thus the locking issue goes away. We were starting to think about switching this site over to memcache but I wanted to try changing the db engine after issuing SHOW PROCESSLIST and seeing lots of entries for the semaphore table. Long story short, we are still not using memcache for this site (4 months later & with more traffic) and we haven't had issues with the cache getting corrupted.

An alternative to using MyISAM would be to lock the table before doing any writes to it.

This isn't the first race condition I've found BTW :)
http://drupal.org/node/261148#comment-5798678

erikwebb’s picture

I'm not sure how InnoDB would be causing this problem. 2 writing queries can't be executed at the exact same time, because row locking would jump in. You shouldn't need table-level locking, because each row represents a unique lock/variable/whatever and shouldn't need to be locked as a group. Are you saying this will only happen for a new entry or for any concurrent write to these tables?

mikeytown2’s picture

I've seen the processlist fill up with over 100 entries trying to interact with the semaphore table. There could be a bug in MySQL in regards to this.

I don't know why InnoDB is causing this issue but it is reproducible when MySQL is under a lot of load. I'm guessing it's not widely reported because once one sees a lot of errors related to the Drupal cache, most people switch away from database caching.

gielfeldt’s picture

Hmmm..

It could be some next/gap-lock deadlock, if a semaphore is placed/released within a transaction. If this is the case, maybe switching to read-committed isolation level will solve it as well?

erikwebb’s picture

Are there any conditions in Drupal where a transaction could be opened and hang before closing? That may cause some of these issues.

vinoth.3v’s picture

I could not change that table back to MYISAM due to 'MySQL: Specified key was too long; max key length is 1000 bytes' error!

but I tried to change it to mysql MEMORY engine. seems working fine.

mikeytown2’s picture

Issue summary:View changes
Status:Needs review» Closed (duplicate)

Going to close this issue and mark it a duplicate of #2164849: Enforce READ COMMITTED transaction isolation level.
By switching the table to MyISAM, we break out of the transaction but lose row level locking. Fixing this properly by running the query outside of the transaction sounds like the proper fix for this.

mikeytown2’s picture

Title:Do not use InnoDB for the semaphore table» Do not use InnoDB for the semaphore table, use Memory
Status:Closed (duplicate)» Active

Still getting deadlocks on the semaphore table. Changing the engine to memory seems to help; just be sure to use a BTREE index instead of a HASH index. HASH is the default for MEMORY and still has issues that BTREE fixes.
http://www.mysqlperformanceblog.com/2008/02/01/performance-gotcha-of-mys...

ALTER TABLE semaphore ENGINE = MEMORY;
ALTER TABLE semaphore DROP PRIMARY KEY;
ALTER TABLE semaphore ADD PRIMARY KEY (name) USING BTREE;
ALTER TABLE semaphore DROP INDEX value;
ALTER TABLE semaphore ADD INDEX value (value) USING BTREE;
ALTER TABLE semaphore DROP INDEX expire;
ALTER TABLE semaphore ADD INDEX expire (expire) USING BTREE;
vinoth.3v’s picture

Also consider to add a index on (name, value) columns,

mikeytown2’s picture

@vinoth.3V
You do have a point.

lock_may_be_available() is the only place that has a condition on name. Almost all other locking code puts a condition on the name and value; the exception to this is lock_release_all which puts a condition on the value.

I do think that would be a different issue though, system_schema() defines the indexes used.

Question is what would be ideal. Have the name value be a primary key & add in a unique name key.

ALTER TABLE semaphore ENGINE = MEMORY;
ALTER TABLE semaphore DROP PRIMARY KEY;
ALTER TABLE semaphore ADD PRIMARY KEY (name, value) USING BTREE;
ALTER TABLE semaphore ADD UNIQUE name (name) USING BTREE;
ALTER TABLE semaphore DROP INDEX value;
ALTER TABLE semaphore ADD INDEX value (value) USING BTREE;
ALTER TABLE semaphore DROP INDEX expire;
ALTER TABLE semaphore ADD INDEX expire (expire) USING BTREE;

Or add in another index called namevalue.

ALTER TABLE semaphore ENGINE = MEMORY;
ALTER TABLE semaphore DROP PRIMARY KEY;
ALTER TABLE semaphore ADD PRIMARY KEY (name) USING BTREE;
ALTER TABLE semaphore DROP INDEX value;
ALTER TABLE semaphore ADD INDEX value (value) USING BTREE;
ALTER TABLE semaphore DROP INDEX expire;
ALTER TABLE semaphore ADD INDEX expire (expire) USING BTREE;
ALTER TABLE semaphore ADD INDEX namevalue (name, value) USING BTREE;

Or does it really matter as there is almost always less than 100 rows in semaphore. If you have more, something is wrong most likely.

mikeytown2’s picture

MySQL might need the key names to be quoted. Remove the -- if running multiple times.

ALTER TABLE semaphore ENGINE = MEMORY;
ALTER TABLE semaphore DROP PRIMARY KEY;
ALTER TABLE semaphore ADD PRIMARY KEY (`name`, `value`) USING BTREE;
-- ALTER TABLE semaphore DROP INDEX name;
ALTER TABLE semaphore ADD UNIQUE name (`name`) USING BTREE;
ALTER TABLE semaphore DROP INDEX value;
ALTER TABLE semaphore ADD INDEX value (`value`) USING BTREE;
ALTER TABLE semaphore DROP INDEX expire;
ALTER TABLE semaphore ADD INDEX expire (`expire`) USING BTREE;

Or If MySQL is having issues with btrees in memory tables this might be the best option

ALTER TABLE semaphore ENGINE = MEMORY;
ALTER TABLE semaphore DROP PRIMARY KEY;
ALTER TABLE semaphore ADD PRIMARY KEY (name);
ALTER TABLE semaphore DROP INDEX value;
ALTER TABLE semaphore ADD INDEX value (value);
ALTER TABLE semaphore DROP INDEX expire;
ALTER TABLE semaphore ADD INDEX expire (expire);
ALTER TABLE semaphore ADD INDEX namevalue (name, value);
skomorokh’s picture

I can confirm that this happens and that memory tables help. Was doing some load testing and MySQL had deadlocks on the semaphore table. Found this, switched to a memory table as per #20 and those stop.

mikeytown2’s picture

Starting to look into creating a patch for this and it doesn't look good...

DatabaseSchema_mysql::createKeysSql does not support index_type http://dev.mysql.com/doc/refman/5.0/en/create-index.html

Storage Engine Permissible Index Types
MyISAM BTREE
InnoDB BTREE
MEMORY/HEAP HASH, BTREE
NDB BTREE, HASH

Looks like no one has tried to create an index using the non default index type; because MyISAM and InnoDB only support BTREE.
https://www.google.com/search?q=site%3Adrupal.org%2Fnode+createKeysSql+i...

mikeytown2’s picture

Status:Active» Needs review
StatusFileSize
new529 bytes
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Well for now I'll create a patch using the default index type which for memory tables is HASH.

This patch just changes the defined schema. I'll work on later patches to update existing installs.

Status:Needs review» Needs work

The last submitted patch, 23: drupal-1898204-23-semaphore-memory-table.patch, failed testing.

mikeytown2’s picture

Status:Needs work» Needs review
StatusFileSize
new381 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,950 pass(es).
[ View ]

Doing something even simpler.

mikeytown2’s picture

Issue summary:View changes
mikeytown2’s picture

Issue summary:View changes
mikeytown2’s picture

Issue tags:+innoDB, +Performance, +semaphore
danblack’s picture

Status:Needs review» Reviewed & tested by the community

nice patch in #25 @mikeytown2. Works for me.

The mysqlperformanceblog has an issue with HASH based index to a large number of index duplicates as the index value is always the same. This isn't applicable here as the value is a random value and the primary key is unique.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 25: drupal-1898204-25-semaphore-memory-table.patch, failed testing.

danblack’s picture

Status:Needs work» Needs review
mikeytown2’s picture

Status:Needs review» Reviewed & tested by the community

Moving back to RTBC

Crell’s picture

Version:7.x-dev» 8.x-dev

Shouldn't this go into 8.x first? I'm OK with it, but it should go to 8.x first. :-)

mikeytown2’s picture

StatusFileSize
new399 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,628 pass(es).
[ View ]

D8 Patch. Same as D7 except for paths.

mikeytown2’s picture

Patch passes tests for D8

catch’s picture

Category:Feature request» Bug report
Status:Reviewed & tested by the community» Needs review
Issue tags:+Needs issue summary update

This needs an issue summary update to summarize the issue better. The 20th slide behind a registration form is a bit hard work...

Moving this to memory means it won't respect transactions. i.e. if you acquire a lock within a transaction, and that transaction gets rolled back, then the lock will persist until the end of the request. Before we'd clear the lock with the rest of the transaction. This seems like it's probably fine, but didn't see that discussed anywhere.

mikeytown2’s picture

Looks like DatabaseLockBackend::releaseAll() will get ran due to the constructor registering it in a shutdown function similar to D7 in _lock_id(). I wonder why the api.d.o site didn't pick up this usage, I ended up searching the code base to make sure this is happening.

Using a backend like memcache will have the same transactional issue. We could release the lock in the catch statement, but that would mean the lock id would need to be known at the catch scope; something that might be hard to get right. Also we could make a guideline that transactions should be started after a lock, similar to how it is being used in RouterRebuildSubscriber::menuLinksRebuild().

Berdir’s picture

The first argument is fairly irrelevant I think because that happens at the end of the request and not when a transaction is rolled back.

The second paragraph is spot on, though, lock is a pluggable backend, so the fact that it happens in the same transaction is not something that can be relied upon, just like caching (although core does in some cases, and there can be nasty side effects due to that, both when it's in the same transaction and when it isn't).

mikeytown2’s picture

Issue summary:View changes
catch’s picture

Right it's true that with memcache and etc. the same thing can happen, just wondering if we might hit conditions with the database backend that we previously didn't as a side effect of this patch.

catch’s picture

This doesn't feel right, but I've not tried using the memory storage engine before, so I don't have incredibly concrete reasons why. Asked nnewton to have a look and comment.

gielfeldt’s picture

I'm not sure if I lost track of this thread. I think maybe I did.

Did we find out if the table locking would cause an issue under high concurrent load? And did switching to READ-COMMITTED not solve the problem?

nnewton’s picture

I worry about this. Adding table level locking to semaphores doesn't seem like a positive. I realize the table is small, but it is used often...very often. I would echo gielfeldt's question. The times I've seen semaphore being a large problem under load was with the default tx_isolation.

-N

mikeytown2’s picture

I was still having issues with this table when using READ-COMMITTED; once I switched to memory the issues went away. Also noted that I'm using a BTREE index instead of HASH due to this http://www.mysqlperformanceblog.com/2008/02/01/performance-gotcha-of-mys... where they report HASH gives 80 deletes per second (12.5ms table locked) and BTREE gives 15,000 of deletes per second for the memory table. Having a table level lock last for 0.07ms is a good tradeoff if that eliminates deadlocks.

nnewton’s picture

Interesting. I don't suppose you have any of the innodb output or lock monitor information? I'm curious as to what exactly you were hitting.

I was having a hard time articulating to myself why I was uncomfortable with this, because its clearly a valid solution. The MEMORY engine, especially recent versions of it, has gotten a lot more usable and this is basically what its for. I think at the end of the day it just feels like premature optimization for a lot of installations.

mikeytown2’s picture

StatusFileSize
new25.11 KB
new43.11 KB

Noted that most people switch to memcache once they start having issues with the cache and semaphore tables. I'll admit that there are not a lot of data points for this change, but a google search for "drupal semaphore deadlock" returns 226k results so people are having this issue. We have a nagios alert that will issue SHOW PROCESSLIST; and SHOW ENGINE innodb STATUS; once a certain number of active database threads reaches a threshold. I've attached our last 2 to this issue.

Currently we'll get a deadlock about once every other day on the cache_field table (totally fine) but the last trigger of our nagios alert happened on april 9th (when things could be going bad).

The site in question is 99% logged in users with 3 webheads with master slave MySQL replication; all queries hitting the master currently at a rate of 1 million questions per hour.

The deadlocks on the semaphore table were not causing major issues for us but they where still happening by checking SHOW ENGINE innodb STATUS; moving semaphore to memory fixed those deadlocks.

If you really need it I could change the engine back to innodb and wait for a deadlock on the semaphore table if you need that. I would rather not, but if information like this is what is preventing this patch from getting accepted then I'll be willing to do it next week.

andypost’s picture

Priority:Normal» Major
Issue summary:View changes
Issue tags:-Needs issue summary update+needs backport to D7

Interesting bug, suppose this is a major issue that "lock" (expiring mutex) could be released by any transaction.
Updated summary with that

pwolanin’s picture

Issue tags:+needs backport to D6

Should backport it all the way. So the 7 and 6 patches will need an update function to change the engine?

mikeytown2’s picture

pwolanin’s picture

Discussed with catch in IRC, given the unknowns (especially of the upgrade path) a core backport doesn't make sense. possibly it could be a pressflow feature.

David Strauss’s picture

chx asked me to comment here, but I don't have much to say other than recommending Redis (or similar) as the locking backend. Using MySQL/MariaDB for locking via the MEMORY engine may be a marginal improvement, but it will never be the best practice.

Fabianx’s picture

To avoid the deadlock issues can we just use another DB connection? To avoid the lock being part of the transaction?

For the lock backend this could be pretty simple implemented by using another 'database key' instead in D7-D8.

That would not even need a core patch then.

The same should be done for cache, which then even for the default DB backend, well the DB backend which uses a different DB key, should use cache_consistent module.

If we were to switch 8.x to use a different MySQL connection for locks and caches, the nice thing is that we would get lots of new failures, where before we took cache consistency for granted with DB backend - but break all others.

On the other hand its one more DB connection per PHP thread.

mikeytown2’s picture

Using another DB connection would help; but as you mention it's another DB connection and people do have issues with hitting the 150 MySQL connection limit. It does give us more options which is nice.

If we were to go this route I would say we create options do do sync and async queries as the async query would have other benefits as well; faster cache sets, preemptive gets, etc. If we're going to play with multiple db connections we might as well throw async into the mix as well.

Crell’s picture

This all sounds like something that should be done in an optional alternate service now that we support those more directly. Definitely if we're talking about MySQL-specific features like memory tables and async queries. (Side note: async queries may be fine for caching but entirely defeat the point for a lock table.)

mikeytown2’s picture

kwerey’s picture

I'm glad to find this thread... I'm commenting to give a different example of this row locking issue for a non-busy non-production site. I've come across it before in Drupal 7.28 on a production site on shared hosting, and now it's shown up again copying a production database down onto a local dev site. I don't know have much experience in database optimisation so it's good to see ideas being thrown around.

This is Drupal 7.34 using Innodb and MySQL 5.5.23, the stray trx locks started after copying the database down from the production site (on a VPS) onto my work machine running Windows. There are a fair few third party modules on the site, but all of the ones that're removed have been uninstalled correctly. As recommended elsewhere I've made sure the innodb_buffer_pool_size is generous - I set it to a gig.

I think what's happening isn't exactly the case Mikey described with two transactions just happening to take place at the precise right time - it looks like transactions are hanging somehow and/or locks never being released even though the transaction's finished. F'rexample whatever this is seems to be locking a lot of rows for 4 whole minutes.

------------------
---TRANSACTION 75653E, ACTIVE 247 sec
27 lock struct(s), heap size 2496, 12 row lock(s), undo log entries 51
MySQL thread id 1658, OS thread handle 0x1f6c, query id 493965 localhost 127.0.0.1 root
Trx read view will not see trx with id >= 75653F, sees < 75653F

I uploaded the whole innodb status log - if anyone's interested in troubleshooting let me know and I can try and replicate stuff.

kwerey’s picture

StatusFileSize
new7.69 KB
Crell’s picture

Note that for Drupal 8, https://www.drupal.org/node/2306083 combined with #2371709: Move the on-demand-table creation into DBTNG may make this process a lot easier to optimize per-site. (Let an alternate backend provide a different schema with a different engine for that one use case, poof.)

nnewton’s picture

I had the opportunity to test this a little bit recently. The MEMORY engine isn't usable for this at high concurrency. Trading row level locks for table level locks...not an improvement :)

Its possible that changing this could help for some largish mid-level sites, but as David said this just isn't the best practice. I don't see the point of changing this for a very small percentage of sites. Smaller/mid-level sites won't need it, InnoDB is fine (if its not fine, its miss-configured or something else is happening). For sites that have _just_ outgrown InnoDB for this and are not yet requiring Memcache/Redis for locking, it would help. However, that is quite a small window to be shooting for.

mikeytown2’s picture

@nnewton
Did you use hash or btree indexes on the memory table?
http://www.percona.com/blog/2008/02/01/performance-gotcha-of-mysql-memor...

nnewton’s picture

Ya, I used BTree. Even if I had used HASH though, I don't think I would have hit that issue because it was failing 'above' that. These do table level locking. They have the same issue that the query cache does at high concurrency. The lock is simply too hot.

-N

mikeytown2’s picture

Thanks for the info nnewton
Created this issue #2387371: Create a lock back end that uses the APDQC connection so locks can happen outside of a transaction when using a DB backend.

Crell’s picture

nnewton: Then should we file this as a won't fix, since there are better options?

mikeytown2’s picture

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

The main issue is you can't guarantee a lock will not be taken/released inside of a transaction; thus using the default connection can result in bad things from happening. Using a non-transactional table is a quick fix that works surprisingly well. I have gotten reports that switching the semaphore table to memory helps with deadlocks
https://twitter.com/btopro/status/502914571058024448

Using a secondary db connection (what my new db backed cache module does) would another way to fix this issue.

Obviously something like memcache is the preferred way to handle locks; but not everyone wants to setup a memcache server. It's true that InnoDB is crazy fast; so much so you can use the memcache api to interface with that engine directly http://dev.mysql.com/doc/refman/5.6/en/innodb-memcached-replication.html and it's been getting tuned like crazy in MySQL 5.7; and the InnoDB performance from 5.5 to 5.6 was a big improvement as well.

I'm thinking that if you're using MySQL 5.5 or less, the memory table might be better than InnoDB; If you're using MySQL 5.6 or higher, InnoDB is will probably be better. I'll take this into consideration for apdqc; offering to convert to memory if MySQL is 5.5 or less; convert to InnoDB if MySQL 5.6 or higher.

1st: non-db locking (memcache or equivalent).
2nd: apdqc locking (no eta currently).
3rd: memory table (no deadlocks).
4th: core.

So I'm thinking even though using the memory table does fix things; the better fix is to use a new db connection. For today I would say the memory table is better due to no deadlocks, but we don't want to force this via core. Using a 2nd db connection would be the correct fix for core.

There is a ~8 month window between when PHP 5.4.5 was released and when MySQL 5.6 was GA, so we can expect to see some MySQL 5.5 installs with D8.

mikeytown2’s picture

There is one other alternative to MEMORY/InnoDB: MySQL_Cluster. Seems nuts but I thought I would point this out just in case any one is interested in this http://www.clusterdb.com/mysql-cluster/replacing-memory-storage-engine-w...

BTW since switching the semaphore table back to InnoDB I've been getting deadlock reports but no down time due to the usage of APDQC. It puts lock queries in a different connection and thus outside of any transaction.

Latest deadlock report from prod below:

------------------------
LATEST DETECTED DEADLOCK
------------------------
2015-02-19 13:04:26 7fd4d60e2700
*** (1) TRANSACTION:
TRANSACTION 94404352067, ACTIVE 0 sec inserting
mysql tables in use 1, locked 1
LOCK WAIT 4 lock struct(s), heap size 1184, 9 row lock(s), undo log entries 1
MySQL thread id 13629753, OS thread handle 0x7fd4d8572700, query id 2112336588 10.112.0.201 myds update
INSERT INTO semaphore
       (name, value, expire)
       VALUES ('theme_registry:runtime:datasphere:cache', '187411741954e64fd720b7b8.27149747', 1424379896.3244)
*** (1) WAITING FOR THIS LOCK TO BE GRANTED:
RECORD LOCKS space id 4021130 page no 4 n bits 120 index `name` of table `merchant_portal`.`semaphore` trx id 94404352067 lock_mode X locks gap before rec insert intention waiting
Record lock, heap no 43 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 32313032313935383435346536346664363935633733342e363734303932; asc 21021958454e64fd695c734.674092; (total 32 bytes);

*** (2) TRANSACTION:
TRANSACTION 94404352108, ACTIVE 0 sec inserting
mysql tables in use 1, locked 1
4 lock struct(s), heap size 1184, 9 row lock(s), undo log entries 1
MySQL thread id 13629711, OS thread handle 0x7fd4d60e2700, query id 2112336628 10.112.0.203 myds update
INSERT INTO semaphore
       (name, value, expire)
       VALUES ('theme_registry:runtime:datasphere:cache', '118714896854e64fd54a3021.88813258', 1424379896.3439)
*** (2) HOLDS THE LOCK(S):
RECORD LOCKS space id 4021130 page no 4 n bits 120 index `name` of table `merchant_portal`.`semaphore` trx id 94404352108 lock mode S
Record lock, heap no 1 PHYSICAL RECORD: n_fields 1; compact format; info bits 0
0: len 8; hex 73757072656d756d; asc supremum;;

Record lock, heap no 36 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 373531323839323535346536346664393861396264372e39393738303430; asc 7512892554e64fd98a9bd7.9978040; (total 31 bytes);

Record lock, heap no 37 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 37333337363632353735346536346664396136323639392e393936373630; asc 73376625754e64fd9a62699.996760; (total 32 bytes);

Record lock, heap no 41 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 3135393133333133373535346536346664353439313934302e3537343831; asc 159133137554e64fd5491940.57481; (total 33 bytes);

Record lock, heap no 43 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 32313032313935383435346536346664363935633733342e363734303932; asc 21021958454e64fd695c734.674092; (total 32 bytes);

Record lock, heap no 44 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 353737323837393135346536346664356265383238372e37323330303837; asc 5772879154e64fd5be8287.7230087; (total 31 bytes);

Record lock, heap no 46 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 3136313635383131383335346536346664613230643465352e3032363435; asc 161658118354e64fda20d4e5.02645; (total 33 bytes);

*** (2) WAITING FOR THIS LOCK TO BE GRANTED:
RECORD LOCKS space id 4021130 page no 4 n bits 120 index `name` of table `merchant_portal`.`semaphore` trx id 94404352108 lock_mode X locks gap before rec insert intention waiting
Record lock, heap no 41 PHYSICAL RECORD: n_fields 2; compact format; info bits 32
0: len 30; hex 7468656d655f72656769737472793a72756e74696d653a64617461737068; asc theme_registry:runtime:datasph; (total 39 bytes);
1: len 30; hex 3135393133333133373535346536346664353439313934302e3537343831; asc 159133137554e64fd5491940.57481; (total 33 bytes);

*** WE ROLL BACK TRANSACTION (2)

You can see that these 2 queries came in right around 20ms apart (i've seen 0.1ms before); same key and the second one fails. All is good, just slightly annoying because potential deadlocks on another table will get pushed out of this report as this deadlocks about once an hour.

gonssal’s picture

Not fixing this for Drupal 8 would be a great failure. The semaphore table is one of the biggest problems in any big Drupal project I've worked on.