List:Commits« Previous MessageNext Message »
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
View as plain text  
 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).
Thread
bzr push into mysql-5.1-telco-7.0 branch (pekka.nousiainen:4751 to 4755)WL#4124Pekka Nousiainen19 Dec