List:Commits« Previous MessageNext Message »
From:Ingo Struewing Date:October 19 2007 4:17pm
Subject:bk commit into 5.0 tree (istruewing:1.2537) BUG#4692
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of istruewing. When istruewing does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-10-19 18:17:02+02:00, istruewing@stripped +3 -0
  Bug#4692 - DISABLE/ENABLE KEYS waste a space
  
  Disabling and enabling indexes on a non-empty table grows the
  index file.
  
  Disabling indexes just sets a flag per non-unique index and does not
  free the index blocks of the affected indexes. Re-enabling indexes
  creates new indexes with new blocks. The old blocks remain unused
  in the index file.
  
  Fixed by dropping and re-creating all indexes if non-empty disabled
  indexes exist when enabling indexes. Dropping all indexes resets
  the internal end-of-file marker to the end of the index file header.
  It also clears the root block pointers of every index and clears the
  deleted blocks chains. This way all blocks are declared as free.

  myisam/mi_check.c@stripped, 2007-10-19 18:17:01+02:00, istruewing@stripped +161 -57
    Bug#4692 - DISABLE/ENABLE KEYS waste a space
    Added function mi_drop_all_indexes() to support drop of all indexes
    in case we want to re-enable non-empty disabled indexes.
    Changed mi_repair(), mi_repair_by_sort(), and mi_repair_parallel()
    to use the new function instead of duplicate drop index code.

  mysql-test/r/myisam.result@stripped, 2007-10-19 18:17:01+02:00, istruewing@stripped +25 -0
    Bug#4692 - DISABLE/ENABLE KEYS waste a space
    Added test result.

  mysql-test/t/myisam.test@stripped, 2007-10-19 18:17:01+02:00, istruewing@stripped +26 -0
    Bug#4692 - DISABLE/ENABLE KEYS waste a space
    Added test.

diff -Nrup a/myisam/mi_check.c b/myisam/mi_check.c
--- a/myisam/mi_check.c	2007-05-16 21:37:31 +02:00
+++ b/myisam/mi_check.c	2007-10-19 18:17:01 +02:00
@@ -1375,6 +1375,135 @@ int chk_data_link(MI_CHECK *param, MI_IN
 } /* chk_data_link */
 
 
+/**
+  @brief Drop all indexes
+
+  @param[in]    param           check parameters
+  @param[in]    info            MI_INFO handle
+  @param[in]    force           if to force drop all indexes
+
+  @return       status
+    @retval     0               OK
+    @retval     1               Error
+
+  @note Once allocated, index blocks remain part of the key file
+    forever. When indexes are disabled, no block is freed.
+    When enabling indexes, no block is freed either. The new indexes
+    are create from new blocks. (Bug #4692)
+    Before recreating formerly disabled indexes, the unused blocks
+    must be freed. There are two options to do this:
+    - Follow the tree of disabled indexes, add all blocks to the
+      deleted blocks chain. Would require a lot of random I/O.
+    - Drop all blocks by clearing all index root pointers and all
+      delete chain pointers and resetting key_file_length to the end
+      of the index file header. This requires to recreate all indexes,
+      even those that may still be intact.
+    The second method is probably faster in most cases.
+
+    When disabling indexes, MySQL disables either all indexes or all
+    non-unique indexes. When MySQL [re-]enables disabled indexes
+    (T_CREATE_MISSING_KEYS), then we either have "lost" blocks in the
+    index file, or there are no non-unique indexes. In the latter case,
+    mi_repair*() would not be called as there would be no disabled
+    indexes.
+
+    If there would be more unique indexes than disabled (non-unique)
+    indexes, we could do the first method. But this is not implemented
+    yet. By now we drop and recreate all indexes when repair is called.
+
+    However, there is an exception. Sometimes MySQL disables non-unique
+    indexes when the table is empty (e.g. when copying a table in
+    mysql_alter_table()). When enabling the non-unique indexes, they
+    are still empty. So there is no index block that can be lost. This
+    optimization is implemented in this function.
+
+    Note that in normal repair (T_CREATE_MISSING_KEYS not set) we
+    recreate all enabled indexes unconditonally. We do not change the
+    key_map. Otherwise we invert the key map temporarily (outside of
+    this function) and recreate the then "seemingly" enabled indexes.
+    When we cannot use the optimization, and drop all indexes, we
+    pretend that all indexes were disabled. By the inversion, we will
+    then recrate all indexes.
+*/
+
+static int mi_drop_all_indexes(MI_CHECK *param, MI_INFO *info, my_bool force)
+{
+  MYISAM_SHARE *share= info->s;
+  uint i;
+  int error= 1;
+  DBUG_ENTER("mi_drop_all_indexes");
+
+  /*
+    If any of the disabled indexes has a key block assigned, we must
+    drop and recreate all indexes to avoid losing index blocks.
+
+    If we want to recreate disabled indexes only _and_ all of these
+    indexes are empty, we don't need to recreate the existing indexes.
+  */
+  if (!force && (param->testflag & T_CREATE_MISSING_KEYS))
+  {
+    DBUG_PRINT("repair", ("creating missing indexes"));
+    for (i= 0; i < info->s->base.keys; i++)
+    {
+      DBUG_PRINT("repair", ("index #: %u  key_root: 0x%lx  active: %d",
+                            i, (long) share->state.key_root[i],
+                            mi_is_key_active(share->state.key_map, i)));
+      if ((share->state.key_root[i] != HA_OFFSET_ERROR) &&
+          !mi_is_key_active(share->state.key_map, i))
+      {
+        /*
+          This index has at least one key block and it is disabled.
+          We would lose its block(s) if would just recreate it.
+          So we need to drop and recreate all indexes.
+        */
+        DBUG_PRINT("repair", ("nonempty and disabled: recreate all"));
+        break;
+      }
+    }
+    if (i >= info->s->base.keys)
+    {
+      /*
+        All of the disabled indexes are empty. We can just recreate them.
+        Flush dirty blocks of this index file from key cache and remove
+        all blocks of this index file from key cache.
+      */
+      DBUG_PRINT("repair", ("all disabled are empty: create missing"));
+      error= flush_key_blocks(share->key_cache, share->kfile,
+                              FLUSH_FORCE_WRITE);
+      goto end;
+    }
+    /*
+      We do now drop all indexes and declare them disabled. With the
+      T_CREATE_MISSING_KEYS flag, mi_repair*() will recreate all
+      disabled indexes and enable them.
+    */
+    mi_clear_all_keys_active(share->state.key_map);
+    DBUG_PRINT("repair", ("declared all indexes disabled"));
+  }
+
+  /* Remove all key blocks of this index file from key cache. */
+  if (flush_key_blocks(share->key_cache, share->kfile, FLUSH_IGNORE_CHANGED))
+    goto end;
+
+  /* Clear index root block pointers. */
+  for (i= 0; i < info->s->base.keys; i++)
+    share->state.key_root[i]= HA_OFFSET_ERROR;
+
+  /* Clear the delete chains. */
+  for (i= 0; i < share->state.header.max_block_size; i++)
+    share->state.key_del[i]= HA_OFFSET_ERROR;
+
+  /* Reset index file length to end of index file header. */
+  info->state->key_file_length= share->base.keystart;
+
+  DBUG_PRINT("repair", ("dropped all indexes"));
+  error= 0;
+
+ end:
+  DBUG_RETURN(error);
+}
+
+
 	/* Recover old table by reading each record and writing all keys */
 	/* Save new datafile-name in temp_filename */
 
@@ -1382,7 +1511,6 @@ int mi_repair(MI_CHECK *param, register 
 	      my_string name, int rep_quick)
 {
   int error,got_error;
-  uint i;
   ha_rows start_records,new_header_length;
   my_off_t del;
   File new_file;
@@ -1486,25 +1614,10 @@ int mi_repair(MI_CHECK *param, register 
 
   info->update= (short) (HA_STATE_CHANGED | HA_STATE_ROW_CHANGED);
 
-  /*
-    Clear all keys. Note that all key blocks allocated until now remain
-    "dead" parts of the key file. (Bug #4692)
-  */
-  for (i=0 ; i < info->s->base.keys ; i++)
-    share->state.key_root[i]= HA_OFFSET_ERROR;
-
-  /* Drop the delete chain. */
-  for (i=0 ; i < share->state.header.max_block_size ; i++)
-    share->state.key_del[i]=  HA_OFFSET_ERROR;
-
-  /*
-    If requested, activate (enable) all keys in key_map. In this case,
-    all indexes will be (re-)built.
-  */
+  /* This function always recreates all enabled indexes. */
   if (param->testflag & T_CREATE_MISSING_KEYS)
     mi_set_all_keys_active(share->state.key_map, share->base.keys);
-
-  info->state->key_file_length=share->base.keystart;
+  mi_drop_all_indexes(param, info, TRUE);
 
   lock_memory(param);			/* Everything is alloced */
 
@@ -2105,7 +2218,7 @@ int mi_repair_by_sort(MI_CHECK *param, r
   ulong   *rec_per_key_part;
   char llbuff[22];
   SORT_INFO sort_info;
-  ulonglong key_map=share->state.key_map;
+  ulonglong key_map;
   DBUG_ENTER("mi_repair_by_sort");
 
   start_records=info->state->records;
@@ -2179,25 +2292,14 @@ int mi_repair_by_sort(MI_CHECK *param, r
   }
 
   info->update= (short) (HA_STATE_CHANGED | HA_STATE_ROW_CHANGED);
-  if (!(param->testflag & T_CREATE_MISSING_KEYS))
-  {
-    /*
-      Flush key cache for this file if we are calling this outside
-      myisamchk
-    */
-    flush_key_blocks(share->key_cache,share->kfile, FLUSH_IGNORE_CHANGED);
-    /* Clear the pointers to the given rows */
-    for (i=0 ; i < share->base.keys ; i++)
-      share->state.key_root[i]= HA_OFFSET_ERROR;
-    for (i=0 ; i < share->state.header.max_block_size ; i++)
-      share->state.key_del[i]=  HA_OFFSET_ERROR;
-    info->state->key_file_length=share->base.keystart;
-  }
-  else
+
+  /* Optionally drop indexes and optionally modify the key_map. */
+  mi_drop_all_indexes(param, info, FALSE);
+  key_map= share->state.key_map;
+  if (param->testflag & T_CREATE_MISSING_KEYS)
   {
-    if (flush_key_blocks(share->key_cache,share->kfile, FLUSH_FORCE_WRITE))
-      goto err;
-    key_map= ~key_map;				/* Create the missing keys */
+    /* Invert the copied key_map to recreate all disabled indexes. */
+    key_map= ~key_map;
   }
 
   sort_info.info=info;
@@ -2240,6 +2342,10 @@ int mi_repair_by_sort(MI_CHECK *param, r
     sort_param.read_cache=param->read_cache;
     sort_param.keyinfo=share->keyinfo+sort_param.key;
     sort_param.seg=sort_param.keyinfo->seg;
+    /*
+      Skip this index if it is marked disabled in the copied
+      (and possibly inverted) key_map.
+    */
     if (! mi_is_key_active(key_map, sort_param.key))
     {
       /* Remember old statistics for key */
@@ -2247,6 +2353,8 @@ int mi_repair_by_sort(MI_CHECK *param, r
 	     (char*) (share->state.rec_per_key_part +
 		      (uint) (rec_per_key_part - param->rec_per_key_part)),
 	     sort_param.keyinfo->keysegs*sizeof(*rec_per_key_part));
+      DBUG_PRINT("repair", ("skipping seemingly disabled index #: %u",
+                            sort_param.key));
       continue;
     }
 
@@ -2302,8 +2410,11 @@ int mi_repair_by_sort(MI_CHECK *param, r
     if (param->testflag & T_STATISTICS)
       update_key_parts(sort_param.keyinfo, rec_per_key_part, sort_param.unique,
                        param->stats_method == MI_STATS_METHOD_IGNORE_NULLS?
-                       sort_param.notnull: NULL,(ulonglong) info->state->records);
+                       sort_param.notnull: NULL,
+                       (ulonglong) info->state->records);
+    /* Enable this index in the permanent (not the copied) key_map. */
     mi_set_key_active(share->state.key_map, sort_param.key);
+    DBUG_PRINT("repair", ("set enabled index #: %u", sort_param.key));
 
     if (sort_param.fix_datafile)
     {
@@ -2504,7 +2615,7 @@ int mi_repair_parallel(MI_CHECK *param, 
   IO_CACHE new_data_cache; /* For non-quick repair. */
   IO_CACHE_SHARE io_share;
   SORT_INFO sort_info;
-  ulonglong key_map=share->state.key_map;
+  ulonglong key_map;
   pthread_attr_t thr_attr;
   DBUG_ENTER("mi_repair_parallel");
 
@@ -2608,25 +2719,14 @@ int mi_repair_parallel(MI_CHECK *param, 
   }
 
   info->update= (short) (HA_STATE_CHANGED | HA_STATE_ROW_CHANGED);
-  if (!(param->testflag & T_CREATE_MISSING_KEYS))
-  {
-    /*
-      Flush key cache for this file if we are calling this outside
-      myisamchk
-    */
-    flush_key_blocks(share->key_cache,share->kfile, FLUSH_IGNORE_CHANGED);
-    /* Clear the pointers to the given rows */
-    for (i=0 ; i < share->base.keys ; i++)
-      share->state.key_root[i]= HA_OFFSET_ERROR;
-    for (i=0 ; i < share->state.header.max_block_size ; i++)
-      share->state.key_del[i]=  HA_OFFSET_ERROR;
-    info->state->key_file_length=share->base.keystart;
-  }
-  else
+
+  /* Optionally drop indexes and optionally modify the key_map. */
+  mi_drop_all_indexes(param, info, FALSE);
+  key_map= share->state.key_map;
+  if (param->testflag & T_CREATE_MISSING_KEYS)
   {
-    if (flush_key_blocks(share->key_cache,share->kfile, FLUSH_FORCE_WRITE))
-      goto err;
-    key_map= ~key_map;				/* Create the missing keys */
+    /* Invert the copied key_map to recreate all disabled indexes. */
+    key_map= ~key_map;
   }
 
   sort_info.info=info;
@@ -2682,6 +2782,10 @@ int mi_repair_parallel(MI_CHECK *param, 
     sort_param[i].key=key;
     sort_param[i].keyinfo=share->keyinfo+key;
     sort_param[i].seg=sort_param[i].keyinfo->seg;
+    /*
+      Skip this index if it is marked disabled in the copied
+      (and possibly inverted) key_map.
+    */
     if (! mi_is_key_active(key_map, key))
     {
       /* Remember old statistics for key */
diff -Nrup a/mysql-test/r/myisam.result b/mysql-test/r/myisam.result
--- a/mysql-test/r/myisam.result	2007-05-22 14:58:29 +02:00
+++ b/mysql-test/r/myisam.result	2007-10-19 18:17:01 +02:00
@@ -1806,4 +1806,29 @@ SELECT a FROM t1 FORCE INDEX (inx) WHERE
 a
 1
 DROP TABLE t1;
+CREATE TABLE t1 (c1 INT, c2 INT, UNIQUE INDEX (c1), INDEX (c2)) ENGINE=MYISAM;
+SHOW TABLE STATUS LIKE 't1';
+Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
+t1	MyISAM	10	Fixed	0	#	#	#	1024	#	#	#	#	#	#	#		
+INSERT INTO t1 VALUES (1,1);
+SHOW TABLE STATUS LIKE 't1';
+Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
+t1	MyISAM	10	Fixed	1	#	#	#	3072	#	#	#	#	#	#	#		
+ALTER TABLE t1 DISABLE KEYS;
+SHOW TABLE STATUS LIKE 't1';
+Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
+t1	MyISAM	10	Fixed	1	#	#	#	3072	#	#	#	#	#	#	#		
+ALTER TABLE t1 ENABLE KEYS;
+SHOW TABLE STATUS LIKE 't1';
+Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
+t1	MyISAM	10	Fixed	1	#	#	#	3072	#	#	#	#	#	#	#		
+ALTER TABLE t1 DISABLE KEYS;
+SHOW TABLE STATUS LIKE 't1';
+Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
+t1	MyISAM	10	Fixed	1	#	#	#	3072	#	#	#	#	#	#	#		
+ALTER TABLE t1 ENABLE KEYS;
+SHOW TABLE STATUS LIKE 't1';
+Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
+t1	MyISAM	10	Fixed	1	#	#	#	3072	#	#	#	#	#	#	#		
+DROP TABLE t1;
 End of 5.0 tests
diff -Nrup a/mysql-test/t/myisam.test b/mysql-test/t/myisam.test
--- a/mysql-test/t/myisam.test	2007-08-29 14:44:21 +02:00
+++ b/mysql-test/t/myisam.test	2007-10-19 18:17:01 +02:00
@@ -1161,4 +1161,30 @@ ALTER TABLE t1 ENABLE KEYS;
 SELECT a FROM t1 FORCE INDEX (inx) WHERE a=1;
 DROP TABLE t1;
 
+#
+# Bug#4692 - DISABLE/ENABLE KEYS waste a space
+#
+CREATE TABLE t1 (c1 INT, c2 INT, UNIQUE INDEX (c1), INDEX (c2)) ENGINE=MYISAM;
+--replace_column 6 # 7 # 8 # 10 # 11 # 12 # 13 # 14 # 15 # 16 #
+SHOW TABLE STATUS LIKE 't1';
+INSERT INTO t1 VALUES (1,1);
+--replace_column 6 # 7 # 8 # 10 # 11 # 12 # 13 # 14 # 15 # 16 #
+SHOW TABLE STATUS LIKE 't1';
+ALTER TABLE t1 DISABLE KEYS;
+--replace_column 6 # 7 # 8 # 10 # 11 # 12 # 13 # 14 # 15 # 16 #
+SHOW TABLE STATUS LIKE 't1';
+ALTER TABLE t1 ENABLE KEYS;
+--replace_column 6 # 7 # 8 # 10 # 11 # 12 # 13 # 14 # 15 # 16 #
+SHOW TABLE STATUS LIKE 't1';
+ALTER TABLE t1 DISABLE KEYS;
+--replace_column 6 # 7 # 8 # 10 # 11 # 12 # 13 # 14 # 15 # 16 #
+SHOW TABLE STATUS LIKE 't1';
+ALTER TABLE t1 ENABLE KEYS;
+--replace_column 6 # 7 # 8 # 10 # 11 # 12 # 13 # 14 # 15 # 16 #
+SHOW TABLE STATUS LIKE 't1';
+#--exec ls -log var/master-data/test/t1.MYI
+#--exec myisamchk -dvv var/master-data/test/t1.MYI
+#--exec myisamchk -iev var/master-data/test/t1.MYI
+DROP TABLE t1;
+
 --echo End of 5.0 tests
Thread
bk commit into 5.0 tree (istruewing:1.2537) BUG#4692Ingo Struewing19 Oct