[Commits] bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (monty:2965)

Michael Widenius monty at askmonty.org
Wed Nov 3 14:14:10 EET 2010


#At lp:maria based on revid:monty at askmonty.org-20101102152257-mwa7etvs9nxewjf2

 2965 Michael Widenius	2010-11-03
      Fixed compiler & valgrind warnings from my previous push.
      Fixed a bug in Aria when two threads was inserting into the same table and row page and one thread did an abort becasue of duplicate key.
      modified:
        mysys/thr_lock.c
        sql/sql_base.cc
        storage/maria/ma_bitmap.c
        storage/maria/ma_blockrec.c
        storage/maria/ma_blockrec.h
        storage/maria/maria_def.h
        storage/myisam/mi_locking.c

per-file messages:
  mysys/thr_lock.c
    Fixed valgrind warning
  sql/sql_base.cc
    Remove not used variable
  storage/maria/ma_bitmap.c
    Added ma_bitmap_lock() & ma_bitmap_unlock() to protect against two threads using the bitmap at the same time.
    More DBUG_PRINT()
  storage/maria/ma_blockrec.c
    Fixed a bug in Aria when two threads was inserting into the same table and row page and one thread did an abort becasue of duplicate key.
    Fix was that we block other threads to modify the bitmap while we are removing the row with a duplicate key.
  storage/maria/ma_blockrec.h
    Added ma_bitmap_lock() & ma_bitmap_unlock() to protect against two threads using the bitmap at the same time.
  storage/maria/maria_def.h
    Changed flush_all_requested to be a counter.
  storage/myisam/mi_locking.c
    Fixed compiler error on windows (typo).
=== modified file 'mysys/thr_lock.c'
--- a/mysys/thr_lock.c	2010-11-02 15:22:57 +0000
+++ b/mysys/thr_lock.c	2010-11-03 12:14:02 +0000
@@ -406,6 +406,7 @@ void thr_lock_data_init(THR_LOCK *lock,T
   data->owner= 0;                               /* no owner yet */
   data->status_param=param;
   data->cond=0;
+  data->priority= 0;
 }
 
 

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-11-02 15:22:57 +0000
+++ b/sql/sql_base.cc	2010-11-03 12:14:02 +0000
@@ -4280,7 +4280,6 @@ void detach_merge_children(TABLE *table,
 {
   TABLE_LIST *child_l;
   TABLE *parent= table->child_l ? table : table->parent;
-  bool first_detach;
   DBUG_ENTER("detach_merge_children");
   /*
     Either table->child_l or table->parent must be set. Parent must have

=== modified file 'storage/maria/ma_bitmap.c'
--- a/storage/maria/ma_bitmap.c	2010-11-02 15:22:57 +0000
+++ b/storage/maria/ma_bitmap.c	2010-11-03 12:14:02 +0000
@@ -227,7 +227,8 @@ my_bool _ma_bitmap_init(MARIA_SHARE *sha
     The +1 is to add the bitmap page, as this doesn't have to be covered
   */
   bitmap->pages_covered= aligned_bit_blocks * 16 + 1;
-  bitmap->flush_all_requested= bitmap->non_flushable= 0;
+  bitmap->flush_all_requested= 0;
+  bitmap->non_flushable= 0;
 
   /* Update size for bits */
   /* TODO; Make this dependent of the row size */
@@ -311,8 +312,8 @@ my_bool _ma_bitmap_flush(MARIA_SHARE *sh
     pthread_mutex_lock(&share->bitmap.bitmap_lock);
     if (share->bitmap.changed)
     {
-      share->bitmap.changed= 0;
       res= write_changed_bitmap(share, &share->bitmap);
+      share->bitmap.changed= 0;
     }
     pthread_mutex_unlock(&share->bitmap.bitmap_lock);
   }
@@ -355,7 +356,7 @@ my_bool _ma_bitmap_flush_all(MARIA_SHARE
   pthread_mutex_lock(&bitmap->bitmap_lock);
   if (bitmap->changed || bitmap->changed_not_flushed)
   {
-    bitmap->flush_all_requested= TRUE;
+    bitmap->flush_all_requested++;
 #ifndef WRONG_BITMAP_FLUSH
     while (bitmap->non_flushable > 0)
     {
@@ -363,6 +364,7 @@ my_bool _ma_bitmap_flush_all(MARIA_SHARE
       pthread_cond_wait(&bitmap->bitmap_cond, &bitmap->bitmap_lock);
     }
 #endif
+    DBUG_ASSERT(bitmap->flush_all_requested == 1);
     /*
       Bitmap is in a flushable state: its contents in memory are reflected by
       log records (complete REDO-UNDO groups) and all bitmap pages are
@@ -391,7 +393,7 @@ my_bool _ma_bitmap_flush_all(MARIA_SHARE
         PCFLUSH_PINNED_AND_ERROR)
       res= TRUE;
     bitmap->changed_not_flushed= FALSE;
-    bitmap->flush_all_requested= FALSE;
+    bitmap->flush_all_requested--;
     /*
       Some well-behaved threads may be waiting for flush_all_requested to
       become false, wake them up.
@@ -405,6 +407,70 @@ my_bool _ma_bitmap_flush_all(MARIA_SHARE
 
 
 /**
+   @brief Lock bitmap from being used by another thread
+
+   @fn _ma_bitmap_lock()
+   @param  share               Table's share
+
+   @notes
+   This is a temporary solution for allowing someone to delete an inserted
+   duplicate-key row while someone else is doing concurrent inserts.
+   This is ok for now as duplicate key errors are not that common.
+
+   In the future we will add locks for row-pages to ensure two threads doesn't
+   work at the same time on the same page.
+*/
+
+void _ma_bitmap_lock(MARIA_SHARE *share)
+{
+  MARIA_FILE_BITMAP *bitmap= &share->bitmap;
+  DBUG_ENTER("_ma_bitmap_lock");
+
+  if (!share->now_transactional)
+    DBUG_VOID_RETURN;
+
+  pthread_mutex_lock(&bitmap->bitmap_lock);
+  bitmap->flush_all_requested++;
+  while (bitmap->non_flushable)
+  {
+    DBUG_PRINT("info", ("waiting for bitmap to be flushable"));
+    pthread_cond_wait(&bitmap->bitmap_cond, &bitmap->bitmap_lock);
+  }
+  /*
+    Ensure that _ma_bitmap_flush_all() and _ma_bitmap_lock() are blocked.
+    ma_bitmap_flushable() is blocked thanks to 'flush_all_requested'.
+  */
+  bitmap->non_flushable= 1;
+  pthread_mutex_unlock(&bitmap->bitmap_lock);
+  DBUG_VOID_RETURN;
+}
+  
+/**
+   @brief Unlock bitmap after _ma_bitmap_lock()
+
+   @fn _ma_bitmap_unlock()
+   @param  share               Table's share
+*/
+
+void _ma_bitmap_unlock(MARIA_SHARE *share)
+{
+  MARIA_FILE_BITMAP *bitmap= &share->bitmap;
+  DBUG_ENTER("_ma_bitmap_unlock");
+
+  if (!share->now_transactional)
+    DBUG_VOID_RETURN;
+  DBUG_ASSERT(bitmap->flush_all_requested > 0 && bitmap->non_flushable == 1);
+
+  pthread_mutex_lock(&bitmap->bitmap_lock);
+  bitmap->flush_all_requested--;
+  bitmap->non_flushable= 0;
+  pthread_mutex_unlock(&bitmap->bitmap_lock);
+  pthread_cond_broadcast(&bitmap->bitmap_cond);
+  DBUG_VOID_RETURN;
+}
+
+
+/**
   @brief Unpin all pinned bitmap pages
 
   @param  share            Table's share
@@ -633,11 +699,12 @@ static void _ma_print_bitmap_changes(MAR
 {
   uchar *pos, *end, *org_pos;
   ulong page;
+  DBUG_ENTER("_ma_print_bitmap_changes");
 
   end= bitmap->map + bitmap->used_size;
   DBUG_LOCK_FILE;
-  fprintf(DBUG_FILE,"\nBitmap page changes at page %lu\n",
-          (ulong) bitmap->page);
+  fprintf(DBUG_FILE,"\nBitmap page changes at page: %lu  bitmap: 0x%lx\n",
+          (ulong) bitmap->page, (long) bitmap->map);
 
   page= (ulong) bitmap->page+1;
   for (pos= bitmap->map, org_pos= bitmap->map + bitmap->block_size ;
@@ -666,6 +733,7 @@ static void _ma_print_bitmap_changes(MAR
   fputc('\n', DBUG_FILE);
   DBUG_UNLOCK_FILE;
   memcpy(bitmap->map + bitmap->block_size, bitmap->map, bitmap->block_size);
+  DBUG_VOID_RETURN;
 }
 
 
@@ -877,6 +945,7 @@ static void fill_block(MARIA_FILE_BITMAP
 {
   uint page, offset, tmp;
   uchar *data;
+  DBUG_ENTER("fill_block");
 
   /* For each 6 bytes we have 6*8/3= 16 patterns */
   page= ((uint) (best_data - bitmap->map)) / 6 * 16 + best_pos;
@@ -902,6 +971,7 @@ static void fill_block(MARIA_FILE_BITMAP
   int2store(data, tmp);
   bitmap->changed= 1;
   DBUG_EXECUTE("bitmap", _ma_print_bitmap_changes(bitmap););
+  DBUG_VOID_RETURN;
 }
 
 
@@ -1514,6 +1584,8 @@ static void use_head(MARIA_HA *info, pgc
   MARIA_BITMAP_BLOCK *block;
   uchar *data;
   uint offset, tmp, offset_page;
+  DBUG_ENTER("use_head");
+
   DBUG_ASSERT(page % bitmap->pages_covered);
 
   block= dynamic_element(&info->bitmap_blocks, block_position,
@@ -1536,6 +1608,7 @@ static void use_head(MARIA_HA *info, pgc
   int2store(data, tmp);
   bitmap->changed= 1;
   DBUG_EXECUTE("bitmap", _ma_print_bitmap_changes(bitmap););
+  DBUG_VOID_RETURN;
 }
 
 

=== modified file 'storage/maria/ma_blockrec.c'
--- a/storage/maria/ma_blockrec.c	2010-11-02 15:22:57 +0000
+++ b/storage/maria/ma_blockrec.c	2010-11-03 12:14:02 +0000
@@ -3592,7 +3592,7 @@ my_bool _ma_write_abort_block_record(MAR
   MARIA_SHARE *share= info->s;
   DBUG_ENTER("_ma_write_abort_block_record");
 
-  _ma_bitmap_flushable(info, 1);
+  _ma_bitmap_lock(share);  /* Lock bitmap from other insert threads */
   if (delete_head_or_tail(info,
                           ma_recordpos_to_page(info->cur_row.lastpos),
                           ma_recordpos_to_dir_entry(info->cur_row.lastpos), 1,
@@ -3630,7 +3630,7 @@ my_bool _ma_write_abort_block_record(MAR
                       &lsn, (void*) 0))
       res= 1;
   }
-  _ma_bitmap_flushable(info, -1);
+  _ma_bitmap_unlock(share);
   _ma_unpin_all_pages_and_finalize_row(info, lsn);
   DBUG_RETURN(res);
 }

=== modified file 'storage/maria/ma_blockrec.h'
--- a/storage/maria/ma_blockrec.h	2010-11-02 15:22:57 +0000
+++ b/storage/maria/ma_blockrec.h	2010-11-03 12:14:02 +0000
@@ -217,6 +217,8 @@ uint _ma_bitmap_get_page_bits(MARIA_HA *
 void _ma_bitmap_delete_all(MARIA_SHARE *share);
 int  _ma_bitmap_create_first(MARIA_SHARE *share);
 void _ma_bitmap_flushable(MARIA_HA *info, int non_flushable_inc);
+void _ma_bitmap_lock(MARIA_SHARE *share);
+void _ma_bitmap_unlock(MARIA_SHARE *share);
 void _ma_bitmap_set_pagecache_callbacks(PAGECACHE_FILE *file,
                                         MARIA_SHARE *share);
 #ifndef DBUG_OFF

=== modified file 'storage/maria/maria_def.h'
--- a/storage/maria/maria_def.h	2010-09-09 23:42:12 +0000
+++ b/storage/maria/maria_def.h	2010-11-03 12:14:02 +0000
@@ -246,7 +246,7 @@ typedef struct st_maria_file_bitmap
   uint used_size;                      /* Size of bitmap head that is not 0 */
   my_bool changed;                     /* 1 if page needs to be written */
   my_bool changed_not_flushed;         /* 1 if some bitmap is not flushed */
-  my_bool flush_all_requested;         /**< If _ma_bitmap_flush_all waiting */
+  uint flush_all_requested;            /**< If _ma_bitmap_flush_all waiting */
   uint non_flushable;                  /**< 0 if bitmap and log are in sync */
   PAGECACHE_FILE file;		       /* datafile where bitmap is stored */
 

=== modified file 'storage/myisam/mi_locking.c'
--- a/storage/myisam/mi_locking.c	2010-11-02 15:22:57 +0000
+++ b/storage/myisam/mi_locking.c	2010-11-03 12:14:02 +0000
@@ -246,7 +246,7 @@ int mi_lock_database(MI_INFO *info, int
        a crash on windows if the table is renamed and 
        later on referenced by the merge table.
      */
-    if ((info->open_flags & HA_OPEN_MERGE_TABLE) && (info->s)->kfile < 0)
+    if ((info->open_flag & HA_OPEN_MERGE_TABLE) && (info->s)->kfile < 0)
     {
       error = HA_ERR_NO_SUCH_TABLE;
     }



More information about the commits mailing list