Bug #42895 Implement a locking scheme for RESTORE
Submitted: 16 Feb 2009 17:14 Modified: 25 Feb 2010 0:27
Reporter: Ingo Strüwing Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Locking Severity:S1 (Critical)
Version:6.0 OS:Any
Assigned to: Ingo Strüwing CPU Architecture:Any
Tags: mdl

[16 Feb 2009 17:14] Ingo Strüwing
Description:
Please implement a table open interface for name-locked tables.

The RESTORE statement works as follows:

- Drop and re-create all tables to restore.
- Open and lock all tables to restore.
- Restore table data.
- Unlock and close all tables to restore.

We need to prevent that another thread

- drops
- creates
- opens
- locks
- modifies

a table to restore from start of RESTORE until its end.
Note that the tables to restore do not need to exist before restore.

What we have at the moment is the lock_table_names() interface.
It can lock tables by their name, regardless if the table exists or not.
It could be used to hold the locks for the whole restore process.
However, at the moment it is not possible to open_and_lock_tables() that
have been locked by lock_table_names(). At least not after they have been
dropped and re-created.

Please implement a table open interface that can open tables under the
above circumstances.

< off-topic >
Please allow me a personal, subjective note. I am pretty unhappy that
I have to write such a stupid feature request. All relevant developers
were involved in the discussions and know exactly, what to do. This
report is required tp please Sun Microsystem bureaucracy.
< /off-topic >

How to repeat:
I don't know, how to repeat a feature request.
[23 Feb 2009 13:21] Rafal Somla
Here is a list of bugs which should be fixed if we have the facility and implement correct locking during restore operation.

Bug #40944 Backup: crash after myisampack
Bug #41716 Backup Error when sending data (for table #1) to Default/Snapshot restore driver
Bug #42519 Maria: RESTORE leads to corrupted table and assertion

Plus QA observes frequent RESTORE failures when the statement is run under server load, which are most probably caused by insufficient locking.
[23 Feb 2009 13:53] Jørgen Løland
BUG#42546 also relies on this
[9 Jun 2009 14:47] Ingo Strüwing
It has been suggested to do one of the following:

1. Extend lock table statement
   Most reliable solution.
   Fairly complex solution.  Maybe 2 weeks.

2. Implement metadata locks on database
   Could also be ok solution.  Medium time to implement.

3. Same as 2 but use BML, i.e. we use the BML using restore.
   Need to ensure there are no deadlocks.
   Easiest one.
   Will not risk other code much.

Due to the little effort, the first attempt should be to use 3.

But RESTORE does already use the backup meta data lock (BML). We do not
have problems with DDLs (it seems).

But there are three bug reports around DMLs that interfere with RESTORE.
I have added a detailed analyze to two of the bug reports, one did
already contain a detailed analyze.

Bug#40944: close_cached_tables() does not close all instances.

For me it looks like a server bug. A thread that reached wait_for_lock()
(thr_lock.c) before close_cached_tables(), does not close and re-open
the table, but works on the old TABLE. Possibly close_cached_tables() is
not the right thing to do under a write-locked table, but after all this
is what mysql_admin_table() does for REPAIR and OPTIMIZE too.

I have not yet tried, if REPAIR and OPTIMIZE fail to refresh all table
instances too.

Bug#41716: INSERT between CREATE TABLE and open_and_lock_tables().

This could be seen as a RESTORE bug. We could TRUNCATE the tables after
locking them. But since this is implemented by DROP+CREATE, which means
a repetition of what has just be done by RESTORE, I suspect that it
suffers from the same risk.

I have no yet tried if TRUNCATE under LOCK TABLES can be undermined by
INSERT.

Bug#42546: open_table(non-existent table) creates short-time MDL,
           which conflicts with lock attempt of CREATE TABLE.

For me it looks like a server bug. It is a race condition in the use of
meta data locks (MDL)s. It does not have to do with RESTORE at all.

However, it is not obvious, how to fix it. It is obvious that it is not
correct by mysql_create_table() to assume that an existent MDL for a
table name implies an existent table. But it must not check for the
existence of the table before it has a MDL on it. Otherwise the result
can become obsolete as soon as it is returned to mysql_create_table().
And it would be pretty inconvenient, if mysql_create_table() waits for
the exclusive MDL if the table exists and long-running queries are going
on on it - just to find out that the table exists already. Acquiring a
shared MDL first and upgrade later might ease the problem, but a long
running operation like OPTIMIZE would still block CREATE unnecessarily.
Still it might be an acceptable compromise since this would not block on
DML MDLs like the one in open_table().

Anyway, all three problems (and perhaps even more) could be worked
around by a lock that spans all the gaps between (DROP-CREATE-open-lock)
and blocks other threads before they open the tables.

The MDL taken in open_table() seems to be exactly, what we need to block
opens. But then we would need an exclusive MDL that survives DROP and
CREATE and can be used with open_table() in the own thread.
[10 Jun 2009 19:44] Konstantin Osipov
Ingo,
the solution to all these bugs is prior to execution of data restore to execute LOCK TABLES <table list to restore> WRITE via the Ed_connection interface.
[10 Jun 2009 19:57] Ingo Strüwing
I may miss something.

If "prior to execution of data restore" means after DROP and CREATE, I fail to see, how it could help. At that point in time it would be too late. All mentioned problems happen before locking the tables. How could changing open_and_lock_tables() to Ed_connection("LOCK TABLES ...") change this?
[10 Jun 2009 23:09] Konstantin Osipov
One on-the-surface difference is that LOCK TABLES takes WRITE locks that are not downgraded, even by InnoDB. So LOCK TABLES locks will exclude all writers, even in transactional engines.

The second reason is that LOCK TABLES locks will not go away after someone calls close_thread_tables(). Which might be the case during restore, AFAIU.

Maybe I'm missing something too, and we need to discuss on IRC.
I'm available tomorrow all day.
[10 Jun 2009 23:12] Konstantin Osipov
I read some of your analysis more carefully. You should not use close_cached_tables() inside backup code.
[15 Jun 2009 10:08] Ingo Strüwing
In a discussion with Konstantin, we decided to try to do as much of the locking through the Ed_connection interface. We will do the following steps:

1) take BML
2) restore DDL (DROP/CREATE)
3) LOCK TABLES <> WRITE;
4) FLUSH TABLES
5) TRUNCATE tables
6) restore data
7) FLUSH TABLES
8) UNLOCK TABLES

Item 7 in co-work with 3 should solve Bug#40944.
Item 5 should solve Bug#41716.
Items 3 and 8 should solve a not-yet detected bug with InnoDB, which can now no longer downgrade lock requests.

Bug#42546 is a pure server bug and will be taken by the runtime team.
[16 Jun 2009 19:21] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/76411

2822 Ingo Struewing	2009-06-16
      Bug#42895 - Implement a locking scheme for RESTORE
      WL#4844 - Implement a locking scheme for RESTORE
      
      Not to be pushed. This patch shows a way, how to do
      locking for RESTORE. It is incomplete as
      Bug 20667 (Truncate table fails for a write locked table)
      forces this patch to use a workaround. But this does not
      work for tables created with an AUTO_INCREMENT option not
      equal to one. Test case backup_engines.backup_triggers
      starts to fail on this problem.
      
      Otherwise RESTORE locking does now work as:
      1) take BML
      2) restore DDL (DROP/CREATE)
      3) LOCK TABLES <> WRITE
      4) TRUNCATE tables
      5) restore data
      6) FLUSH TABLES
      7) UNLOCK TABLES
      
      This does implicitly fix:
      Bug#40944 (Backup: crash after myisampack) and
      Bug#41716 (Backup Error when sending data (for table #1)
                to Default/Snapshot restore driver)
     @ mysql-test/suite/backup/r/backup_myisam_sync.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test result.
     @ mysql-test/suite/backup/r/backup_restore_locking.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test result.
     @ mysql-test/suite/backup/t/backup_myisam_sync.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test.
     @ mysql-test/suite/backup/t/backup_restore_locking.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test case.
     @ sql/backup/kernel.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added calls to obs_acquire_table_locks(),
        obs_release_table_locks(), obs_flush_tables(), and
        obs_truncate_tables() to
        Backup_restore_ctx::lock_tables_for_restore() and
        Backup_restore_ctx::unlock_tables().
     @ sql/bml.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added DBUG.
     @ sql/mdl.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added DBUG.
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Fixed function comment for run_service_interface_sql().
        Added definitions for obs_acquire_table_locks(),
        obs_release_table_locks(), obs_flush_tables(), and
        obs_truncate_tables().
     @ sql/si_objects.h
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added declarations for obs_acquire_table_locks(),
        obs_release_table_locks(), obs_flush_tables(), and
        obs_truncate_tables().
     @ sql/sql_base.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Returning result from THD::locked_tables_list.reopen_tables()
        in close_cached_tables().
        Added DBUG.
     @ sql/sql_prepare.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added DBUG.
     @ sql/sql_table.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added DBUG.
[17 Jun 2009 15:13] Ingo Strüwing
The final solution of this problem does now depend on Bug#20667 (Truncate table fails for a write locked table).
[19 Jun 2009 14:02] Rafal Somla
Set to "in progress" as the final patch is yet to be created.
[24 Jul 2009 0:19] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/79209

2849 Ingo Struewing	2009-07-23
      Bug#42895 - Implement a locking scheme for RESTORE
      WL#4844 - Implement a locking scheme for RESTORE
      
      Not to be pushed. This patch shows a way, how to do
      locking for RESTORE. However, it does not retain the
      AUTO_INCREMENT value for all engines. In particular
      InnoDB tables get their AUTO_INCREMENT value reset.
      
      This patch includes a temporarily disabled
      SHOW CREATE TABLE in backup_engines.backup_triggers
      so that the problem is temporarily hidden.
      
      RESTORE locking does now work as:
      1) take BML
      2) restore DDL (DROP/CREATE)
      3) LOCK TABLES <> WRITE
      4) TRUNCATE tables
      5) restore data
      6) FLUSH TABLES
      7) UNLOCK TABLES
      
      This does implicitly fix:
      Bug#40944 (Backup: crash after myisampack) and
      Bug#41716 (Backup Error when sending data (for table #1)
              to Default/Snapshot restore driver)
     @ mysql-test/suite/backup/r/backup_myisam_sync.result
        WL#4844 - Implement a locking scheme for RESTORE
        Added test result.
     @ mysql-test/suite/backup/r/backup_restore_locking.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test result.
     @ mysql-test/suite/backup/t/backup_myisam_sync.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test.
     @ mysql-test/suite/backup/t/backup_restore_locking.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test case.
     @ mysql-test/suite/backup_engines/r/backup_triggers.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result.
     @ mysql-test/suite/backup_engines/t/backup_triggers.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Temporarily disabled a SHOW CREATE TABLE due to inconsistent
        AUTO_INCREMENT handling between engines.
     @ sql/backup/kernel.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added calls to lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables() to
        Backup_restore_ctx::lock_tables_for_restore() and
        Backup_restore_ctx::unlock_tables().
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Fixed function comment for run_service_interface_sql().
        Added definitions for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/si_objects.h
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added declarations for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/sql_base.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Returning result from THD::locked_tables_list.reopen_tables()
        in close_cached_tables().
[28 Oct 2009 8:43] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/88458

2882 Ingo Struewing	2009-10-28
      Bug#42895 - Implement a locking scheme for RESTORE
      WL#4844 - Implement a locking scheme for RESTORE
      
      The old RESTORE locking scheme did allow other
      sessions to modify the tables in restore in some ways.
      
      RESTORE locking does now work as:
      1) take BML
      2) restore DDL (DROP/CREATE)
      3) LOCK TABLES <> WRITE
      4) TRUNCATE tables
      5) restore data
      6) FLUSH TABLES
      7) UNLOCK TABLES
      
      This does implicitly fix:
      Bug#40944 (Backup: crash after myisampack) and
      Bug#41716 (Backup Error when sending data (for table #1)
            to Default/Snapshot restore driver)
     @ mysql-test/suite/backup/r/backup_external.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_intr_errors.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_coverage.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_sync.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test result.
     @ mysql-test/suite/backup/r/backup_restore_locking.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test result.
     @ mysql-test/suite/backup/t/backup_external.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_coverage.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_sync.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test.
     @ mysql-test/suite/backup/t/backup_restore_locking.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test case.
     @ mysql-test/suite/backup_engines/r/backup_interruption.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ sql/backup/kernel.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added calls to lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables() to
        Backup_restore_ctx::lock_tables_for_restore() and
        Backup_restore_ctx::unlock_tables().
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Fixed function comment for run_service_interface_sql().
        Added definitions for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/si_objects.h
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added declarations for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/sql_base.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Returning result from THD::locked_tables_list.reopen_tables()
        in close_cached_tables().
[28 Oct 2009 12:19] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/88476

2882 Ingo Struewing	2009-10-28
      Bug#42895 - Implement a locking scheme for RESTORE
      WL#4844 - Implement a locking scheme for RESTORE
      
      The old RESTORE locking scheme did allow other
      sessions to modify the tables in restore in some ways.
      
      RESTORE locking does now work as:
      1) take BML
      2) restore DDL (DROP/CREATE)
      3) LOCK TABLES <> WRITE
      4) TRUNCATE tables
      5) restore data
      6) FLUSH TABLES
      7) UNLOCK TABLES
      
      This does implicitly fix:
      Bug#40944 (Backup: crash after myisampack) and
      Bug#41716 (Backup Error when sending data (for table #1)
            to Default/Snapshot restore driver)
     @ mysql-test/suite/backup/r/backup_external.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_intr_errors.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_coverage.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_sync.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test result.
     @ mysql-test/suite/backup/r/backup_restore_locking.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test result.
     @ mysql-test/suite/backup/t/backup_external.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_coverage.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_sync.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test.
     @ mysql-test/suite/backup/t/backup_restore_locking.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test case.
     @ mysql-test/suite/backup_engines/r/backup_interruption.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ sql/backup/kernel.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added calls to lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables() to
        Backup_restore_ctx::lock_tables_for_restore() and
        Backup_restore_ctx::unlock_tables().
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Fixed function comment for run_service_interface_sql().
        Added definitions for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/si_objects.h
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added declarations for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/sql_base.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Returning result from THD::locked_tables_list.reopen_tables()
        in close_cached_tables().
[16 Nov 2009 10:03] Rafal Somla
Moving to "in-progress" as Ingo is considering adding warning reporting for the new code.
[16 Nov 2009 15:27] Rafal Somla
Conditionally approved - see my review.
[16 Nov 2009 16:05] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/90541

2882 Ingo Struewing	2009-11-16
      Bug#42895 - Implement a locking scheme for RESTORE
      WL#4844 - Implement a locking scheme for RESTORE
      
      The old RESTORE locking scheme did allow other
      sessions to modify the tables in restore in some ways.
      
      RESTORE locking does now work as:
      1) take BML
      2) restore DDL (DROP/CREATE)
      3) LOCK TABLES <> WRITE
      4) TRUNCATE tables
      5) restore data
      6) FLUSH TABLES
      7) UNLOCK TABLES
      
      This does implicitly fix:
      Bug#40944 (Backup: crash after myisampack) and
      Bug#41716 (Backup Error when sending data (for table #1)
            to Default/Snapshot restore driver)
     @ mysql-test/suite/backup/r/backup_external.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_intr_errors.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_coverage.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_sync.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test result.
     @ mysql-test/suite/backup/r/backup_restore_locking.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test result.
     @ mysql-test/suite/backup/t/backup_external.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_coverage.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_sync.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test.
     @ mysql-test/suite/backup/t/backup_restore_locking.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test case.
     @ mysql-test/suite/backup_engines/r/backup_interruption.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ sql/backup/kernel.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added calls to lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables() to
        Backup_restore_ctx::lock_tables_for_restore() and
        Backup_restore_ctx::unlock_tables().
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Fixed function comment for run_service_interface_sql().
        Added definitions for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/si_objects.h
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added declarations for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/sql_base.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Returning result from THD::locked_tables_list.reopen_tables()
        in close_cached_tables().
     @ sql/sql_delete.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added error injection point "error_inject_truncate_enter".
[16 Nov 2009 19:21] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/90567

2882 Ingo Struewing	2009-11-16
      Bug#42895 - Implement a locking scheme for RESTORE
      WL#4844 - Implement a locking scheme for RESTORE
      
      The old RESTORE locking scheme did allow other
      sessions to modify the tables in restore in some ways.
      
      RESTORE locking does now work as:
      1) take BML
      2) restore DDL (DROP/CREATE)
      3) LOCK TABLES <> WRITE
      4) TRUNCATE tables
      5) restore data
      6) FLUSH TABLES
      7) UNLOCK TABLES
      
      This does implicitly fix:
      Bug#40944 (Backup: crash after myisampack) and
      Bug#41716 (Backup Error when sending data (for table #1)
            to Default/Snapshot restore driver)
      
      ******
      auto-merge
     @ mysql-test/suite/backup/r/backup_external.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_intr_errors.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_coverage.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ mysql-test/suite/backup/r/backup_myisam_sync.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test result.
     @ mysql-test/suite/backup/r/backup_restore_locking.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test result.
     @ mysql-test/suite/backup/t/backup_external.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_coverage.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added comments.
     @ mysql-test/suite/backup/t/backup_myisam_sync.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added test.
     @ mysql-test/suite/backup/t/backup_restore_locking.test
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        New test case.
     @ mysql-test/suite/backup_engines/r/backup_interruption.result
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Updated test result due to unrelated changes.
     @ sql/backup/kernel.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added calls to lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables() to
        Backup_restore_ctx::lock_tables_for_restore() and
        Backup_restore_ctx::unlock_tables().
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Fixed function comment for run_service_interface_sql().
        Added definitions for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/si_objects.h
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added declarations for lock_tables_for_write(),
        unlock_tables(), flush_tables(), and
        truncate_tables().
     @ sql/sql_base.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Returning result from THD::locked_tables_list.reopen_tables()
        in close_cached_tables().
     @ sql/sql_delete.cc
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        Added error injection point "error_inject_truncate_enter".
[17 Nov 2009 16:20] Ingo Strüwing
Queued to mysql-6.0-backup.
[27 Nov 2009 0:07] Alexey Stroganov
I would like to highlight performance aspects of this change. I've run RESTORE for the latest mysql-6.0-backup tree (rev.2894) that includes patch for this issue and found notable performance drop up to 5-10 times. It turned out that most of the time server spent in executing of following statements one by one - TRUNCATE TABLE and FLUSH TABLE. I don't see how we can improve TRUNCATE TABLE but for FLUSH TABLE I've replaced loop with similar one for LOCK TABLE that significantly helped to improve situation.

 RESTORE of backup image with 15.000 tables + 15.000 SPs
----------------------------------------------------------
                                           MyISAM| InnoDB|
----------------------------------------------------------
RESTORE old way                          |   318 |    43 |
----------------------------------------------------------
RESTORE new way                          |  1663 |   649 |
----------------------------------------------------------
RESTORE new way+fix for flush table      |  1133 |   311 |
----------------------------------------------------------
RESTORE mysqldump file                   |   65  |    36 |
----------------------------------------------------------
numbers - elapsed time in seconds

fix for flush table statement
=== modified file 'sql/si_objects.cc'
--- sql/si_objects.cc   2009-11-24 16:47:23 +0000
+++ sql/si_objects.cc   2009-11-26 23:58:34 +0000
@@ -3514,19 +3514,17 @@
   bool error_status= FALSE;
   DBUG_ENTER("obs:flush_tables");

-  for (tlist= table_list; tlist; tlist= tlist->next_global)
+  if (table_list)
   {
-    s_stream.reset();
-    s_stream << "FLUSH TABLE ";
-    s_stream << "`" << tlist->db << "`.`" << tlist->table_name << "`";
-    if (run_service_interface_sql(thd, &ed_connection,
-                                  s_stream.lex_string(), TRUE))
+    s_stream << "FLUSH TABLE  ";
+    for (tlist= table_list; tlist; tlist= tlist->next_global)
     {
-      error_status= TRUE;
-      DBUG_PRINT("error", ("execute direct: '%s'  errno: %d",
-                           s_stream.lex_string()->str,
-                           ed_connection.get_last_errno()));
+      if (tlist != table_list)
+        s_stream << ", ";
+      s_stream << "`" << tlist->db << "`.`" << tlist->table_name << "`";
     }
+    error_status= run_service_interface_sql(thd, &ed_connection,
+                                            s_stream.lex_string(), TRUE);
   }
[27 Nov 2009 8:49] Alexey Stroganov
Changed back to In-Progress to address performance aspects.
[2 Dec 2009 10:42] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/92425

2900 Ingo Struewing	2009-12-02
      Bug#42895 - Implement a locking scheme for RESTORE (performance degression)
      
      The new locking scheme slows RESTORE down.
      
      Alexey Stroganov provided this fix, to combine multiple tables
      into a single FLUSH TABLE statement, instead of one statement per
      table.
      
      Note that this relieves part of the degression only.
     @ mysql-test/suite/backup/r/backup_intr_errors.result
        Bug#42895 - Implement a locking scheme for RESTORE (performance degression)
        One flush less - one warning less.
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE (performance degression)
        Change a bunch of FLUSH statements into a single one.
[4 Dec 2009 6:58] Rafal Somla
Good to push.
[4 Dec 2009 17:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/92920

2900 Ingo Struewing	2009-12-04
      Bug#42895 - Implement a locking scheme for RESTORE (performance regression)
      
      The new locking scheme slows RESTORE down.
      
      Alexey Stroganov provided this fix, to combine multiple tables
      into a single FLUSH TABLE statement, instead of one statement per
      table.
      
      Note that this relieves part of the regression only.
     @ mysql-test/suite/backup/r/backup_intr_errors.result
        Bug#42895 - Implement a locking scheme for RESTORE (performance regression)
        One flush less - one warning less.
     @ sql/si_objects.cc
        Bug#42895 - Implement a locking scheme for RESTORE (performance regression)
        Change a bunch of FLUSH statements into a single one.
[4 Dec 2009 17:45] Ingo Strüwing
Queued to mysql-6.0-backup.
[8 Jan 2010 12:34] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/96379

3040 Ingo Struewing	2010-01-08
      WL#5101 - MySQL Backup back port
      Merged revid:ingo.struewing@sun.com-20091116192034-edi1mvf8in5w4f1w
        Bug#42895 - Implement a locking scheme for RESTORE
        WL#4844 - Implement a locking scheme for RESTORE
        
        The old RESTORE locking scheme did allow other
        sessions to modify the tables in restore in some ways.
        
        RESTORE locking does now work as:
        1) take BML
        2) restore DDL (DROP/CREATE)
        3) LOCK TABLES <> WRITE
        4) TRUNCATE tables
        5) restore data
        6) FLUSH TABLES
        7) UNLOCK TABLES
        
        This does implicitly fix:
        Bug#40944 (Backup: crash after myisampack) and
        Bug#41716 (Backup Error when sending data (for table #1)
              to Default/Snapshot restore driver)
     @ sql/sql_base.cc
        WL#5101 - MySQL Backup back port
            Bug#42895 - Implement a locking scheme for RESTORE
            WL#4844 - Implement a locking scheme for RESTORE
            Returning result from THD::locked_tables_list.reopen_tables()
            in close_cached_tables().
     @ sql/sql_delete.cc
        WL#5101 - MySQL Backup back port
            Bug#42895 - Implement a locking scheme for RESTORE
            WL#4844 - Implement a locking scheme for RESTORE
            Added error injection point "error_inject_truncate_enter".
[20 Feb 2010 9:19] Bugs System
Pushed into 6.0.14-alpha (revid:ingo.struewing@sun.com-20100218152520-s4v1ld76bif06eqn) (version source revid:ingo.struewing@sun.com-20100119103538-wtp5alpz4p2jayl5) (merge vers: 6.0.14-alpha) (pib:16)
[25 Feb 2010 0:27] Paul DuBois
Noted in 6.0.14 changelog.

RESTORE locking did not always prevent other sessions from modifying
tables.