bzr commit into MySQL/Maria:mysql-maria branch (guilhem:2731)

[Guilhem Bichot]

#At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-maria/ based on revid:guilhem@stripped

 2731 Guilhem Bichot	2009-02-09
      Callers of translog_deassign_id_from_share() need intern_lock.
      Assert that keys don't point to bitmap pages.
      modified:
        storage/maria/ma_blockrec.h
        storage/maria/ma_locking.c

per-file messages:
  storage/maria/ma_blockrec.h
    assertion
  storage/maria/ma_locking.c
    With concurrent INSERTs, it is possible that two threads enter _ma_mark_file_changed()
    at the same time, so they should serialize their access to the "changed" state/share
    members; another reason is that this function may call _ma_update_state_lsns_sub()
    which may call translog_deassign_id_from_share() (I saw it during testing of
    online backup) which requires the intern_lock mutex.
    As INSERTs only change from "not changed" to "changed", we can first check without mutex:
    if it says "changed", some other thread has set or is setting the variables now,
    we don't need to do it; if it says "not changed", we serialize and re-check.
=== modified file 'storage/maria/ma_blockrec.h'
--- a/storage/maria/ma_blockrec.h	2009-01-15 15:14:47 +0000
+++ b/storage/maria/ma_blockrec.h	2009-02-09 21:52:42 +0000
@@ -122,6 +122,7 @@ static inline MARIA_RECORD_POS ma_record
                                             uint dir_entry)
 {
   DBUG_ASSERT(dir_entry <= 255);
+  DBUG_ASSERT(page > 0); /* page 0 is bitmap, not data page */
   return (MARIA_RECORD_POS) (((ulonglong) page << 8) | dir_entry);
 }
 

=== modified file 'storage/maria/ma_locking.c'
--- a/storage/maria/ma_locking.c	2009-01-08 08:20:04 +0000
+++ b/storage/maria/ma_locking.c	2009-02-09 21:52:42 +0000
@@ -391,9 +391,15 @@ int _ma_mark_file_changed(MARIA_HA *info
 {
   uchar buff[3];
   register MARIA_SHARE *share= info->s;
+  int error= 1;
   DBUG_ENTER("_ma_mark_file_changed");
 
-  if (!(share->state.changed & STATE_CHANGED) || ! share->global_changed)
+#define _MA_ALREADY_MARKED_FILE_CHANGED                                 \
+  ((share->state.changed & STATE_CHANGED) && share->global_changed)
+  if (_MA_ALREADY_MARKED_FILE_CHANGED)
+    DBUG_RETURN(0);
+  pthread_mutex_lock(&share->intern_lock); /* recheck under mutex */
+  if (! _MA_ALREADY_MARKED_FILE_CHANGED)
   {
     share->state.changed|=(STATE_CHANGED | STATE_NOT_ANALYZED |
 			   STATE_NOT_OPTIMIZED_KEYS);
@@ -420,7 +426,7 @@ int _ma_mark_file_changed(MARIA_HA *info
                     sizeof(share->state.header) +
                     MARIA_FILE_OPEN_COUNT_OFFSET,
                     MYF(MY_NABP)))
-        DBUG_RETURN(1);
+        goto err;
     }
     /* Set uuid of file if not yet set (zerofilled file) */
     if (share->base.born_transactional &&
@@ -432,11 +438,15 @@ int _ma_mark_file_changed(MARIA_HA *info
            _ma_update_state_lsns_sub(share, LSN_IMPOSSIBLE,
                                      trnman_get_min_trid(),
                                      TRUE, TRUE)))
-        DBUG_RETURN(1);
+        goto err;
       share->state.changed|= STATE_NOT_MOVABLE;
     }
   }
-  DBUG_RETURN(0);
+  error= 0;
+err:
+  pthread_mutex_unlock(&share->intern_lock);
+  DBUG_RETURN(error);
+#undef _MA_ALREADY_MARKED_FILE_CHANGED
 }
 
 /*