From: Pekka Nousiainen Date: December 18 2011 12:22pm Subject: bzr push into mysql-5.1-telco-7.0 branch (pekka.nousiainen:4751 to 4755) WL#4124 List-Archive: http://lists.mysql.com/commits/142167 Message-Id: <20111218122256.1015657837@cuda.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4755 Pekka Nousiainen 2011-12-18 wl#4124 k03_mutex.diff handler: safe_mutex asserts modified: sql/ha_ndb_index_stat.cc 4754 Pekka Nousiainen 2011-12-18 wl#4124 k02_error.diff handler: stat_error server vs client modified: sql/ha_ndb_index_stat.cc storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp 4753 Pekka Nousiainen 2011-12-18 wl#4124 k01_mutex.diff handler: remove list_mutex, use stat_mutex modified: sql/ha_ndb_index_stat.cc sql/ha_ndb_index_stat.h 4752 Pekka Nousiainen 2011-12-18 wl#4124 x43_fix.diff handler: move internal error codes to ndb api modified: sql/ha_ndb_index_stat.cc sql/ha_ndbcluster.cc storage/ndb/include/ndbapi/NdbIndexStat.hpp storage/ndb/src/ndbapi/ndberror.c 4751 Pekka Nousiainen 2011-12-15 wl#4124 x42_fix.diff handler: unused variables modified: sql/ha_ndb_index_stat.cc storage/ndb/tools/ndb_index_stat.cpp === modified file 'sql/ha_ndb_index_stat.cc' --- a/sql/ha_ndb_index_stat.cc 2011-12-15 15:13:59 +0000 +++ b/sql/ha_ndb_index_stat.cc 2011-12-18 12:21:39 +0000 @@ -57,7 +57,6 @@ Ndb_index_stat_thread::Ndb_index_stat_th pthread_mutex_init(&LOCK, MY_MUTEX_INIT_FAST); pthread_cond_init(&COND, NULL); pthread_cond_init(&COND_ready, NULL); - pthread_mutex_init(&list_mutex, MY_MUTEX_INIT_FAST); pthread_mutex_init(&stat_mutex, MY_MUTEX_INIT_FAST); pthread_cond_init(&stat_cond, NULL); } @@ -67,7 +66,6 @@ Ndb_index_stat_thread::~Ndb_index_stat_t pthread_mutex_destroy(&LOCK); pthread_cond_destroy(&COND); pthread_cond_destroy(&COND_ready); - pthread_mutex_destroy(&list_mutex); pthread_mutex_destroy(&stat_mutex); pthread_cond_destroy(&stat_cond); } @@ -102,6 +100,7 @@ struct Ndb_index_stat { bool force_update; /* one-time force update from analyze table */ bool no_stats; /* have detected that no stats exist */ NdbIndexStat::Error error; + NdbIndexStat::Error client_error; time_t error_time; uint error_count; struct Ndb_index_stat *share_next; /* per-share list */ @@ -620,6 +619,8 @@ Ndb_index_stat_glob::Ndb_index_stat_glob void Ndb_index_stat_glob::set_status() { + safe_mutex_assert_owner(&ndb_index_stat_thread.stat_mutex); + const Ndb_index_stat_opt &opt= ndb_index_stat_opt; char* p= status[status_i]; @@ -748,21 +749,35 @@ Ndb_index_stat::Ndb_index_stat() abort_request= false; } +/* + Called by stats thread and (rarely) by client. Caller must hold + stat_mutex. Client errors currently have no effect on execution + since they are probably local e.g. bad range (internal error). + Argument "from" is 0=stats thread 1=client. +*/ void -ndb_index_stat_error(Ndb_index_stat *st, const char* place, int line) +ndb_index_stat_error(Ndb_index_stat *st, + int from, const char* place, int line) { + safe_mutex_assert_owner(&ndb_index_stat_thread.stat_mutex); + time_t now= ndb_index_stat_time(); NdbIndexStat::Error error= st->is->getNdbError(); if (error.code == 0) { - // XXX why this if + /* Make sure code is not 0 */ NdbIndexStat::Error error2; error= error2; error.code= NdbIndexStat::InternalError; error.status= NdbError::TemporaryError; } - st->error= error; - st->error_time= now; + if (from == 0) + { + st->error= error; + st->error_time= now; /* Controls proc_error */ + } + else + st->client_error= error; st->error_count++; DBUG_PRINT("index_stat", ("%s line %d: error %d line %d extra %d", @@ -870,6 +885,8 @@ ndb_index_stat_list_move(Ndb_index_stat void ndb_index_stat_force_update(Ndb_index_stat *st, bool onoff) { + safe_mutex_assert_owner(&ndb_index_stat_thread.stat_mutex); + Ndb_index_stat_glob &glob= ndb_index_stat_glob; if (onoff) { @@ -895,6 +912,8 @@ ndb_index_stat_force_update(Ndb_index_st void ndb_index_stat_no_stats(Ndb_index_stat *st, bool flag) { + safe_mutex_assert_owner(&ndb_index_stat_thread.stat_mutex); + Ndb_index_stat_glob &glob= ndb_index_stat_glob; if (st->no_stats != flag) { @@ -916,6 +935,8 @@ ndb_index_stat_no_stats(Ndb_index_stat * void ndb_index_stat_ref_count(Ndb_index_stat *st, bool flag) { + safe_mutex_assert_owner(&ndb_index_stat_thread.stat_mutex); + uint old_count= st->ref_count; (void)old_count; // USED if (flag) @@ -940,6 +961,7 @@ struct Ndb_index_stat_snap { Ndb_index_stat_snap() { load_time= 0; sample_version= 0; } }; +/* Subroutine, have lock */ Ndb_index_stat* ndb_index_stat_alloc(const NDBINDEX *index, const NDBTAB *table, @@ -958,8 +980,8 @@ ndb_index_stat_alloc(const NDBINDEX *ind #endif if (is->set_index(*index, *table) == 0) return st; - ndb_index_stat_error(st, "set_index", __LINE__); - err_out= st->error.code; + ndb_index_stat_error(st, 1, "set_index", __LINE__); + err_out= st->client_error.code; } else { @@ -1020,7 +1042,6 @@ ndb_index_stat_get_share(NDB_SHARE *shar Ndb_index_stat_glob &glob= ndb_index_stat_glob; pthread_mutex_lock(&share->mutex); - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); time_t now= ndb_index_stat_time(); err_out= 0; @@ -1031,7 +1052,7 @@ ndb_index_stat_get_share(NDB_SHARE *shar { if (unlikely(!ndb_index_stat_allow())) { - err_out= Ndb_index_stat_error_NOT_ALLOW; + err_out= NdbIndexStat::MyNotAllow; break; } st= ndb_index_stat_find_share(share, index, st_last); @@ -1039,7 +1060,7 @@ ndb_index_stat_get_share(NDB_SHARE *shar { if (!allow_add) { - err_out= Ndb_index_stat_error_NOT_FOUND; + err_out= NdbIndexStat::MyNotFound; break; } st= ndb_index_stat_alloc(index, table, err_out); @@ -1054,7 +1075,7 @@ ndb_index_stat_get_share(NDB_SHARE *shar } else if (unlikely(st->abort_request)) { - err_out= Ndb_index_stat_error_ABORT_REQUEST; + err_out= NdbIndexStat::MyAbortReq; break; } if (force_update) @@ -1074,7 +1095,6 @@ ndb_index_stat_get_share(NDB_SHARE *shar st= 0; pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); pthread_mutex_unlock(&share->mutex); return st; } @@ -1084,10 +1104,12 @@ ndb_index_stat_get_share(NDB_SHARE *shar list and set "to_delete" flag. Stats thread does real delete. */ -/* caller must hold list_mutex and stat_mutex */ +/* caller must hold stat_mutex */ void ndb_index_stat_free(Ndb_index_stat *st) { + safe_mutex_assert_owner(&ndb_index_stat_thread.stat_mutex); + DBUG_ENTER("ndb_index_stat_free"); Ndb_index_stat_glob &glob= ndb_index_stat_glob; NDB_SHARE *share= st->share; @@ -1138,7 +1160,6 @@ ndb_index_stat_free(NDB_SHARE *share, in DBUG_PRINT("index_stat", ("(index_id:%d index_version:%d", index_id, index_version)); Ndb_index_stat_glob &glob= ndb_index_stat_glob; - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); uint found= 0; @@ -1161,7 +1182,6 @@ ndb_index_stat_free(NDB_SHARE *share, in glob.set_status(); pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); DBUG_VOID_RETURN; } @@ -1170,7 +1190,6 @@ ndb_index_stat_free(NDB_SHARE *share) { DBUG_ENTER("ndb_index_stat_free"); Ndb_index_stat_glob &glob= ndb_index_stat_glob; - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); uint found= 0; @@ -1195,7 +1214,6 @@ ndb_index_stat_free(NDB_SHARE *share) glob.set_status(); pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); DBUG_VOID_RETURN; } @@ -1206,7 +1224,7 @@ ndb_index_stat_find_entry(int index_id, { DBUG_ENTER("ndb_index_stat_find_entry"); pthread_mutex_lock(&ndbcluster_mutex); - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); + pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); DBUG_PRINT("index_stat", ("find index:%d version:%d table:%d", index_id, index_version, table_id)); @@ -1219,7 +1237,7 @@ ndb_index_stat_find_entry(int index_id, if (st->index_id == index_id && st->index_version == index_version) { - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); + pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); pthread_mutex_unlock(&ndbcluster_mutex); DBUG_RETURN(st); } @@ -1227,7 +1245,7 @@ ndb_index_stat_find_entry(int index_id, } } - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); + pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); pthread_mutex_unlock(&ndbcluster_mutex); DBUG_RETURN(0); } @@ -1345,7 +1363,7 @@ void ndb_index_stat_proc_new(Ndb_index_stat_proc &pr) { Ndb_index_stat_glob &glob= ndb_index_stat_glob; - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); + pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); const int lt= Ndb_index_stat::LT_New; Ndb_index_stat_list &list= ndb_index_stat_list[lt]; @@ -1359,10 +1377,8 @@ ndb_index_stat_proc_new(Ndb_index_stat_p assert(pr.lt != lt); ndb_index_stat_list_move(st, pr.lt); } - pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); glob.set_status(); pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); } void @@ -1370,8 +1386,8 @@ ndb_index_stat_proc_update(Ndb_index_sta { if (st->is->update_stat(pr.ndb) == -1) { - ndb_index_stat_error(st, "update_stat", __LINE__); pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); + ndb_index_stat_error(st, 0, "update_stat", __LINE__); /* Turn off force update or else proc_error() thinks @@ -1425,7 +1441,7 @@ ndb_index_stat_proc_read(Ndb_index_stat_ if (st->is->read_stat(pr.ndb) == -1) { pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); - ndb_index_stat_error(st, "read_stat", __LINE__); + ndb_index_stat_error(st, 0, "read_stat", __LINE__); const bool force_update= st->force_update; ndb_index_stat_force_update(st, false); @@ -1600,7 +1616,8 @@ ndb_index_stat_proc_check(Ndb_index_stat NdbIndexStat::Head head; if (st->is->read_head(pr.ndb) == -1) { - ndb_index_stat_error(st, "read_head", __LINE__); + pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); + ndb_index_stat_error(st, 0, "read_head", __LINE__); /* no stats is not unexpected error */ if (st->is->getNdbError().code == NdbIndexStat::NoIndexStats) { @@ -1611,6 +1628,8 @@ ndb_index_stat_proc_check(Ndb_index_stat { pr.lt= Ndb_index_stat::LT_Error; } + pthread_cond_broadcast(&ndb_index_stat_thread.stat_cond); + pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); return; } st->is->get_head(head); @@ -1703,7 +1722,6 @@ ndb_index_stat_proc_evict(Ndb_index_stat return; /* Mutex entire routine (protect access_time) */ - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); /* Create a LRU batch */ @@ -1797,7 +1815,6 @@ ndb_index_stat_proc_evict(Ndb_index_stat glob.evict_count+= cnt; pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); } void @@ -2115,7 +2132,7 @@ void ndb_index_stat_list_verify(Ndb_index_stat_proc &pr) { const Ndb_index_stat_glob &glob= ndb_index_stat_glob; - pthread_mutex_lock(&ndb_index_stat_thread.list_mutex); + pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); pr.cache_query_bytes= 0; pr.cache_clean_bytes= 0; @@ -2124,7 +2141,7 @@ ndb_index_stat_list_verify(Ndb_index_sta assert(glob.cache_query_bytes == pr.cache_query_bytes); assert(glob.cache_clean_bytes == pr.cache_clean_bytes); - pthread_mutex_unlock(&ndb_index_stat_thread.list_mutex); + pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); } void @@ -2634,7 +2651,7 @@ ndb_index_stat_wait_query(Ndb_index_stat glob.query_count++; if (st->lt == Ndb_index_stat::LT_Error) { - err= Ndb_index_stat_error_HAS_ERROR; + err= NdbIndexStat::MyHasError; break; } ndb_index_stat_clear_error(st); @@ -2668,7 +2685,7 @@ ndb_index_stat_wait_query(Ndb_index_stat } if (st->abort_request) { - err= Ndb_index_stat_error_ABORT_REQUEST; + err= NdbIndexStat::MyAbortReq; break; } count++; @@ -2744,7 +2761,7 @@ ndb_index_stat_wait_analyze(Ndb_index_st } if (st->abort_request) { - err= Ndb_index_stat_error_ABORT_REQUEST; + err= NdbIndexStat::MyAbortReq; break; } count++; @@ -2821,15 +2838,19 @@ ha_ndbcluster::ndb_index_stat_query(uint const NdbRecord* key_record= data.ndb_record_key; if (st->is->convert_range(range, key_record, &ib) == -1) { - ndb_index_stat_error(st, "convert_range", __LINE__); - err= st->error.code; + pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); + ndb_index_stat_error(st, 1, "convert_range", __LINE__); + err= st->client_error.code; + pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); break; } if (st->is->query_stat(range, stat) == -1) { /* Invalid cache - should remove the entry */ - ndb_index_stat_error(st, "query_stat", __LINE__); - err= st->error.code; + pthread_mutex_lock(&ndb_index_stat_thread.stat_mutex); + ndb_index_stat_error(st, 1, "query_stat", __LINE__); + err= st->client_error.code; + pthread_mutex_unlock(&ndb_index_stat_thread.stat_mutex); break; } } === modified file 'sql/ha_ndb_index_stat.h' --- a/sql/ha_ndb_index_stat.h 2011-12-15 12:17:06 +0000 +++ b/sql/ha_ndb_index_stat.h 2011-12-18 12:20:44 +0000 @@ -40,10 +40,10 @@ public: pthread_cond_t COND; pthread_cond_t COND_ready; - /* protect entry lists where needed */ - pthread_mutex_t list_mutex; - - /* protect and signal changes in stats entries */ + /* + protect stats entry lists where needed + protect and signal changes in stats entries + */ pthread_mutex_t stat_mutex; pthread_cond_t stat_cond; === modified file 'sql/ha_ndbcluster.cc' --- a/sql/ha_ndbcluster.cc 2011-12-15 12:17:06 +0000 +++ b/sql/ha_ndbcluster.cc 2011-12-18 12:19:55 +0000 @@ -1240,9 +1240,9 @@ void ha_ndbcluster::set_rec_per_key() /* no stats is not unexpected error */ err != NdbIndexStat::NoIndexStats && /* warning was printed at first error */ - err != Ndb_index_stat_error_HAS_ERROR && + err != NdbIndexStat::MyHasError && /* stats thread aborted request */ - err != Ndb_index_stat_error_ABORT_REQUEST) + err != NdbIndexStat::MyAbortReq) { push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_CANT_GET_STAT, /* pun? */ @@ -12700,9 +12700,9 @@ ha_ndbcluster::records_in_range(uint inx /* no stats is not unexpected error */ err != NdbIndexStat::NoIndexStats && /* warning was printed at first error */ - err != Ndb_index_stat_error_HAS_ERROR && + err != NdbIndexStat::MyHasError && /* stats thread aborted request */ - err != Ndb_index_stat_error_ABORT_REQUEST) + err != NdbIndexStat::MyAbortReq) { push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_CANT_GET_STAT, /* pun? */ === modified file 'storage/ndb/include/ndbapi/NdbIndexStat.hpp' --- a/storage/ndb/include/ndbapi/NdbIndexStat.hpp 2011-12-15 12:19:18 +0000 +++ b/storage/ndb/include/ndbapi/NdbIndexStat.hpp 2011-12-18 12:19:55 +0000 @@ -104,7 +104,15 @@ public: NoSysEvents = 4710, BadSysEvents = BadSysTables, HaveSysEvents = 746, - AlienUpdate = 4721 // mysqld: somebody else messed with stats + /* + * Following are for mysqld. Most are consumed by mysqld itself + * and should therefore not be seen by clients. + */ + MyNotAllow = 4721, // stats thread not open for requests + MyNotFound = 4722, // stats entry unexpectedly not found + MyHasError = 4723, // request ignored due to recent error + MyAbortReq = 4724, // request aborted by stats thread + AlienUpdate = 4725 // somebody else messed with stats }; /* === modified file 'storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp' --- a/storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp 2011-12-15 12:19:18 +0000 +++ b/storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp 2011-12-18 12:21:13 +0000 @@ -2005,6 +2005,21 @@ NdbIndexStatImpl::convert_range(Range& r return -1; } } + +#ifdef VM_TRACE + { + const char* p = NdbEnv_GetEnv("NDB_INDEX_STAT_RANGE_ERROR", (char*)0, 0); + if (p != 0 && strchr("1Y", p[0]) != 0) + { + if (rand() % 10 == 0) + { + setError(InternalError, __LINE__, NdbIndexStat::InternalError); + return -1; + } + } + } +#endif + return 0; } === modified file 'storage/ndb/src/ndbapi/ndberror.c' --- a/storage/ndb/src/ndbapi/ndberror.c 2011-12-15 12:18:17 +0000 +++ b/storage/ndb/src/ndbapi/ndberror.c 2011-12-18 12:19:55 +0000 @@ -552,7 +552,11 @@ ErrorBundle ErrorCodes[] = { { 4718, DMEC, IE, "Index stats samples data or memory cache is invalid" }, { 4719, DMEC, IE, "Index stats internal error" }, { 4720, DMEC, AE, "Index stats sys tables " NDB_INDEX_STAT_PREFIX " partly missing or invalid" }, - { 4721, DMEC, AE, "Index stats were deleted/updated by another process " }, + { 4721, DMEC, IE, "Mysqld: index stats thread not open for requests" }, + { 4722, DMEC, IE, "Mysqld: index stats entry unexpectedly not found" }, + { 4723, DMEC, AE, "Mysqld: index stats request ignored due to recent error" }, + { 4724, DMEC, AE, "Mysqld: index stats request aborted by stats thread" }, + { 4725, DMEC, AE, "Index stats were deleted by another process" }, /** * Still uncategorized No bundle (reason: useless for push emails).