List:Commits« Previous MessageNext Message »
From:Magne Mahre Date:October 21 2010 10:52am
Subject:bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3328) Bug#49177
View as plain text  
#At file:///export/home/tmp/x/mysql-next-mr-bugfixing-49177/ based on revid:anitha.gopi@stripped

 3328 Magne Mahre	2010-10-21
      Bug#49177 MyISAM share list scalability problems
            
      (Note: MyISAM issue only)
                  
      As the number of open tables is increased, table lookup 
      (testing if a table is already open)  and  (in particular) 
      the case when a table is not open, became increasingly more 
      expensive.
                  
      The problem was caused by the open table lookup mechanism, 
      which was based on traversing a linked list comparing the 
      file names. 
            
      As the list was replaced by a hash table, the lookup 
      time dropped significantly when used on systems with
      a large number of open tables.
      
      The performance on smaller installations remains in the same
      ballpark.  Instead of growing exponentially, the lookup time
      now grows more or less linearly, at least in the ballpark
      with less than 100 000 open tables.
     @ storage/myisam/myisamchk.c
        Need to initialize MyISAM open table hash
     @ storage/myisam/myisampack.c
        Need to initialize MyISAM open table hash

    modified:
      storage/myisam/ha_myisam.cc
      storage/myisam/mi_close.c
      storage/myisam/mi_dbug.c
      storage/myisam/mi_keycache.c
      storage/myisam/mi_open.c
      storage/myisam/mi_panic.c
      storage/myisam/mi_static.c
      storage/myisam/myisamchk.c
      storage/myisam/myisamdef.h
      storage/myisam/myisampack.c
=== modified file 'storage/myisam/ha_myisam.cc'
--- a/storage/myisam/ha_myisam.cc	2010-07-13 17:29:44 +0000
+++ b/storage/myisam/ha_myisam.cc	2010-10-21 10:51:58 +0000
@@ -2099,6 +2099,10 @@ static int myisam_init(void *p)
   myisam_hton->create= myisam_create_handler;
   myisam_hton->panic= myisam_panic;
   myisam_hton->flags= HTON_CAN_RECREATE | HTON_SUPPORT_LOG_TABLES;
+
+  if (mi_init_open_table_hash(table_cache_size))
+    return 1;
+
   return 0;
 }
 

=== modified file 'storage/myisam/mi_close.c'
--- a/storage/myisam/mi_close.c	2010-07-08 21:20:08 +0000
+++ b/storage/myisam/mi_close.c	2010-10-21 10:51:58 +0000
@@ -54,7 +54,9 @@ int mi_close(register MI_INFO *info)
     info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
   }
   flag= !--share->reopen;
-  myisam_open_list=list_delete(myisam_open_list,&info->open_list);
+
+  my_hash_delete(&myisam_open_table_hash, (uchar *) info);
+
   mysql_mutex_unlock(&share->intern_lock);
 
   my_free(mi_get_rec_buff_ptr(info, info->rec_buff));

=== modified file 'storage/myisam/mi_dbug.c'
--- a/storage/myisam/mi_dbug.c	2010-06-07 12:05:34 +0000
+++ b/storage/myisam/mi_dbug.c	2010-10-21 10:51:58 +0000
@@ -181,14 +181,14 @@ void _mi_print_key(FILE *stream, registe
 my_bool check_table_is_closed(const char *name, const char *where)
 {
   char filename[FN_REFLEN];
-  LIST *pos;
+  uint idx;
   DBUG_ENTER("check_table_is_closed");
 
   (void) fn_format(filename,name,"",MI_NAME_IEXT,4+16+32);
   mysql_mutex_lock(&THR_LOCK_myisam);
-  for (pos=myisam_open_list ; pos ; pos=pos->next)
+  for (idx= 0; idx < myisam_open_table_hash.records; ++idx)
   {
-    MI_INFO *info=(MI_INFO*) pos->data;
+    MI_INFO *info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, idx);
     MYISAM_SHARE *share=info->s;
     if (!strcmp(share->unique_file_name,filename))
     {

=== modified file 'storage/myisam/mi_keycache.c'
--- a/storage/myisam/mi_keycache.c	2009-12-05 01:26:15 +0000
+++ b/storage/myisam/mi_keycache.c	2010-10-21 10:51:58 +0000
@@ -137,16 +137,16 @@ int mi_assign_to_key_cache(MI_INFO *info
 void mi_change_key_cache(KEY_CACHE *old_key_cache,
 			 KEY_CACHE *new_key_cache)
 {
-  LIST *pos;
+  uint idx;
   DBUG_ENTER("mi_change_key_cache");
 
   /*
     Lock list to ensure that no one can close the table while we manipulate it
   */
   mysql_mutex_lock(&THR_LOCK_myisam);
-  for (pos=myisam_open_list ; pos ; pos=pos->next)
+  for (idx= 0; idx < myisam_open_table_hash.records; ++idx)
   {
-    MI_INFO *info= (MI_INFO*) pos->data;
+    MI_INFO *info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, idx);
     MYISAM_SHARE *share= info->s;
     if (share->key_cache == old_key_cache)
       mi_assign_to_key_cache(info, (ulonglong) ~0, new_key_cache);

=== modified file 'storage/myisam/mi_open.c'
--- a/storage/myisam/mi_open.c	2010-07-23 20:59:42 +0000
+++ b/storage/myisam/mi_open.c	2010-10-21 10:51:58 +0000
@@ -36,24 +36,76 @@ if (pos > end_pos)             \
   goto err;                    \
 }
 
+/*
+  Get the value used as hash key (helper function for the 
+  open table hash).  Function is used as a callback 
+  from the hash table
+*/
+static uchar *mi_open_table_hash_key(const uchar *record, size_t *length,
+                                     my_bool not_used __attribute__((unused)))
+{
+  MI_INFO *info= (MI_INFO *) record;
+  *length= info->s->unique_name_length;
+  return (uchar*) info->s->unique_file_name;
+}
 
-/******************************************************************************
-** Return the shared struct if the table is already open.
-** In MySQL the server will handle version issues.
-******************************************************************************/
+/**
+  Initialize open table hash
+ 
+  Function is normally called from myisam_init
+  with the system variable table_cache_size used
+  as hash_size.
+
+  @param[in]      hash_size Initial has size (elements)
+  @return         inidicates success or failure of initialization
+    @retval 0 success
+    @retval 1 failure
 
+*/
+int mi_init_open_table_hash(ulong hash_size)
+{
+  if (hash_size == 0)
+    hash_size= 32; /* default hash size */
+
+  if (my_hash_init(&myisam_open_table_hash, &my_charset_filename, 
+                   hash_size, 0, 0, mi_open_table_hash_key, 0, 0))
+    return 1; /* error */
+  
+  return 0;
+}
+
+
+/**
+  Retrieve the shared struct if the table is already
+  open (i.e  in the open table hash)
+
+  @param[in] filename table file name
+  @return    shared struct, 0 if not in the cache
+*/
 MI_INFO *test_if_reopen(char *filename)
 {
-  LIST *pos;
+  HASH_SEARCH_STATE current_record;
+  int len= strlen(filename);
 
-  for (pos=myisam_open_list ; pos ; pos=pos->next)
+  MI_INFO *info= (MI_INFO*) my_hash_first(&myisam_open_table_hash, 
+                                          (uchar *) filename, 
+                                          len, &current_record);
+  /*
+    There might be more than one instance of a table share for
+    a given table in the hash table.  We're interested in the one with 
+    last_version set, so we iterate until we find it
+  */
+  while (info)
   {
-    MI_INFO *info=(MI_INFO*) pos->data;
     MYISAM_SHARE *share=info->s;
-    if (!strcmp(share->unique_file_name,filename) && share->last_version)
-      return info;
+    if (share->last_version)
+      break;
+    
+    info= (MI_INFO*) my_hash_next(&myisam_open_table_hash, 
+                                  (uchar *) filename, 
+                                  len, &current_record);
   }
-  return 0;
+  return info;
 }
 
 
@@ -628,8 +680,9 @@ MI_INFO *mi_open(const char *name, int m
 #ifdef THREAD
   thr_lock_data_init(&share->lock,&m_info->lock,(void*) m_info);
 #endif
-  m_info->open_list.data=(void*) m_info;
-  myisam_open_list=list_add(myisam_open_list,&m_info->open_list);
+
+  if (my_hash_insert(&myisam_open_table_hash, (uchar *) m_info))
+    goto err;
 
   mysql_mutex_unlock(&THR_LOCK_myisam);
 

=== modified file 'storage/myisam/mi_panic.c'
--- a/storage/myisam/mi_panic.c	2009-12-05 01:26:15 +0000
+++ b/storage/myisam/mi_panic.c	2010-10-21 10:51:58 +0000
@@ -26,90 +26,102 @@
 int mi_panic(enum ha_panic_function flag)
 {
   int error=0;
-  LIST *list_element,*next_open;
   MI_INFO *info;
+  uint idx;
   DBUG_ENTER("mi_panic");
 
   mysql_mutex_lock(&THR_LOCK_myisam);
-  for (list_element=myisam_open_list ; list_element ; list_element=next_open)
+  if (my_hash_inited(&myisam_open_table_hash))
   {
-    next_open=list_element->next;		/* Save if close */
-    info=(MI_INFO*) list_element->data;
-    switch (flag) {
-    case HA_PANIC_CLOSE:
-      mysql_mutex_unlock(&THR_LOCK_myisam);     /* Not exactly right... */
-      if (mi_close(info))
-	error=my_errno;
-      mysql_mutex_lock(&THR_LOCK_myisam);
-      break;
-    case HA_PANIC_WRITE:		/* Do this to free databases */
-#ifdef CANT_OPEN_FILES_TWICE
-      if (info->s->options & HA_OPTION_READ_ONLY_DATA)
-	break;
-#endif
-      if (flush_key_blocks(info->s->key_cache, info->s->kfile, FLUSH_RELEASE))
-	error=my_errno;
-      if (info->opt_flag & WRITE_CACHE_USED)
-	if (flush_io_cache(&info->rec_cache))
-	  error=my_errno;
-      if (info->opt_flag & READ_CACHE_USED)
+    if (flag == HA_PANIC_CLOSE)
+    {
+      while (myisam_open_table_hash.records)
       {
-	if (flush_io_cache(&info->rec_cache))
-	  error=my_errno;
-	reinit_io_cache(&info->rec_cache,READ_CACHE,0,
-		       (pbool) (info->lock_type != F_UNLCK),1);
+        /* 
+           As long as there are records in the hash, fetch the
+           first, and close it.
+        */
+        info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, 0);
+        mysql_mutex_unlock(&THR_LOCK_myisam);     /* Not exactly right... */
+        if (mi_close(info))
+          error= my_errno;
+        mysql_mutex_lock(&THR_LOCK_myisam);
       }
-      if (info->lock_type != F_UNLCK && ! info->was_locked)
+      (void) mi_log(0);				/* Close log if neaded */
+      ft_free_stopwords();
+    }
+    else
+    {
+      for (idx= 0; idx < myisam_open_table_hash.records; ++idx)
       {
-	info->was_locked=info->lock_type;
-	if (mi_lock_database(info,F_UNLCK))
-	  error=my_errno;
-      }
+        info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, idx);
+        switch (flag) {
+        case HA_PANIC_CLOSE: break;      /* To eliminate warning.  Handled above */
+        case HA_PANIC_WRITE:		/* Do this to free databases */
 #ifdef CANT_OPEN_FILES_TWICE
-      if (info->s->kfile >= 0 && mysql_file_close(info->s->kfile, MYF(0)))
-	error = my_errno;
-      if (info->dfile >= 0 && mysql_file_close(info->dfile, MYF(0)))
-	error = my_errno;
-      info->s->kfile=info->dfile= -1;	/* Files aren't open anymore */
-      break;
+          if (info->s->options & HA_OPTION_READ_ONLY_DATA)
+            break;
 #endif
-    case HA_PANIC_READ:			/* Restore to before WRITE */
+          if (flush_key_blocks(info->s->key_cache, info->s->kfile, FLUSH_RELEASE))
+            error=my_errno;
+          if (info->opt_flag & WRITE_CACHE_USED)
+            if (flush_io_cache(&info->rec_cache))
+              error=my_errno;
+          if (info->opt_flag & READ_CACHE_USED)
+          {
+            if (flush_io_cache(&info->rec_cache))
+              error=my_errno;
+            reinit_io_cache(&info->rec_cache,READ_CACHE,0,
+                            (pbool) (info->lock_type != F_UNLCK),1);
+          }
+          if (info->lock_type != F_UNLCK && ! info->was_locked)
+          {
+            info->was_locked=info->lock_type;
+            if (mi_lock_database(info,F_UNLCK))
+              error=my_errno;
+          }
 #ifdef CANT_OPEN_FILES_TWICE
-      {					/* Open closed files */
-	char name_buff[FN_REFLEN];
-	if (info->s->kfile < 0)
-          if ((info->s->kfile= mysql_file_open(mi_key_file_kfile,
-                                               fn_format(name_buff,
-                                                         info->filename, "",
-                                                         N_NAME_IEXT, 4),
-                                               info->mode, MYF(MY_WME))) < 0)
-	    error = my_errno;
-	if (info->dfile < 0)
-	{
-          if ((info->dfile= mysql_file_open(mi_key_file_dfile,
-                                            fn_format(name_buff,
-                                                      info->filename, "",
-                                                      N_NAME_DEXT, 4),
-                                            info->mode, MYF(MY_WME))) < 0)
-	    error = my_errno;
-	  info->rec_cache.file=info->dfile;
-	}
-      }
+          if (info->s->kfile >= 0 && mysql_file_close(info->s->kfile, MYF(0)))
+            error = my_errno;
+          if (info->dfile >= 0 && mysql_file_close(info->dfile, MYF(0)))
+            error = my_errno;
+          info->s->kfile=info->dfile= -1;	/* Files aren't open anymore */
+          break;
 #endif
-      if (info->was_locked)
-      {
-	if (mi_lock_database(info, info->was_locked))
-	  error=my_errno;
-	info->was_locked=0;
+        case HA_PANIC_READ:			/* Restore to before WRITE */
+#ifdef CANT_OPEN_FILES_TWICE
+          {					/* Open closed files */
+            char name_buff[FN_REFLEN];
+            if (info->s->kfile < 0)
+              if ((info->s->kfile= mysql_file_open(mi_key_file_kfile,
+                                                   fn_format(name_buff,
+                                                             info->filename, "",
+                                                             N_NAME_IEXT, 4),
+                                                   info->mode, MYF(MY_WME))) < 0)
+                error = my_errno;
+            if (info->dfile < 0)
+            {
+              if ((info->dfile= mysql_file_open(mi_key_file_dfile,
+                                                fn_format(name_buff,
+                                                          info->filename, "",
+                                                          N_NAME_DEXT, 4),
+                                                info->mode, MYF(MY_WME))) < 0)
+                error = my_errno;
+              info->rec_cache.file=info->dfile;
+            }
+          }
+#endif
+          if (info->was_locked)
+          {
+            if (mi_lock_database(info, info->was_locked))
+              error=my_errno;
+            info->was_locked=0;
+          }
+          break;
+        }
       }
-      break;
     }
   }
-  if (flag == HA_PANIC_CLOSE)
-  {
-    (void) mi_log(0);				/* Close log if neaded */
-    ft_free_stopwords();
-  }
   mysql_mutex_unlock(&THR_LOCK_myisam);
   if (!error)
     DBUG_RETURN(0);

=== modified file 'storage/myisam/mi_static.c'
--- a/storage/myisam/mi_static.c	2010-08-05 12:34:19 +0000
+++ b/storage/myisam/mi_static.c	2010-10-21 10:51:58 +0000
@@ -22,7 +22,7 @@
 #include "myisamdef.h"
 #endif
 
-LIST	*myisam_open_list=0;
+
 uchar	myisam_file_magic[]=
 { (uchar) 254, (uchar) 254,'\007', '\001', };
 uchar	myisam_pack_file_magic[]=
@@ -40,6 +40,9 @@ ulong myisam_concurrent_insert= 0;
 ulonglong myisam_max_temp_length= MAX_FILE_SIZE;
 ulong    myisam_data_pointer_size=4;
 ulonglong    myisam_mmap_size= SIZE_T_MAX, myisam_mmap_used= 0;
+HASH     myisam_open_table_hash;
+
+
 
 static int always_valid(const char *filename __attribute__((unused)))
 {

=== modified file 'storage/myisam/myisamchk.c'
--- a/storage/myisam/myisamchk.c	2010-07-23 20:16:29 +0000
+++ b/storage/myisam/myisamchk.c	2010-10-21 10:51:58 +0000
@@ -88,6 +88,13 @@ int main(int argc, char **argv)
   get_options(&argc,(char***) &argv);
   myisam_quick_table_bits=decode_bits;
   error=0;
+
+  if (mi_init_open_table_hash(0))
+  {
+    fprintf(stderr, "Can't initialize MyISAM storage engine\n");
+    exit(-1);
+  }
+
   while (--argc >= 0)
   {
     int new_error=myisamchk(&check_param, *(argv++));

=== modified file 'storage/myisam/myisamdef.h'
--- a/storage/myisam/myisamdef.h	2010-07-26 11:34:07 +0000
+++ b/storage/myisam/myisamdef.h	2010-10-21 10:51:58 +0000
@@ -25,6 +25,7 @@
 #include <my_no_pthread.h>
 #endif
 #include <mysql/psi/mysql_file.h>
+#include "hash.h"
 
 /* undef map from my_nosys; We need test-if-disk full */
 #if defined(my_write)
@@ -287,7 +288,6 @@ struct st_myisam_info {
   uint	data_changed;			/* Somebody has changed data */
   uint	save_update;			/* When using KEY_READ */
   int	save_lastinx;
-  LIST	open_list;
   IO_CACHE rec_cache;			/* When cacheing records */
   uint  preload_buff_size;              /* When preloading indexes */
   myf lock_wait;			/* is 0 or MY_DONT_WAIT */
@@ -477,7 +477,7 @@ extern mysql_mutex_t THR_LOCK_myisam;
 
 	/* Some extern variables */
 
-extern LIST *myisam_open_list;
+extern HASH myisam_open_table_hash;
 extern uchar myisam_file_magic[], myisam_pack_file_magic[];
 extern uint myisam_read_vec[], myisam_readnext_vec[];
 extern uint myisam_quick_table_bits;
@@ -759,6 +759,7 @@ my_bool mi_check_status(void* param);
 void mi_disable_non_unique_index(MI_INFO *info, ha_rows rows);
 
 extern MI_INFO *test_if_reopen(char *filename);
+extern int mi_init_open_table_hash(ulong size);
 my_bool check_table_is_closed(const char *name, const char *where);
 int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *orn_name,
                      File file_to_dup);

=== modified file 'storage/myisam/myisampack.c'
--- a/storage/myisam/myisampack.c	2010-07-23 20:17:55 +0000
+++ b/storage/myisam/myisampack.c	2010-10-21 10:51:58 +0000
@@ -210,6 +210,13 @@ int main(int argc, char **argv)
   if (load_defaults("my",load_default_groups,&argc,&argv))
     exit(1);
 
+  if (mi_init_open_table_hash(0))
+  {
+    fputs("Can't initialize MyISAM storage engine", stderr);
+    exit(1);
+  }
+    
+
   default_argv= argv;
   get_options(&argc,&argv);
 


Attachment: [text/bzr-bundle] bzr/magne.mahre@sun.com-20101021105158-hzgqvagpc102j831.bundle
Thread
bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3328) Bug#49177Magne Mahre21 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch(magne.mahre:3328) Bug#49177Sergey Vojtovich19 Nov
    • Re: bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3328)Bug#49177Mattias Jonsson19 Nov
      • Re: bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3328)Bug#49177Magne Mæhre19 Nov
        • Re: bzr commit into mysql-next-mr-bugfixing branch(magne.mahre:3328) Bug#49177Sergey Vojtovich19 Nov
          • Re: bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3328)Bug#49177Magne Mæhre19 Nov
            • Re: bzr commit into mysql-next-mr-bugfixing branch(magne.mahre:3328) Bug#49177Sergey Vojtovich19 Nov