List:Maria Storage Engine« Previous MessageNext Message »
From:Guilhem Bichot Date:February 9 2009 9:02pm
Subject:bzr commit into MySQL/Maria:mysql-maria branch (guilhem:2730)
View as plain text  
#At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-maria/ based on revid:guilhem@stripped

 2730 Guilhem Bichot	2009-02-09
      Comments. Take bitmap mutex lock when changing bitmap.changed.
      modified:
        storage/maria/ha_maria.cc
        storage/maria/ma_checkpoint.c
        storage/maria/ma_extra.c
        storage/maria/ma_pagecache.c
        storage/maria/ma_recovery.c

per-file messages:
  storage/maria/ha_maria.cc
    comment
  storage/maria/ma_checkpoint.c
    comment
  storage/maria/ma_extra.c
    use bitmap mutex when changing bitmap.changed, sounds safer
  storage/maria/ma_pagecache.c
    comment
  storage/maria/ma_recovery.c
    comments
=== modified file 'storage/maria/ha_maria.cc'
--- a/storage/maria/ha_maria.cc	2009-01-16 16:18:17 +0000
+++ b/storage/maria/ha_maria.cc	2009-02-09 21:02:04 +0000
@@ -2381,7 +2381,8 @@ int ha_maria::external_lock(THD *thd, in
         Note that we can come here without having an exclusive lock on the
         table, for example in this case:
         external_lock(F_(WR|RD)LCK); thr_lock() which fails due to lock
-        abortion; external_lock(F_UNLCK).
+        abortion; external_lock(F_UNLCK). Fortunately, the re-enabling happens
+        only if we were the thread which disabled logging.
       */
       if (_ma_reenable_logging_for_table(file, TRUE))
         DBUG_RETURN(1);

=== modified file 'storage/maria/ma_checkpoint.c'
--- a/storage/maria/ma_checkpoint.c	2009-01-08 08:20:04 +0000
+++ b/storage/maria/ma_checkpoint.c	2009-02-09 21:02:04 +0000
@@ -653,6 +653,14 @@ pthread_handler_t ma_checkpoint_backgrou
             We use FLUSH_KEEP_LAZY: if a file is already in flush, it's
             smarter to move to the next file than wait for this one to be
             completely flushed, which may take long.
+            StaleFilePointersInFlush: notice how below we use "dfile" which
+            is an OS file descriptor plus some function and MARIA_SHARE
+            pointers; this data dates from a previous checkpoint; since then,
+            the table may have been closed (so MARIA_SHARE* became stale), and
+            the file descriptor reassigned to another table which does not
+            have the same CRC-read-set callbacks: it is thus important that
+            flush_pagecache_blocks_with_filter() does not use the pointers,
+            only the OS file descriptor.
           */
           int res=
             flush_pagecache_blocks_with_filter(maria_pagecache,

=== modified file 'storage/maria/ma_extra.c'
--- a/storage/maria/ma_extra.c	2009-01-08 08:20:04 +0000
+++ b/storage/maria/ma_extra.c	2009-02-09 21:02:04 +0000
@@ -592,7 +592,11 @@ int _ma_flush_table_files(MARIA_HA *info
           error= 1;
       }
       else
-        info->s->bitmap.changed= 0;
+      {
+        pthread_mutex_lock(&share->bitmap.bitmap_lock);
+        share->bitmap.changed= 0;
+        pthread_mutex_unlock(&share->bitmap.bitmap_lock);
+      }
       if (flush_pagecache_blocks(share->pagecache, &info->dfile,
                                  flush_type_for_data))
         error= 1;

=== modified file 'storage/maria/ma_pagecache.c'
--- a/storage/maria/ma_pagecache.c	2008-12-27 02:05:16 +0000
+++ b/storage/maria/ma_pagecache.c	2009-02-09 21:02:04 +0000
@@ -4227,11 +4227,11 @@ static int flush_cached_blocks(PAGECACHE
        @todo IO If page is contiguous with next page to flush, group flushes
        in one single my_pwrite().
     */
-    /*
+    /**
       It is important to use block->hash_link->file below and not 'file', as
-      the first one is right and the second may have different content (and
-      this matters for callbacks, bitmap pages and data pages have different
-      ones).
+      the first one is right and the second may have different out-of-date
+      content (see StaleFilePointersInFlush in ma_checkpoint.c).
+      @todo change argument of functions to be File.
     */
     error= pagecache_fwrite(pagecache, &block->hash_link->file,
                             block->buffer,

=== modified file 'storage/maria/ma_recovery.c'
--- a/storage/maria/ma_recovery.c	2009-02-05 22:38:30 +0000
+++ b/storage/maria/ma_recovery.c	2009-02-09 21:02:04 +0000
@@ -3326,7 +3326,10 @@ void _ma_tmp_disable_logging_for_table(M
 /**
    Re-enables logging for a table which had it temporarily disabled.
 
-   Only the thread which disabled logging is allowed to reenable it.
+   Only the thread which disabled logging is allowed to reenable it. Indeed,
+   re-enabling logging affects all open instances, one must have exclusive
+   access to the table to do that. In practice, the one which disables has
+   such access.
 
    @param  info            table
    @param  flush_pages     if function needs to flush pages first

Thread
bzr commit into MySQL/Maria:mysql-maria branch (guilhem:2730) Guilhem Bichot10 Feb