List:Commits« Previous MessageNext Message »
From:Pekka Nousiainen Date:July 23 2011 1:09pm
Subject:bzr push into mysql-5.1-telco-7.0-wl4124-new1 branch (pekka.nousiainen:4412
to 4414) WL#4124
View as plain text  
 4414 Pekka Nousiainen	2011-07-23
      wl#4124 e02_error.diff
      mutex cleanups

    modified:
      sql/ha_ndb_index_stat.cc
      sql/ha_ndb_index_stat.h
      sql/ha_ndbcluster.cc
 4413 Pekka Nousiainen	2011-07-23
      wl#4124 e01_error.diff
      entry list cleanups

    modified:
      sql/ha_ndb_index_stat.cc
 4412 Pekka Nousiainen	2011-07-19
      wl#4124 x13_fix.diff
      bugfix: wrong byte size from date etc

    modified:
      mysql-test/suite/ndb/r/ndb_index_stat.result
      mysql-test/suite/ndb/t/ndb_index_stat.test
      storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp
=== modified file 'sql/ha_ndb_index_stat.cc'
--- a/sql/ha_ndb_index_stat.cc	2011-07-19 09:53:30 +0000
+++ b/sql/ha_ndb_index_stat.cc	2011-07-23 13:01:57 +0000
@@ -500,38 +500,29 @@ struct Ndb_index_stat_glob {
   uint wait_update;
   uint cache_query_bytes; /* In use */
   uint cache_clean_bytes; /* Obsolete versions not yet removed */
-  bool is_locked;
   Ndb_index_stat_glob() :
     total_count(0),
     force_update(0),
     wait_update(0),
     cache_query_bytes(0),
-    cache_clean_bytes(0),
-    is_locked(false)
+    cache_clean_bytes(0)
   {
   }
   void set_list_count()
   {
+    total_count= 0;
     int lt;
     for (lt= 0; lt < Ndb_index_stat::LT_Count; lt++)
     {
       const Ndb_index_stat_list &list= ndb_index_stat_list[lt];
       list_count[lt]= list.count;
+      total_count++;
     }
   }
-  void lock()
+  void set_status_variables()
   {
-    pthread_mutex_lock(&ndb_index_stat_glob_mutex);
-    assert(!is_locked);
-    is_locked= true;
-  }
-  void unlock()
-  {
-    assert(is_locked);
     g_ndb_status_index_stat_cache_query= cache_query_bytes;
     g_ndb_status_index_stat_cache_clean= cache_clean_bytes;
-    is_locked= false;
-    pthread_mutex_unlock(&ndb_index_stat_glob_mutex);
   }
 };
 
@@ -567,7 +558,6 @@ Ndb_index_stat::Ndb_index_stat()
 void
 ndb_index_stat_error(Ndb_index_stat *st, const char* place, int line)
 {
-  pthread_mutex_lock(&ndb_index_stat_stat_mutex);
   time_t now= ndb_index_stat_time();
   NdbIndexStat::Error error= st->is->getNdbError();
   if (error.code == 0)
@@ -581,13 +571,18 @@ ndb_index_stat_error(Ndb_index_stat *st,
   st->error= error;
   st->error_time= now;
   st->error_count++;
-  pthread_cond_broadcast(&ndb_index_stat_stat_cond);
-  pthread_mutex_unlock(&ndb_index_stat_stat_mutex);
 
   DBUG_PRINT("index_stat", ("%s line %d: error %d line %d extra %d",
                             place, line, error.code, error.line, error.extra));
 }
 
+void
+ndb_index_stat_clear_error(Ndb_index_stat *st)
+{
+  st->error.code= 0;
+  st->error.status= NdbError::Success;
+}
+
 /* Lists across shares */
 
 Ndb_index_stat_list::Ndb_index_stat_list(int the_lt, const char* the_name)
@@ -611,7 +606,7 @@ Ndb_index_stat_list ndb_index_stat_list[
 };
 
 void
-ndb_index_stat_list_add(Ndb_index_stat* st, int lt, int place= +1)
+ndb_index_stat_list_add(Ndb_index_stat* st, int lt)
 {
   Ndb_index_stat_glob &glob= ndb_index_stat_glob;
   assert(st != 0 && st->lt == 0);
@@ -627,13 +622,6 @@ ndb_index_stat_list_add(Ndb_index_stat*
     list.head= st;
     list.tail= st;
   }
-  else if (place < 0)
-  {
-    assert(list.head != 0 && list.head->list_prev == 0);
-    st->list_next= list.head;
-    list.head->list_prev= st;
-    list.head= st;
-  }
   else
   {
     assert(list.tail != 0 && list.tail->list_next == 0);
@@ -642,9 +630,6 @@ ndb_index_stat_list_add(Ndb_index_stat*
     list.tail= st;
   }
   list.count++;
-  glob.lock();
-  glob.total_count++;
-  glob.unlock();
 
   st->lt= lt;
 }
@@ -669,10 +654,6 @@ ndb_index_stat_list_remove(Ndb_index_sta
     list.tail= prev;
   assert(list.count != 0);
   list.count--;
-  glob.lock();
-  assert(glob.total_count != 0);
-  glob.total_count--;
-  glob.unlock();
 
   if (next != 0)
     next->list_prev= prev;
@@ -686,50 +667,32 @@ ndb_index_stat_list_remove(Ndb_index_sta
 }
 
 void
-ndb_index_stat_list_move(Ndb_index_stat *st, int lt, int place= +1)
+ndb_index_stat_list_move(Ndb_index_stat *st, int lt)
 {
   assert(st != 0);
   ndb_index_stat_list_remove(st);
-  ndb_index_stat_list_add(st, lt, place);
+  ndb_index_stat_list_add(st, lt);
 }
 
-/* Move entry in / out error list */
+/* Stats entry changes (must hold stat_mutex) */
 
 void
-ndb_index_stat_list_to_error(Ndb_index_stat *st)
+ndb_index_stat_force_update(Ndb_index_stat *st, bool onoff)
 {
   Ndb_index_stat_glob &glob= ndb_index_stat_glob;
-
-  assert(st != 0);
-  const int lt= st->lt; NDB_IGNORE_VALUE(lt);
-  assert(1 <= lt && lt < Ndb_index_stat::LT_Count);
-  assert(lt != Ndb_index_stat::LT_Error);
-
-  if (st->force_update != 0)
+  if (onoff)
+  {
+    /* One more request */
+    glob.force_update++;
+    st->force_update++;
+  }
+  else
   {
-    glob.lock();
+    /* All done */
     assert(glob.force_update >= st->force_update);
     glob.force_update-= st->force_update;
-    glob.unlock();
     st->force_update= 0;
   }
-
-  time_t now= ndb_index_stat_time();
-  st->error_time= now;
-  ndb_index_stat_list_move(st, Ndb_index_stat::LT_Error);
-}
-
-void
-ndb_index_stat_list_from_error(Ndb_index_stat *st)
-{
-  assert(st != 0);
-  assert(st->lt == Ndb_index_stat::LT_Error);
-  if (st->force_update)
-    ndb_index_stat_list_move(st, Ndb_index_stat::LT_Update);
-  else
-    ndb_index_stat_list_move(st, Ndb_index_stat::LT_Read);
-  st->error.code= 0;
-  st->error.status= NdbError::Success;
 }
 
 /* Find or add entry under the share */
@@ -810,6 +773,7 @@ ndb_index_stat_get_share(NDB_SHARE *shar
 {
   pthread_mutex_lock(&share->mutex);
   pthread_mutex_lock(&ndb_index_stat_list_mutex);
+  pthread_mutex_lock(&ndb_index_stat_stat_mutex);
   time_t now= ndb_index_stat_time();
 
   struct Ndb_index_stat *st= 0;
@@ -825,18 +789,15 @@ ndb_index_stat_get_share(NDB_SHARE *shar
     }
     if (st != 0)
     {
-      if (force_update != 0)
+      if (force_update)
       {
-        st->force_update++;
-        Ndb_index_stat_glob &glob= ndb_index_stat_glob;
-        glob.lock();
-        glob.force_update++;
-        glob.unlock();
+        ndb_index_stat_force_update(st, true);
       }
       st->access_time= now;
     }
   }
 
+  pthread_mutex_unlock(&ndb_index_stat_stat_mutex);
   pthread_mutex_unlock(&ndb_index_stat_list_mutex);
   pthread_mutex_unlock(&share->mutex);
   return st;
@@ -909,12 +870,11 @@ ndb_index_stat_cache_move(Ndb_index_stat
   DBUG_PRINT("index_stat", ("st %s cache move: query:%u clean:%u",
                             st->id, new_query_bytes, old_query_bytes));
   st->is->move_cache();
-  glob.lock();
   assert(glob.cache_query_bytes >= old_query_bytes);
   glob.cache_query_bytes-= old_query_bytes;
   glob.cache_query_bytes+= new_query_bytes;
   glob.cache_clean_bytes+= old_query_bytes;
-  glob.unlock();
+  glob.set_status_variables();
 }
 
 void
@@ -928,10 +888,9 @@ ndb_index_stat_cache_clean(Ndb_index_sta
   DBUG_PRINT("index_stat", ("st %s cache clean: clean:%u",
                             st->id, old_clean_bytes));
   st->is->clean_cache();
-  glob.lock();
   assert(glob.cache_clean_bytes >= old_clean_bytes);
   glob.cache_clean_bytes-= old_clean_bytes;
-  glob.unlock();
+  glob.set_status_variables();
 }
 
 /* Misc in/out parameters for process steps */
@@ -1021,8 +980,12 @@ ndb_index_stat_proc_read(Ndb_index_stat_
   NdbIndexStat::Head head;
   if (st->is->read_stat(pr.ndb) == -1)
   {
+    pthread_mutex_lock(&ndb_index_stat_stat_mutex);
     ndb_index_stat_error(st, "read_stat", __LINE__);
     pr.lt= Ndb_index_stat::LT_Error;
+    ndb_index_stat_force_update(st, false);
+    pthread_cond_broadcast(&ndb_index_stat_stat_cond);
+    pthread_mutex_unlock(&ndb_index_stat_stat_mutex);
     return;
   }
 
@@ -1033,15 +996,7 @@ ndb_index_stat_proc_read(Ndb_index_stat_
   st->read_time= pr.now;
   st->sample_version= head.m_sampleVersion;
 
-  if (st->force_update != 0)
-  {
-    Ndb_index_stat_glob &glob= ndb_index_stat_glob;
-    glob.lock();
-    assert(glob.force_update >= st->force_update);
-    glob.force_update-= st->force_update;
-    glob.unlock();
-    st->force_update= 0;
-  }
+  ndb_index_stat_force_update(st, false);
 
   ndb_index_stat_cache_move(st);
   st->cache_clean= false;
@@ -1073,6 +1028,7 @@ ndb_index_stat_proc_read(Ndb_index_stat_
     pr.busy= true;
 }
 
+// wl4124_todo detect force_update faster
 void
 ndb_index_stat_proc_idle(Ndb_index_stat_proc &pr, Ndb_index_stat *st)
 {
@@ -1124,11 +1080,9 @@ ndb_index_stat_proc_idle(Ndb_index_stat_
     st_loop= st_loop->list_next;
     DBUG_PRINT("index_stat", ("st %s proc %s", st->id, list.name));
     ndb_index_stat_proc_idle(pr, st);
-    if (pr.lt != lt)
-    {
-      ndb_index_stat_list_move(st, pr.lt);
-      cnt++;
-    }
+    // rotates list if entry remains LT_Idle
+    ndb_index_stat_list_move(st, pr.lt);
+    cnt++;
   }
   if (cnt == batch)
     pr.busy= true;
@@ -1213,9 +1167,7 @@ ndb_index_stat_proc_evict()
 {
   const Ndb_index_stat_opt &opt= ndb_index_stat_opt;
   Ndb_index_stat_glob &glob= ndb_index_stat_glob;
-  glob.lock();
   uint curr_size= glob.cache_query_bytes + glob.cache_clean_bytes;
-  glob.unlock();
   const uint cache_lowpct= opt.get(Ndb_index_stat_opt::Icache_lowpct);
   const uint cache_limit= opt.get(Ndb_index_stat_opt::Icache_limit);
   if (100 * curr_size <= cache_lowpct * cache_limit)
@@ -1330,11 +1282,15 @@ ndb_index_stat_proc_error(Ndb_index_stat
   const int error_delay= opt.get(Ndb_index_stat_opt::Ierror_delay);
   const time_t error_wait= st->error_time + error_delay - pr.now;
 
-  if (error_wait <= 0)
-  {
-    ndb_index_stat_list_from_error(st);
-    DBUG_PRINT("index_stat", ("st %s error wait:%ds error count:%u",
-                              st->id, (int)error_wait, st->error_count));
+  if (error_wait <= 0 ||
+      /* Analyze issued after previous error */
+      st->force_update)
+  {
+    DBUG_PRINT("index_stat", ("st %s error wait:%ds error count:%u"
+                              " force update:%u",
+                              st->id, (int)error_wait, st->error_count,
+                              st->force_update));
+    ndb_index_stat_clear_error(st);
     if (st->force_update)
       pr.lt= Ndb_index_stat::LT_Update;
     else
@@ -1361,11 +1317,8 @@ ndb_index_stat_proc_error(Ndb_index_stat
     st_loop= st_loop->list_next;
     DBUG_PRINT("index_stat", ("st %s proc %s", st->id, list.name));
     ndb_index_stat_proc_error(pr, st);
-    if (pr.lt != lt)
-    {
-      ndb_index_stat_list_move(st, pr.lt);
-      cnt++;
-    }
+    ndb_index_stat_list_move(st, pr.lt);
+    cnt++;
   }
   if (cnt == batch)
     pr.busy= true;

=== modified file 'sql/ha_ndb_index_stat.h'
--- a/sql/ha_ndb_index_stat.h	2011-06-15 10:37:56 +0000
+++ b/sql/ha_ndb_index_stat.h	2011-07-23 13:01:57 +0000
@@ -22,13 +22,16 @@ extern struct st_ndb_status g_ndb_status
 extern pthread_t ndb_index_stat_thread;
 extern pthread_cond_t COND_ndb_index_stat_thread;
 extern pthread_mutex_t LOCK_ndb_index_stat_thread;
-extern pthread_mutex_t ndb_index_stat_glob_mutex;
+
+/* protect entry lists where needed */
 extern pthread_mutex_t ndb_index_stat_list_mutex;
+
+/* protect and signal changes in stats entries */
 extern pthread_mutex_t ndb_index_stat_stat_mutex;
 extern pthread_cond_t ndb_index_stat_stat_cond;
 
+/* these have to live in ha_ndbcluster.cc */
 extern bool ndb_index_stat_get_enable(THD *thd);
-
 extern long g_ndb_status_index_stat_cache_query;
 extern long g_ndb_status_index_stat_cache_clean;
 

=== modified file 'sql/ha_ndbcluster.cc'
--- a/sql/ha_ndbcluster.cc	2011-07-08 12:28:37 +0000
+++ b/sql/ha_ndbcluster.cc	2011-07-23 13:01:57 +0000
@@ -427,7 +427,6 @@ int ndb_index_stat_thread_running= 0;
 pthread_mutex_t LOCK_ndb_index_stat_thread;
 pthread_cond_t COND_ndb_index_stat_thread;
 pthread_cond_t COND_ndb_index_stat_ready;
-pthread_mutex_t ndb_index_stat_glob_mutex;
 pthread_mutex_t ndb_index_stat_list_mutex;
 pthread_mutex_t ndb_index_stat_stat_mutex;
 pthread_cond_t ndb_index_stat_stat_cond;
@@ -11427,7 +11426,6 @@ static int ndbcluster_init(void *p)
   pthread_mutex_init(&LOCK_ndb_index_stat_thread, MY_MUTEX_INIT_FAST);
   pthread_cond_init(&COND_ndb_index_stat_thread, NULL);
   pthread_cond_init(&COND_ndb_index_stat_ready, NULL);
-  pthread_mutex_init(&ndb_index_stat_glob_mutex, MY_MUTEX_INIT_FAST);
   pthread_mutex_init(&ndb_index_stat_list_mutex, MY_MUTEX_INIT_FAST);
   pthread_mutex_init(&ndb_index_stat_stat_mutex, MY_MUTEX_INIT_FAST);
   pthread_cond_init(&ndb_index_stat_stat_cond, NULL);
@@ -11538,7 +11536,6 @@ static int ndbcluster_init(void *p)
     pthread_mutex_destroy(&LOCK_ndb_index_stat_thread);
     pthread_cond_destroy(&COND_ndb_index_stat_thread);
     pthread_cond_destroy(&COND_ndb_index_stat_ready);
-    pthread_mutex_destroy(&ndb_index_stat_glob_mutex);
     pthread_mutex_destroy(&ndb_index_stat_list_mutex);
     pthread_mutex_destroy(&ndb_index_stat_stat_mutex);
     pthread_cond_destroy(&ndb_index_stat_stat_cond);
@@ -11559,7 +11556,6 @@ static int ndbcluster_init(void *p)
     pthread_mutex_destroy(&LOCK_ndb_index_stat_thread);
     pthread_cond_destroy(&COND_ndb_index_stat_thread);
     pthread_cond_destroy(&COND_ndb_index_stat_ready);
-    pthread_mutex_destroy(&ndb_index_stat_glob_mutex);
     pthread_mutex_destroy(&ndb_index_stat_list_mutex);
     pthread_mutex_destroy(&ndb_index_stat_stat_mutex);
     pthread_cond_destroy(&ndb_index_stat_stat_cond);

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.1-telco-7.0-wl4124-new1 branch (pekka.nousiainen:4412to 4414) WL#4124Pekka Nousiainen25 Jul