bzr commit into mysql-6.0-backup branch (thavamuni.alagu:2894) Bug#47939

[thavamuni.alagu]

#At file:///home/thava/repo/backup/ based on revid:thavamuni.alagu@stripped

 2894 Thava Alagu	2009-11-18
      Bug#47939 : MyISAM backup driver can wait in unlock() too long
      
      As per the protocol, the backup driver should return from 
      unlock() as early as possible. However, the killing of  
      locking thread by MyISAM driver from within unlock() can
      take too long time.
      
      This is fixed by returning from unlock() immediately 
      after remembering to kill locking thread later.
      The locking thread is killed from the very next get_data()
      call at the beginning of "final-transfer" phase.

    modified:
      storage/myisam/myisam_backup_engine.cc
=== modified file 'storage/myisam/myisam_backup_engine.cc'
--- a/storage/myisam/myisam_backup_engine.cc	2009-10-12 09:08:34 +0000
+++ b/storage/myisam/myisam_backup_engine.cc	2009-11-18 12:08:15 +0000
@@ -273,6 +273,7 @@ private:
   /** After copying bytes_between_sleeps we sleep sleep_time */
   ulong sleep_time;
   size_t bytes_since_last_sleep; ///< how many bytes sent since we last slept
+  bool should_kill_locking_thread; ///< Remember to kill locking thread asap.
 };
 
 /* Needed for VisualAge 6.0 */
@@ -392,7 +393,8 @@ result_t Engine::get_backup(const uint32
 Backup::Backup(const Table_list &tables):
   Backup_driver(tables), state(ERROR), image(NULL), stream(1),
   hash_of_tables(NULL), lock_state(LOCK_NOT_STARTED), lock_thd(NULL),
-  cannot_delete_lock_thd(FALSE), bytes_since_last_sleep(0)
+  cannot_delete_lock_thd(FALSE), bytes_since_last_sleep(0),
+  should_kill_locking_thread(FALSE)
 {
   /*
     Driver is not ready at this point, so state is ERROR.
@@ -670,6 +672,19 @@ result_t Backup::get_data(Buffer &buf)
   DBUG_ENTER("myisam::backup::Backup::get_data");
   DBUG_PRINT("enter",("stream %d",stream));
 
+  /**
+    If we had marked to kill the locking thread in the last unlock() call,
+    do it now. This will unlock table locks owned by the locking thread.
+    Note that this killing of locking thread happens only once at the
+    beginning of "Final-transfer" phase.
+  */
+  if (should_kill_locking_thread)
+  {
+    DBUG_PRINT("myisam_backup",("Killing MyISAM backup locking thread ..."));
+    kill_locking_thread();
+    should_kill_locking_thread= FALSE;
+  }
+
   /* we are currently on stream 'stream' */
   buf.table_num= stream;
 
@@ -946,8 +961,13 @@ result_t Backup::lock()
 result_t Backup::unlock()
 {
   DBUG_ENTER("myisam_backup::Backup::unlock");
-  /* kill the locking thread which owns table locks, it will unlock them */
-  kill_locking_thread();
+  /**
+    As per the protocol, unlock() should be as quick as possible.
+    We remember to kill the locking thread later.
+    We will kill the locking thread during the very next get_data() call.
+  */
+  should_kill_locking_thread= TRUE;
+ 
   DBUG_RETURN(backup::OK);
 }
 


Attachment: [text/bzr-bundle] bzr/[e-mail]