bk commit into 5.1 tree (istruewing:1.2423) BUG#25460

[ingo]

Below is the list of changes that have just been committed into a local
5.1 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-02-08 21:24:12+01:00, istruewing@stripped +5 -0
  Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
  
  Under high load it was possible that memory mapping was started on a table
  while other threads were working with the table.
  
  I fixed the start of memory mapping so that it is done at the first table
  open or when the requesting thread is using the table exclusively only.

  include/my_base.h@stripped, 2007-02-08 21:24:06+01:00, istruewing@stripped +1 -0
    Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
    Added a new MyISAM open_flag HA_OPEN_MMAP.

  storage/myisam/ha_myisam.cc@stripped, 2007-02-08 21:24:06+01:00, istruewing@stripped
+13 -3
    Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
    Replaced the call to mi_extra(... HA_EXTRA_MMAP ...) by the new open_flag
    HA_OPEN_MMAP. This effects that the mapping will no longer be done on
    every open of the table but just on the initial open, when the MyISAM
    share is created.

  storage/myisam/mi_dynrec.c@stripped, 2007-02-08 21:24:06+01:00, istruewing@stripped +8
-0
    Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
    Added a comment with a concern regarding the mmap flag MAP_NORESERVE.

  storage/myisam/mi_extra.c@stripped, 2007-02-08 21:24:06+01:00, istruewing@stripped +6
-1
    Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
    Limited the start of memory mapping to situations where the requesting
    thread has the table exclusively opened.

  storage/myisam/mi_open.c@stripped, 2007-02-08 21:24:06+01:00, istruewing@stripped +16
-0
    Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
    Added memory mapping code. Used if the new open_flag HA_OPEN_MMAP is set.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	istruewing
# Host:	synthia.local
# Root:	/home/mydev/mysql-5.1-bug25460

--- 1.95/include/my_base.h	2007-02-08 21:24:23 +01:00
+++ 1.96/include/my_base.h	2007-02-08 21:24:23 +01:00
@@ -47,6 +47,7 @@
 #define HA_OPEN_ABORT_IF_CRASHED	16
 #define HA_OPEN_FOR_REPAIR		32	/* open even if crashed */
 #define HA_OPEN_FROM_SQL_LAYER          64
+#define HA_OPEN_MMAP                    128     /* open memory mapped */
 
 	/* The following is parameter to ha_rkey() how to use key */
 

--- 1.57/storage/myisam/mi_dynrec.c	2007-02-08 21:24:23 +01:00
+++ 1.58/storage/myisam/mi_dynrec.c	2007-02-08 21:24:23 +01:00
@@ -71,6 +71,14 @@ my_bool mi_dynmap_file(MI_INFO *info, my
     DBUG_PRINT("warning", ("File is too large for mmap"));
     DBUG_RETURN(1);
   }
+  /*
+    I wonder if it is good to use MAP_NORESERVE. From the Linux man page:
+    MAP_NORESERVE
+      Do not reserve swap space for this mapping. When swap space is
+      reserved, one has the guarantee that it is possible to modify the
+      mapping. When swap space is not reserved one might get SIGSEGV
+      upon a write if no physical memory is available.
+  */
   info->s->file_map= (byte*)
                   my_mmap(0, (size_t)(size + MEMMAP_EXTRA_MARGIN),
                           info->s->mode==O_RDONLY ? PROT_READ :

--- 1.52/storage/myisam/mi_extra.c	2007-02-08 21:24:23 +01:00
+++ 1.53/storage/myisam/mi_extra.c	2007-02-08 21:24:23 +01:00
@@ -349,7 +349,12 @@ int mi_extra(MI_INFO *info, enum ha_extr
   case HA_EXTRA_MMAP:
 #ifdef HAVE_MMAP
     pthread_mutex_lock(&share->intern_lock);
-    if (!share->file_map)
+    /*
+      Memory map the data file if it is not already mapped and if there
+      are no other threads using this table. intern_lock prevents other
+      threads from starting to use the table while we are mapping it.
+    */
+    if (!share->file_map && (share->tot_locks == 1))
     {
       if (mi_dynmap_file(info, share->state.state.data_file_length))
       {

--- 1.115/storage/myisam/mi_open.c	2007-02-08 21:24:23 +01:00
+++ 1.116/storage/myisam/mi_open.c	2007-02-08 21:24:23 +01:00
@@ -506,6 +506,22 @@ MI_INFO *mi_open(const char *name, int m
       share->data_file_type = DYNAMIC_RECORD;
     my_afree((gptr) disk_cache);
     mi_setup_functions(share);
+    if (open_flags & HA_OPEN_MMAP)
+    {
+      info.s= share;
+      if (mi_dynmap_file(&info, share->state.state.data_file_length))
+      {
+        /* purecov: begin inspected */
+        /* Ignore if mmap fails. Use file I/O instead. */
+        DBUG_PRINT("warning", ("mmap failed: errno: %d", errno));
+        /* purecov: end */
+      }
+      else
+      {
+        share->file_read= mi_mmap_pread;
+        share->file_write= mi_mmap_pwrite;
+      }
+    }
     share->is_log_table= FALSE;
 #ifdef THREAD
     thr_lock_init(&share->lock);

--- 1.208/storage/myisam/ha_myisam.cc	2007-02-08 21:24:23 +01:00
+++ 1.209/storage/myisam/ha_myisam.cc	2007-02-08 21:24:23 +01:00
@@ -598,14 +598,24 @@ bool ha_myisam::check_if_locking_is_allo
 int ha_myisam::open(const char *name, int mode, uint test_if_locked)
 {
   uint i;
+
+  /*
+    If the user wants to have memory mapped data files, add an open_flag.
+    Do not memory map temporary tables because they are expected to be
+    inserted and thus extended a lot. Memory mapping is efficient for
+    files that keep their size, but very inefficient for growing files.
+    Using an open_flag instead of calling mi_extra(... HA_EXTRA_MMAP ...)
+    after mi_open() has the advantage that it is not repeated for every
+    open, but just done on the initial open, when the MyISAM share is created.
+  */
+  if (!(test_if_locked & HA_OPEN_TMP_TABLE) && opt_myisam_use_mmap)
+    test_if_locked|= HA_OPEN_MMAP;
+
   if (!(file=mi_open(name, mode, test_if_locked | HA_OPEN_FROM_SQL_LAYER)))
     return (my_errno ? my_errno : -1);
   
   if (test_if_locked & (HA_OPEN_IGNORE_IF_LOCKED | HA_OPEN_TMP_TABLE))
     VOID(mi_extra(file, HA_EXTRA_NO_WAIT_LOCK, 0));
-
-  if (!(test_if_locked & HA_OPEN_TMP_TABLE) && opt_myisam_use_mmap)
-    VOID(mi_extra(file, HA_EXTRA_MMAP, 0));
 
   info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
   if (!(test_if_locked & HA_OPEN_WAIT_IF_LOCKED))