#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, ¤t_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, ¤t_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
