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

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...

Files: 
CommentFileSizeAuthor
#25 drupal-1898204-25-semaphore-memory-table.patch381 bytesmikeytown2
PASSED: [[SimpleTest]]: [MySQL] 40,716 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

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);
  }
}
?>

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

Patch to fix fresh installs.

Status:Active» Needs review

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

Last patch has a typo in it.

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

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

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.

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?

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

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?

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.

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?

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

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.

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.

Title:Do not use InnoDB for the semaphore tableDo 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;

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

@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.

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);

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.

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...

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.

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

Doing something even simpler.

Issue summary:View changes

Issue summary:View changes

Issue tags:+innoDB, +Performance, +semaphore