Commit ace7e536 authored by unknown's avatar unknown
Browse files

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:
  Bug#25460 - High concurrency MyISAM access causes severe mysqld crash.
  Added a new MyISAM open_flag HA_OPEN_MMAP.
storage/myisam/ha_myisam.cc:
  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:
  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:
  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:
  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.
parent c52165de

include/my_base.h

+1 −0
Original line number Diff line number Diff line
@@ -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 */

storage/myisam/ha_myisam.cc

+19 −3
Original line number Diff line number Diff line
@@ -598,15 +598,31 @@ bool ha_myisam::check_if_locking_is_allowed(uint sql_command, 
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 the

    mapping is not repeated for every open, but just done on the initial

    open, when the MyISAM share is created. Everytime the server

    requires to open a new instance of a table it calls this method. We

    will always supply HA_OPEN_MMAP for a permanent table. However, the

    MyISAM storage engine will ignore this flag if this is a secondary

    open of a table that is in use by other threads already (if the

    MyISAM share exists already).

  */

  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))

    VOID(mi_extra(file, HA_EXTRA_WAIT_LOCK, 0));

storage/myisam/mi_dynrec.c

+8 −0
Original line number Diff line number Diff line
@@ -71,6 +71,14 @@ my_bool mi_dynmap_file(MI_INFO *info, my_off_t size) 
    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 :

storage/myisam/mi_extra.c

+6 −1
Original line number Diff line number Diff line
@@ -349,7 +349,12 @@ int mi_extra(MI_INFO *info, enum ha_extra_function function, void *extra_arg) 
  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))

      {

storage/myisam/mi_open.c

+16 −0
Original line number Diff line number Diff line
@@ -506,6 +506,22 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags) 
      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);