From: John David Duncan Date: May 17 2011 5:23am Subject: [Resend] bzr commit into mysql-5.1-telco-7.2 branch (john.duncan:4180) List-Archive: http://lists.mysql.com/commits/137496 Message-Id: <201105170524.p4H5O2MI010239@acsmt358.oracle.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0552560009==" --===============0552560009== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline [This commit e-mail is a repeat.] #At file:///Users/jdd/bzr-repo/working/cluster-7.2-labs-memcached/ based on revid:john.duncan@stripped 4180 John David Duncan 2011-05-16 New Scheduler::yield() method. Fix a possible race condition in the Flex and Stockholm schedulers where notify_io_complete() is called, and the worker thread invalidates an Ndb, before sendPollNdb() on that Ndb has actually returned in the commit thread. So, notify_io_complete() must be called from the commit thread loop in Stockholm and Flex, but from the callback function in Bulk. modified: storage/ndb/memcache/include/Scheduler.h storage/ndb/memcache/src/ndb_worker.cc storage/ndb/memcache/src/schedulers/Bulk.cc storage/ndb/memcache/src/schedulers/Bulk.h storage/ndb/memcache/src/schedulers/Flex.h storage/ndb/memcache/src/schedulers/Flex_cluster.cc storage/ndb/memcache/src/schedulers/Stockholm.cc storage/ndb/memcache/src/schedulers/Stockholm.h === modified file 'storage/ndb/memcache/include/Scheduler.h' --- a/storage/ndb/memcache/include/Scheduler.h 2011-05-17 04:55:39 +0000 +++ b/storage/ndb/memcache/include/Scheduler.h 2011-05-17 05:22:56 +0000 @@ -30,6 +30,7 @@ #include "Configuration.h" #include "thread_identifier.h" + /* Scheduler is an interface */ class Scheduler { @@ -54,10 +55,15 @@ public: an Ndb object for the operation and send the workitem to be executed. */ virtual ENGINE_ERROR_CODE schedule(workitem *) = 0; - /** when a workitem requires multiple NDB operations, reschedule() is used - to schedule the second and subsequent ones. */ + /** Before an NDB callback function completes, it must call either + reschedule() or yield(). yield() indicates that work is comlpete. */ + virtual void yield(workitem *) const = 0; + + /** Before an NDB callback function completes, it must call either + reschedule() or yield(). reschedule() indicates to that the workitem + requires the scheduler to send & poll an additional operation. */ virtual void reschedule(workitem *) const = 0; - + /** io_completed() is called from the NDB Engine thread when an IO completion notification has been received */ virtual void io_completed(workitem *) = 0; === modified file 'storage/ndb/memcache/src/ndb_worker.cc' --- a/storage/ndb/memcache/src/ndb_worker.cc 2011-05-17 05:07:20 +0000 +++ b/storage/ndb/memcache/src/ndb_worker.cc 2011-05-17 05:22:56 +0000 @@ -548,14 +548,14 @@ void DB_callback(int result, NdbTransact if(wqitem->base.is_sync) { wqitem->status = return_status; pipeline->engine->server.cookie->store_engine_specific(wqitem->cookie, wqitem); - pipeline->engine->server.cookie->notify_io_complete(wqitem->cookie, io_status); + pipeline->scheduler->yield(wqitem); } else { /* The workitem was allocated back in the engine thread; if used in a callback, it would be freed there, too. But we must free it here. */ pipeline->engine->server.cookie->store_engine_specific(wqitem->cookie, wqitem->previous); - pipeline_io_completed(pipeline, wqitem); + pipeline->scheduler->io_completed(wqitem); workitem_free(wqitem); } } @@ -571,7 +571,7 @@ void rewrite_callback(int result, NdbTra item->status = & status_block_bad_replace; tx->close(); item->pipeline->engine->server.cookie->store_engine_specific(item->cookie, item); - item->pipeline->engine->server.cookie->notify_io_complete(item->cookie, ENGINE_SUCCESS); + item->pipeline->scheduler->yield(item); return; } else if(tx->getNdbError().classification != NdbError::NoError) { @@ -734,13 +734,13 @@ void incr_callback(int result, NdbTransa if(wqitem->base.is_sync) { wqitem->status = return_status; pipeline->engine->server.cookie->store_engine_specific(wqitem->cookie, wqitem); - pipeline->engine->server.cookie->notify_io_complete(wqitem->cookie, io_status); + pipeline->scheduler->yield(wqitem); } else { /* The workitem was allocated back in the engine thread; if used in a callback, it would be freed there, too. But we must free it here. */ pipeline->engine->server.cookie->store_engine_specific(wqitem->cookie, wqitem->previous); - pipeline_io_completed(pipeline, wqitem); + pipeline->scheduler->io_completed(wqitem); workitem_free(wqitem); } } === modified file 'storage/ndb/memcache/src/schedulers/Bulk.cc' --- a/storage/ndb/memcache/src/schedulers/Bulk.cc 2011-05-17 05:07:20 +0000 +++ b/storage/ndb/memcache/src/schedulers/Bulk.cc 2011-05-17 05:22:56 +0000 @@ -283,3 +283,17 @@ void * Scheduler_bulk::run_ndb_commit_th cluster[c].stats.vtime = get_thread_vtime(); } } + + +void Scheduler_bulk::reschedule(workitem *item) const { + LockableNdbInstance * inst = (LockableNdbInstance *) item->ndb_instance ; + DEBUG_ASSERT(inst->is_locked); + inst->npending++; +} + + +void Scheduler_bulk::yield(workitem *item) const { + /* In this scheduler, a small number of Ndb objects are shared, + so we call notify_io_complete() while the callback is still running. */ + pipeline->engine->server.cookie->notify_io_complete(item->cookie, ENGINE_SUCCESS); +} === modified file 'storage/ndb/memcache/src/schedulers/Bulk.h' --- a/storage/ndb/memcache/src/schedulers/Bulk.h 2011-05-17 04:55:39 +0000 +++ b/storage/ndb/memcache/src/schedulers/Bulk.h 2011-05-17 05:22:56 +0000 @@ -43,7 +43,8 @@ public: void init(int threadnum, int nthreads, const char *config_string); void attach_thread(thread_identifier *); ENGINE_ERROR_CODE schedule(workitem *); - void reschedule(workitem *) const; // inlined + void yield(workitem *) const; + void reschedule(workitem *) const; void io_completed(workitem *); void add_stats(ADD_STAT, const void *); void * run_ndb_commit_thread(int thread_id); @@ -63,11 +64,5 @@ private: }; -inline void Scheduler_bulk::reschedule(workitem *item) const { - LockableNdbInstance * inst = (LockableNdbInstance *) item->ndb_instance ; - assert(inst->is_locked); - inst->npending++; -} - #endif === modified file 'storage/ndb/memcache/src/schedulers/Flex.h' --- a/storage/ndb/memcache/src/schedulers/Flex.h 2011-05-17 04:55:39 +0000 +++ b/storage/ndb/memcache/src/schedulers/Flex.h 2011-05-17 05:22:56 +0000 @@ -56,7 +56,8 @@ public: void init(int threadnum, int nthreads, const char *config_string); void attach_thread(thread_identifier *); ENGINE_ERROR_CODE schedule(workitem *); - void reschedule(workitem *) const; // inlined + void yield(workitem *) const; // inlined + void reschedule(workitem *) const; // inlined void io_completed(workitem *); void add_stats(ADD_STAT, const void *); @@ -88,6 +89,8 @@ inline void Scheduler_flex::reschedule(w item->base.reschedule = 1; } +inline void Scheduler_flex::yield(workitem *item) const { } + /* Random stat samples will on average be taken twice as often as FLEX_STATS_SAMPLE_INTERVAL (based on uniform random distribution). === modified file 'storage/ndb/memcache/src/schedulers/Flex_cluster.cc' --- a/storage/ndb/memcache/src/schedulers/Flex_cluster.cc 2011-05-17 05:15:50 +0000 +++ b/storage/ndb/memcache/src/schedulers/Flex_cluster.cc 2011-05-17 05:22:56 +0000 @@ -234,6 +234,10 @@ void * Scheduler_flex::Cluster::run_comm item->base.reschedule = 0; polled = item->ndb_instance->db->sendPollNdb(10, 1, 1); } while(item->base.reschedule || ! polled); + + /* Now that sendPollNdb() has returned, it is OK to notify_io_complete(), + which will trigger the worker thread to release the Ndb instance. */ + item->pipeline->engine->server.cookie->notify_io_complete(item->cookie, ENGINE_SUCCESS); } return NULL; === modified file 'storage/ndb/memcache/src/schedulers/Stockholm.cc' --- a/storage/ndb/memcache/src/schedulers/Stockholm.cc 2011-05-17 05:15:50 +0000 +++ b/storage/ndb/memcache/src/schedulers/Stockholm.cc 2011-05-17 05:22:56 +0000 @@ -269,9 +269,13 @@ void * Scheduler_stockholm::run_ndb_comm polled = item->ndb_instance->db->sendPollNdb(10, 1, 1); } while(item->base.reschedule || ! polled); + DEBUG_ASSERT(polled == 1); // i.e. not > 1 - cluster[c].stats.cycles++; - if(! (cluster[c].stats.cycles % STAT_INTERVAL)) + /* Now that sendPollNdb() has returned, it is OK to notify_io_complete(), + which will trigger the worker thread to release the Ndb instance. */ + pipeline->engine->server.cookie->notify_io_complete(item->cookie, ENGINE_SUCCESS); + + if(! (cluster[c].stats.cycles++ % STAT_INTERVAL)) cluster[c].stats.commit_thread_vtime = get_thread_vtime(); } === modified file 'storage/ndb/memcache/src/schedulers/Stockholm.h' --- a/storage/ndb/memcache/src/schedulers/Stockholm.h 2011-05-17 04:55:39 +0000 +++ b/storage/ndb/memcache/src/schedulers/Stockholm.h 2011-05-17 05:22:56 +0000 @@ -46,7 +46,8 @@ public: void init(int threadnum, int nthreads, const char *config_string); void attach_thread(thread_identifier *); ENGINE_ERROR_CODE schedule(workitem *); - void reschedule(workitem *) const; // inlined + void yield(workitem *) const; // inlined + void reschedule(workitem *) const; // inlined void io_completed(workitem *); void add_stats(ADD_STAT, const void *); void * run_ndb_commit_thread(int cluster_id); @@ -71,5 +72,8 @@ inline void Scheduler_stockholm::resched } +inline void Scheduler_stockholm::yield(workitem *item) const { } + + #endif --===============0552560009== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/john.duncan@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: john.duncan@stripped # target_branch: file:///Users/jdd/bzr-repo/working/cluster-7.2-labs-\ # memcached/ # testament_sha1: 9fcc7479dc06023ed2c0267d4d30819070ca5e5b # timestamp: 2011-05-16 22:23:55 -0700 # base_revision_id: john.duncan@stripped\ # wogdvu21wzmzjgt9 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWaqEwT4ABzZ/gHgQAAF7f/// f73fCr////pgC/vvGq8jgO5qVVKlLjWAUoUAFKEQA4yNMmJoMmTCaZAyGgNAaZNDACaAwlIAaRkp 6ZUyNDQaAAGmgAAAaaAGiaZEjTKDQbU/VNGgAAAA9QAaBoAJERNCJozUNT0kehtTGlPyoyA0AADa hoHqDjI0yYmgyZMJpkDIaA0Bpk0MAJoDBUkhGgAmERmpinoBPSCeimxqRoAAaHtUYEpgiMjxv64Z 5rklUdRs7PhGZ1ZlJz53qwK/etAT199ePizMjPbRhWnCqKO99cMmW6tqtrqN2ctazqxvwGeSm32X AjraD72kUsgw1cz8VKPXhfcSVq818RE63xX3s0nMJGytOU5+/2FhJF9yQMIDfKlmTQOhu2R1Za8y TsiAqG0D8QNgDbbY22zMjMEXtRfF0wQquQwHRB7ZUoyzFIGuWz4WADVwib2r8ZkqzDpgrhF7CaRR zYmS54NsqNQ8XV4nbpoGOewEtm+RM5VUMo80UXhr/cwEzO/LKWBHBJmfagZd8g1Qd4+tIwIA/nwz 4St9jdXD+3FCgZabFaFC4TewedEDftG/aGL16OYNmBh4OJaFcU2aFEDCTtueQ8WEgDTcJPm+Q1jX VuK9rWDPaPHQwMpjUNY00/0Y2C68wCgek9XV3efsncdfaquvbbjODbZRS1uPIBiY41tbDekr2kBq Du5aGUNw0QUO9xKmIbGmxW1c/X7YMUg2L8mHfQHZxz3G5aYUqH4d39LaWfXOlt3nZi7+/98FW5dZ HZ6lMU02XKUpThJJkff6ng2L4baMkzS5puulrLC6eAeDMWevLfr1dHvyxx8655+zWXZPs5bc260p SzKC015wcbsnXluXLlz+nSOkdP4qn0fyWHaauW0XHj1dMoWr8qXVONJKqOlYVGRDuZIup7wuRJkM L3YxXB3qL6LizMaOkvXSYSSURlDPkZ9JkzMGLkaGtk1MMUXSicXYCIiIUBosLImprpMIATGSxcHU hERSZGTbgAep6Igkpdsy3l4bogiPh0x2ffVr8tcM9lSeTBxUsYCFkDhu/NCRkZg9ShqXmiIkOBxr kKUN+hjRe0NA2utizOTHFycWDnixdZG2MdaWpQIbjkcr2R0sbLGiYIiCTYOiuR/UG61KAhQYRsr2 LnFlZ1Tsyu1iGvqvI37XDtszZ8qrVtxXXWI41us5uDF2X75G1ly/Ve3sltazBo3nIy0VlqxwyHKe ALAoTkZy1bzMSZM3ihVSA7dAZuXFlzOxjZlXa251ZG5i05mjNkcTKpvNy+diA4uQy2NCwIf4Kk7X blkVsjcwgIMzIDQo+GW5179u3Q25WVpus4bKtFr2C1F1OFM6+zYtIhkWJAl0BxoKYHOHGBmbwYOr jN78hjVMYsjawarmCl4WIaVrEbSLWC9NTpka5HPRsi9ZTUWdXY0kaepbjhgm9laJmuXbDDMwNyBY zGEtDI2xsR666ozGcLvaCEpCkQY7WGS6sVlyzJ1NTI672THiy5FcuCN6/qalMjSo8vgjbOmXbETH JZE0REgOpFpMWhUcUkypoGbxxchhVkBw8cMGvW0WQKI2MWNkPtnHA2ZqI0qZmheF+q69dSpdnzNc q74Mrdwsyt+stz1lcKUGhtEG3gaGpMEMyJtPMBsR8bZthN7ygayBSxKUSo4aOWJeNJlWpcYIbQ7R lxSYwrNzzXqXaW5bDY2OO5pfbe0LG5tXsreHqIto5Ep2Pu9y7iuNGOUxyPMhXQgpAh6bRVBekWvd wV36mjW49EEA/qQUXx02BxNpjbaT9obW0By0i7o4AR5lzMBWAD/PA9XrMxxgMBt96VMD+J9k4x/A woUanlKySSVgKORTiTGG+bv6OJ/pYcayTgOCeovgOhXLJDWBTOuSAGf6XaboXvFN/u5OemXvjP5o tuPHpZaSnX8xaR/F/ZgGXJODXgj7lD2LmZ8n5EebB24Y0veb5v0t/B4YNur9txcD26Iwb7T+Djkk eYdf6/z+mefhSX4D03aSgWjNdP0rt3SFTYz2ivhmWxzff7PFlKhR1rD0979Vm30EZe93Nz1MGIZE 6F674+GRi56la+Gp+r19u5oronkH5bKr5NXlEfOodDj0sP0Xe1ycmEunB+/H9nPg2Q9cDm+hOj4L 5kpi93g0MzCsnwSfhSoqS0XeolgeeTv7Z+W0G7trDBCBL1qq2F65M2RWQo9EMHCqSpggOxCoNpsv 69e653OCndOt7Xa8OWy5l2o3vS/rcW1GRuZc0FSvzHfS42Nj3tTq9XOW4uTFmfdCltXXjCXQAHaj 0aU12H5wdDp0gVcwaCeIoHplsYmaprwlpQvxacgjm63NXoZGtcee7Wy+3CqrNVreDKs632uT8RiO 66hvtuRrkWfHQ6427WVzdr69Ze5W1Pvz4XuLQkmpx8drkv51t8n4PRp962byqvfZ3pN7yHGRO7xL sswh/CVI4tHR2TsaKg4TLKVl5aS2GbDPKUwqGZ0ZCWjQUAuYRRJ7boZIL/ZloPF2OCm9d4PJyYMj op3uCmK7B5h9nqYQ0vK9sh1v/B9g9Q/Hz+VrfZwmD5U4LWmgYOLsI4B2R7SeTKyzO83wj0vtJTyk Nc74PS3YJJc8yvk2wLe+FRe7JI5Z73s07XuXb/fpkfLug91tietJp/HTCw0OI1UF29TbHqzyLNKP TKOGjfZhH31upavHswwHm1HOqRXmOnZnY1wrG+RY8W8n+B3NtKmHY7nBj/9uHL4e6PaRzke52woW 9Hxe4NLF6zxrTJcW5EXLOasEqHrT1SLie0jtXugYxpqVzPgcpF93eFS09tFknvHZWaTYCwuUL2Ew SMMpPRPbKpp9bEUIsg0QGK88qQbKOEGXl8WjQJ7zOCgsQgcklJaS2/lNGHsNI4d23XXxI2T1tRjl hYmWd0Y4V6LS1YRZ9LLwVtUMAUyjZASK0LjlSkQt23a6FL4QGSiFK1bJFjmFiE2ilJrWmJKpGFj0 cU2fO9GH+bryQ6tuHzDzj9/IHrkbk9jy+srRDQ8Qm01VEt3VBtwz5LU+oN0jjN0hU03SvRjQ/Nbx zeFCkWhsGTBq7fljS1NAYkv+7Tb1dxqvO9828/F5pIIXWxaRiRrGLPKQjhOeSHd40O4aQeGuYGuN p8IQX8OYxEjJR4ynLJcB0ls+FFK4qE3QSMlaWmLTVlI01SvNiO7ulip1GYoGVk0uym4X59Mi4aUf jh4v32mzZv8HOUS6o/O6M3DLVatpP6jtcepqtaZ+oulKuOimvpFkvuH0C6TdC1MsiXHKskvbg8qg yKuqL9PpoZdrnMcleBinnMqqSa51hetoZs1rkZrX97VLPg+Em53b7PuwC+JP1bFUZ9X8sAXTVuzz 3zTGbS/ZoMZv6t8jI9y6GuNx2snYwfVW1T1xR4SvjJOB+senO+ipP0IqDq507XoHd8Q1eFPjRaWa 296NkM7pGpsPGNtmmPSr/8XckU4UJCqhME+A --===============0552560009==--