List:Commits« Previous MessageNext Message »
From:Ramil Kalimullin Date:January 26 2009 6:35am
Subject:bzr commit into mysql-5.1-bugteam branch (ramil:2747) Bug#37756
View as plain text  
#At file:///home/ram/mysql/b37756-5.1-bugteam/ based on revid:gshchepa@stripped

 2747 Ramil Kalimullin	2009-01-26
      Fix for bug #37756: enabling fulltext indexes with
        myisam_repair_threads > 1 causes crash
      
      Problem: parallel repair (myisam_repair_threads > 1) of a myisam
      table with two or more fulltext keys that use the same parser may
      lead to a server crash. ALTER TABLE ENABLE KEYS is affected as well.
      
      Fix: properly initialize fulltext structures for parallel repair.
      
      Note: 1. there's no deterministic test case.
      2. now we call parser->init() for each fulltext key
      (not for each fulltext parser used).
modified:
  include/myisam.h
  storage/myisam/ft_parser.c
  storage/myisam/ftdefs.h
  storage/myisam/mi_check.c
  storage/myisam/mi_open.c
  storage/myisam/myisamdef.h

per-file messages:
  storage/myisam/ft_parser.c
    Fix for bug #37756: enabling fulltext indexes with
      myisam_repair_threads > 1 causes crash
    
    In ftparser_call_initializer() we "group" parsers
    and allocate parameters for each unique parser used.
    In case of parallel repairing we may have a bunch
    of parsers that use the only MI_INFO structure.
    Each of these parsers must have its own parameter
    structure in order not to interfere with others.
    
    Moreover, the allocation is done without mutex
    lock so parallel ftparser_call_initializer() calls
    are unsafe.
    
    Now we don't "group" the fulltext parsers.
    Parameter allocation code moved to ftparser_alloc_param()
    function which is called from mi_repair_parallel() as well
    to allocate parameters for each parser used.
  storage/myisam/ftdefs.h
    Fix for bug #37756: enabling fulltext indexes with
      myisam_repair_threads > 1 causes crash
    
    ftparser_alloc_param(MI_INFO *info) added.
  storage/myisam/mi_check.c
    Fix for bug #37756: enabling fulltext indexes with
      myisam_repair_threads > 1 causes crash
    
    ftparser_alloc_param(info) called in case of parallel
    repair to allocate fulltext parser parameters.
  storage/myisam/mi_open.c
    Fix for bug #37756: enabling fulltext indexes with
      myisam_repair_threads > 1 causes crash
    
    set keyinfo->ftkey_nr and share->ftkeys.
=== modified file 'include/myisam.h'
--- a/include/myisam.h	2009-01-15 18:11:25 +0000
+++ b/include/myisam.h	2009-01-26 06:35:15 +0000
@@ -185,7 +185,7 @@ typedef struct st_mi_keydef		/* Key defi
   uint16 maxlength;			/* max length of (packed) key (auto) */
   uint16 block_size_index;		/* block_size (auto) */
   uint32 version;			/* For concurrent read/write */
-  uint32 ftparser_nr;                   /* distinct ftparser number */
+  uint32 ftkey_nr;                      /* full-text index number */
 
   HA_KEYSEG *seg,*end;
   struct st_mysql_ftparser *parser;     /* Fulltext [pre]parser */

=== modified file 'storage/myisam/ft_parser.c'
--- a/storage/myisam/ft_parser.c	2007-11-14 14:32:03 +0000
+++ b/storage/myisam/ft_parser.c	2009-01-26 06:35:15 +0000
@@ -323,60 +323,41 @@ int ft_parse(TREE *wtree, uchar *doc, in
   DBUG_RETURN(parser->parse(param));
 }
 
+
 #define MAX_PARAM_NR 2
-MYSQL_FTPARSER_PARAM *ftparser_call_initializer(MI_INFO *info,
-                                                uint keynr, uint paramnr)
+
+MYSQL_FTPARSER_PARAM* ftparser_alloc_param(MI_INFO *info)
 {
-  uint32 ftparser_nr;
-  struct st_mysql_ftparser *parser;
-  if (! info->ftparser_param)
+  if (!info->ftparser_param)
   {
-    /* info->ftparser_param can not be zero after the initialization,
-       because it always includes built-in fulltext parser. And built-in
-       parser can be called even if the table has no fulltext indexes and
-       no varchar/text fields. */
-    if (! info->s->ftparsers)
-    {
-      /* It's ok that modification to shared structure is done w/o mutex
-         locks, because all threads would set the same variables to the
-         same values. */
-      uint i, j, keys= info->s->state.header.keys, ftparsers= 1;
-      for (i= 0; i < keys; i++)
-      {
-        MI_KEYDEF *keyinfo= &info->s->keyinfo[i];
-        if (keyinfo->flag & HA_FULLTEXT)
-        {
-          for (j= 0;; j++)
-          {
-            if (j == i)
-            {
-              keyinfo->ftparser_nr= ftparsers++;
-              break;
-            }
-            if (info->s->keyinfo[j].flag & HA_FULLTEXT &&
-                keyinfo->parser == info->s->keyinfo[j].parser)
-            {
-              keyinfo->ftparser_nr= info->s->keyinfo[j].ftparser_nr;
-              break;
-            }
-          }
-        }
-      }
-      info->s->ftparsers= ftparsers;
-    }
-    /*
-      We have to allocate two MYSQL_FTPARSER_PARAM structures per plugin
-      because in a boolean search a parser is called recursively
-      ftb_find_relevance* calls ftb_check_phrase*
-      (MAX_PARAM_NR=2)
+    /* 
+.     info->ftparser_param can not be zero after the initialization,
+      because it always includes built-in fulltext parser. And built-in
+      parser can be called even if the table has no fulltext indexes and
+      no varchar/text fields.
+
+      ftb_find_relevance... parser (ftb_find_relevance_parse,
+      ftb_find_relevance_add_word) calls ftb_check_phrase... parser
+      (ftb_check_phrase_internal, ftb_phrase_add_word). Thus MAX_PARAM_NR=2.
     */
     info->ftparser_param= (MYSQL_FTPARSER_PARAM *)
       my_malloc(MAX_PARAM_NR * sizeof(MYSQL_FTPARSER_PARAM) *
-                info->s->ftparsers, MYF(MY_WME|MY_ZEROFILL));
+                info->s->ftkeys, MYF(MY_WME | MY_ZEROFILL));
     init_alloc_root(&info->ft_memroot, FTPARSER_MEMROOT_ALLOC_SIZE, 0);
-    if (! info->ftparser_param)
-      return 0;
   }
+  return info->ftparser_param;
+}
+
+
+MYSQL_FTPARSER_PARAM *ftparser_call_initializer(MI_INFO *info,
+                                                uint keynr, uint paramnr)
+{
+  uint32 ftparser_nr;
+  struct st_mysql_ftparser *parser;
+  
+  if (!ftparser_alloc_param(info))
+    return 0;
+
   if (keynr == NO_SUCH_KEY)
   {
     ftparser_nr= 0;
@@ -384,7 +365,7 @@ MYSQL_FTPARSER_PARAM *ftparser_call_init
   }
   else
   {
-    ftparser_nr= info->s->keyinfo[keynr].ftparser_nr;
+    ftparser_nr= info->s->keyinfo[keynr].ftkey_nr;
     parser= info->s->keyinfo[keynr].parser;
   }
   DBUG_ASSERT(paramnr < MAX_PARAM_NR);
@@ -416,7 +397,7 @@ void ftparser_call_deinitializer(MI_INFO
     for (j=0; j < MAX_PARAM_NR; j++)
     {
       MYSQL_FTPARSER_PARAM *ftparser_param=
-        &info->ftparser_param[keyinfo->ftparser_nr*MAX_PARAM_NR + j];
+        &info->ftparser_param[keyinfo->ftkey_nr * MAX_PARAM_NR + j];
       if (keyinfo->flag & HA_FULLTEXT && ftparser_param->mysql_add_word)
       {
         if (keyinfo->parser->deinit)

=== modified file 'storage/myisam/ftdefs.h'
--- a/storage/myisam/ftdefs.h	2007-05-10 09:59:39 +0000
+++ b/storage/myisam/ftdefs.h	2009-01-26 06:35:15 +0000
@@ -146,6 +146,7 @@ void ft_boolean_close_search(FT_INFO *);
 float ft_boolean_get_relevance(FT_INFO *);
 my_off_t ft_boolean_get_docid(FT_INFO *);
 void ft_boolean_reinit_search(FT_INFO *);
+MYSQL_FTPARSER_PARAM* ftparser_alloc_param(MI_INFO *info);
 extern MYSQL_FTPARSER_PARAM *ftparser_call_initializer(MI_INFO *info,
                                                        uint keynr,
                                                        uint paramnr);

=== modified file 'storage/myisam/mi_check.c'
--- a/storage/myisam/mi_check.c	2009-01-15 18:11:25 +0000
+++ b/storage/myisam/mi_check.c	2009-01-26 06:35:15 +0000
@@ -2397,7 +2397,7 @@ int mi_repair_by_sort(MI_CHECK *param, r
 
         Note, built-in parser is always nr. 0 - see ftparser_call_initializer()
       */
-      if (sort_param.keyinfo->ftparser_nr == 0)
+      if (sort_param.keyinfo->ftkey_nr == 0)
       {
         /*
           for built-in parser the number of generated index entries
@@ -2895,6 +2895,9 @@ int mi_repair_parallel(MI_CHECK *param, 
   sort_param[0].fix_datafile= (my_bool)(! rep_quick);
   sort_param[0].calc_checksum= test(param->testflag & T_CALC_CHECKSUM);
 
+  if (!ftparser_alloc_param(info))
+    goto err;
+
   sort_info.got_error=0;
   pthread_mutex_lock(&sort_info.mutex);
 

=== modified file 'storage/myisam/mi_open.c'
--- a/storage/myisam/mi_open.c	2009-01-15 18:11:25 +0000
+++ b/storage/myisam/mi_open.c	2009-01-26 06:35:15 +0000
@@ -331,6 +331,7 @@ MI_INFO *mi_open(const char *name, int m
     share->blocksize=min(IO_SIZE,myisam_block_size);
     {
       HA_KEYSEG *pos=share->keyparts;
+      uint32 ftkey_nr= 1;
       for (i=0 ; i < keys ; i++)
       {
         share->keyinfo[i].share= share;
@@ -412,6 +413,7 @@ MI_INFO *mi_open(const char *name, int m
             share->ft2_keyinfo.end=pos;
             setup_key_functions(& share->ft2_keyinfo);
           }
+          share->keyinfo[i].ftkey_nr= ftkey_nr++;
 	}
         setup_key_functions(share->keyinfo+i);
 	share->keyinfo[i].end=pos;
@@ -421,6 +423,7 @@ MI_INFO *mi_open(const char *name, int m
 	pos->flag=0;					/* For purify */
 	pos++;
       }
+      
       for (i=0 ; i < uniques ; i++)
       {
 	disk_pos=mi_uniquedef_read(disk_pos, &share->uniqueinfo[i]);
@@ -449,7 +452,7 @@ MI_INFO *mi_open(const char *name, int m
 	pos->flag=0;
 	pos++;
       }
-      share->ftparsers= 0;
+      share->ftkeys= ftkey_nr;
     }
 
     disk_pos_assert(disk_pos + share->base.fields *MI_COLUMNDEF_SIZE, end_pos);
@@ -1112,7 +1115,7 @@ uchar *mi_keydef_read(uchar *ptr, MI_KEY
    keydef->underflow_block_length=keydef->block_length/3;
    keydef->version	= 0;			/* Not saved */
    keydef->parser       = &ft_default_parser;
-   keydef->ftparser_nr  = 0;
+   keydef->ftkey_nr     = 0;
    return ptr;
 }
 

=== modified file 'storage/myisam/myisamdef.h'
--- a/storage/myisam/myisamdef.h	2009-01-15 18:11:25 +0000
+++ b/storage/myisam/myisamdef.h	2009-01-26 06:35:15 +0000
@@ -193,7 +193,7 @@ typedef struct st_mi_isam_share {	/* Sha
   ulong state_diff_length;
   uint	rec_reflength;			/* rec_reflength in use now */
   uint  unique_name_length;
-  uint32 ftparsers;                     /* Number of distinct ftparsers + 1 */
+  uint32 ftkeys;                        /* Number of full-text keys + 1 */
   File	kfile;				/* Shared keyfile */
   File	data_file;			/* Shared data file */
   int	mode;				/* mode of file on open */

Thread
bzr commit into mysql-5.1-bugteam branch (ramil:2747) Bug#37756Ramil Kalimullin26 Jan