| Bug #38426 | Backup of partitioned Falcon table will not use snapshot driver | ||
|---|---|---|---|
| Submitted: | 29 Jul 2008 11:54 | Modified: | 13 Dec 2008 17:20 |
| Reporter: | Øystein Grøvlen | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: Backup | Severity: | S3 (Non-critical) |
| Version: | 6.0 | OS: | Any |
| Assigned to: | Jørgen Løland | CPU Architecture: | Any |
[29 Jul 2008 11:54]
Øystein Grøvlen
[29 Jul 2008 18:46]
Sveta Smirnova
Thank you for the report. Verified as described.
[26 Sep 2008 8:24]
Sergey Vojtovich
Reassigning to Rafal as discussed in Riga.
[22 Oct 2008 13:49]
Rafal Somla
TO BE DONE (this plan was agreed upon at Riga DevMtg): 1. Extend partitioned handler so that it can start consistent snapshot if all underlying tables support it. 2. Determine how to find out if partitioned table supports consistent read. 3. Change backup kernel logics for detecting if a table supports consistent read so that it works also for partitioned tables. Note: probably we will need opened TABLE for this. Svoj will work on 1,2. Backup team on 3. Further comments from Svoj: On 23/09/2008 10:23 Sergey Vojtovich wrote: > Hi! > > I just found out that the first item doesn't really need to be implemented. > Check sql/handler.cc - ha_start_consistent_snapshot() iterates through all > available storage engines and calls start_consistent_snapshot(). We do not > need to do the same in partitioning. That also means that all of our locking > concerns do not apply. > > Also the second item seems to be implemented, I stole the following hunk > of code from sql_show.cc: > #ifdef WITH_PARTITION_STORAGE_ENGINE > if (share->db_type() == partition_hton && > share->partition_info_len) > tmp_db_type= share->default_part_db_type; > #endif > > So we probably only need to fix get_storage_engine(), so it takes into > account partitioned tables. Though I'm not sure if that wouldn't break > native backup. > ... > > Regards, > Sergey On the backup team part, point 3, the current logic selects CS backup driver iff hton->start_consistent_snapshot is not NULL, where hton is the *handlerton* representing the storage engine of the table in question. This poses a problem, because decision is made based on the storage engine of the table, not on its handler. This is not going to work for partitioned tables, because there can be 2 different tables using partition storage engine, one which supports consistent snapshot and other which does not. There are 2 possible ways of dealing with this: A) Change the driver selection logic to base its decision not on the storage engine but on a particular table instance. Best: extend SI with a function which tells, for a given table if that table supports consistent snapshot or not. B) Implement a "native" backup driver for partitioned tables. Then the driver selection logic will select it, and the driver will recognize tables which support consistent snapshots and use this method to get backup image. It seems to me that method A could be simpler to implement.
[4 Nov 2008 10:10]
Rafal Somla
Another (simpler) approach to selecting correct backup engine for partitioned tables.
C) Modify get_storage_engine() so that if it sees a partitioned table, it returns the underlying storage engine of the partitions, not the "partition storage engine". The function can verify that all partitions use the same underlying storage engine.
Here is a preliminary patch implementing this solution:
=== modified file 'sql/backup/backup_info.cc'
--- sql/backup/backup_info.cc 2008-10-27 13:06:21 +0000
+++ sql/backup/backup_info.cc 2008-11-04 09:59:46 +0000
@@ -8,6 +8,7 @@
*/
#include "../mysql_priv.h"
+#include "../ha_partition.h"
#include "backup_info.h"
#include "backup_kernel.h"
@@ -34,6 +35,31 @@ storage_engine_ref get_storage_engine(TH
if (table)
{
se= plugin_ref_to_se_ref(table->s->db_plugin);
+
+ if (table->part_info)
+ {
+ partition_info *p_info= table->part_info;
+ List_iterator<partition_element> p_it(p_info->partitions);
+ partition_element *p_el;
+ storage_engine_ref se1= NULL;
+
+ while ((p_el= p_it++))
+ {
+ if (!se1)
+ {
+ se1= get_hton_se(p_el->engine_type);
+ continue;
+ }
+
+ if (se1 != get_hton_se(p_el->engine_type))
+ goto close;
+ };
+
+ se= se1;
+ }
+
+ close:
+
::intern_close_table(table);
my_free(table, MYF(0));
}
=== modified file 'sql/backup/backup_aux.h'
--- sql/backup/backup_aux.h 2008-10-14 12:08:56 +0000
+++ sql/backup/backup_aux.h 2008-11-04 09:26:34 +0000
@@ -19,6 +19,7 @@ typedef st_plugin_int* storage_engine_re
#define se_ref_to_plugin_ref(A) &(A)
#endif
+
inline
const char* se_name(storage_engine_ref se)
{ return se->name.str; }
@@ -32,6 +33,17 @@ handlerton* se_hton(storage_engine_ref s
{ return (handlerton*)(se->data); }
inline
+storage_engine_ref get_hton_se(const handlerton *hton)
+{
+ DBUG_ASSERT(hton);
+#ifdef DBUG_OFF
+ return plugin_ref_to_se_ref(hton2plugin[hton->slot]);
+#else
+ return plugin_ref_to_se_ref(&hton2plugin[hton->slot]);
+#endif
+}
+
+inline
storage_engine_ref get_se_by_name(const LEX_STRING name)
{
plugin_ref plugin= ::ha_resolve_by_name(::current_thd, &name);
[25 Nov 2008 14:30]
Jørgen Løland
While the code pasted by Rafal above does select the correct driver, a simple modification of the test script reveals that this is not a good solution. The MyISAM native driver is not able to handle partitioned tables because the filenames are not as expected. One possible solution is to treat partitions as first-class citizens, i.e., back up partitions instead of tables. However, this will require some refactoring of other parts of the backup code. Another possibility is to make myisam driver aware of the partitioning. So far, there is no conclusion for what the correct solution would be. Script that fails with the suggested code change: ------------------------------------------------- CREATE DATABASE backup_concurrent; USE backup_concurrent; CREATE TABLE inter1 ( t1_autoinc INTEGER NOT NULL AUTO_INCREMENT, t1_uuid CHAR(36), PRIMARY KEY (t1_autoinc) ) ENGINE = MyISAM; ALTER TABLE inter1 PARTITION BY HASH(t1_autoinc); BACKUP DATABASE backup_concurrent TO 'backup1'; ------------------------------------------------ ^Fails, saying that it can't find file 'inter1.myi'. The partitioned tables are stored in filenames 'inter1#P#p0.myi' etc.
[25 Nov 2008 17:07]
Rafal Somla
Yet another solution is to accept the fact that MyISAM native driver can not handle partitioned tables and only use the built-in, logical drivers for such ones. If the underlying engine of a partitioned table supports MVCC then the snapshot driver could be used, otherwise the default blocking driver.
[28 Nov 2008 8:53]
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/60123 2746 Jorgen Loland 2008-11-28 Bug#38426 - Backup of partitioned Falcon table will not use snapshot driver Before, the Default backup driver was used when backing up a partitioned table. However, the Snapshot driver is capable of backing up partitioned tables if the underlying storage engine is InnoDB or Falcon. With this patch, the Snapshot backup driver is used if the underlying storage engine of a partitioned table is InnoDB or Falcon
[28 Nov 2008 9: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/60126 2746 Jorgen Loland 2008-11-28 Bug#38426 - Backup of partitioned Falcon table will not use snapshot driver Before, the Default backup driver was used when backing up a partitioned table. However, the Snapshot driver is capable of backing up partitioned tables if the underlying storage engine is InnoDB or Falcon. With this patch, the Snapshot backup driver is used if the underlying storage engine of a partitioned table is InnoDB or Falcon
[28 Nov 2008 9:58]
Rafal Somla
Good to push (but fix the code layout first!).
[4 Dec 2008 12:43]
Øystein Grøvlen
Patch approved.
[4 Dec 2008 13:03]
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/60597 2737 Jorgen Loland 2008-12-04 Bug#38426 - Backup of partitioned Falcon table will not use snapshot driver Before, the Default backup driver was used when backing up a partitioned table. However, the Snapshot driver is capable of backing up partitioned tables if the underlying storage engine is InnoDB or Falcon. With this patch, the Snapshot backup driver is used if the underlying storage engine of a partitioned table is InnoDB or Falcon
[4 Dec 2008 14:36]
Jørgen Løland
Pushed to mysql-6.0-backup Nov 4
[4 Dec 2008 15:24]
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/60618 2738 Jorgen Loland 2008-12-04 Bug#38426 - Backup of partitioned Falcon table will not use snapshot driver Followup patch - compile partition-specific parts of backup code only if partitioning is included in the server
[5 Dec 2008 7:29]
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/60687 2739 Jorgen Loland 2008-12-05 Bug#38426 - Backup of partitioned Falcon table will not use snapshot driver Followup patch - do not run backup_partition test in embedded mode
[12 Dec 2008 6:25]
Bugs System
Pushed into 6.0.9-alpha (revid:jorgen.loland@sun.com-20081205073027-oar6voklddqr6o6l) (version source revid:ingo.struewing@sun.com-20081205135144-jgtg6bwlpd8c9rwg) (pib:5)
[13 Dec 2008 17:20]
Paul DuBois
Noted in 6.0.9 changelog. BACKUP DATABASE failed to use the native driver for a Falcon table if the table was partitioned.
