Commit e06a76cd authored by unknown's avatar unknown

Fix for three bugs:

number 1: "./mtr --mysqld=--default-storage-engine=maria backup"
restored no rows (forgot to flush data pages before my_copy(),
and also the maria_repair() used by ha_maria::restore() needed
a correct data_file_length to not miss rows). [note that BACKUP
TABLE will be removed anyway in 5.2]
number 2: "./mtr --mysqld=--default-storage-engine=maria bootstrap"
caused segfault (uninitialized variable)
number 3: "./mtr --mysqld=--default-storage-engine=maria check"
showed warning in CHECK TABLE (maria_create() created a non-empty
data file with data_file_length==0).


storage/maria/ha_maria.cc:
  in ha_maria::backup, need to flush the data file before copying it,
  otherwise data misses from the copy (bug 1)
storage/maria/ma_bitmap.c:
  when allocating data at the end of the bitmap, best_data is at "end",
  should not be left to 0 (bug 2)
storage/maria/ma_check.c:
  _ma_scan_block_record() is used in QUICK repair. It relies on
  data_file_length. RESTORE TABLE mixes the MAI of an empty table
  (so, data_file_length==0) with an non-empty MAD, and does a 
  QUICK repair; that got fooled (thought it had hit EOF immediately,
  so found no records) (bug 1)
storage/maria/ma_create.c:
  At the end of maria_create() we have, in the index file,
  data_file_length==0, while the data file has a bitmap page (8192).
  This inconsistency makes CHECK TABLE rightly complain.
  Fixed by not creating a first bitmap page during maria_create()
  (also saves disk space) (bug 3) Question for Monty.
storage/maria/ma_extra.c:
  A function to flush the data and index files before one can
  use OS syscalls (reads, writes) on those files. For example,
  ha_maria::backup() does a my_copy() of the data file and so
  all cached pieces of this file must be sent to the OS (bug 1)
  This function will have to be used elsewhere in Maria, several places
  have not been updated when we added pagecache-ing of the data file
  (they still only flush the index file), they are probable bugs.
storage/maria/maria_def.h:
  new function. Needs to be visible from ha_maria::backup.
parent 4f87d067
...@@ -1063,6 +1063,13 @@ int ha_maria::backup(THD * thd, HA_CHECK_OPT *check_opt) ...@@ -1063,6 +1063,13 @@ int ha_maria::backup(THD * thd, HA_CHECK_OPT *check_opt)
} }
strxmov(src_path, table->s->normalized_path.str, MARIA_NAME_DEXT, NullS); strxmov(src_path, table->s->normalized_path.str, MARIA_NAME_DEXT, NullS);
if (_ma_flush_table_files(file, MARIA_FLUSH_DATA, FLUSH_FORCE_WRITE,
FLUSH_KEEP))
{
error= HA_ADMIN_FAILED;
errmsg= "Failed in flush (Error %d)";
goto err;
}
if (my_copy(src_path, dst_path, if (my_copy(src_path, dst_path,
MYF(MY_WME | MY_HOLD_ORIGINAL_MODES | MY_DONT_OVERWRITE_FILE))) MYF(MY_WME | MY_HOLD_ORIGINAL_MODES | MY_DONT_OVERWRITE_FILE)))
{ {
......
...@@ -837,6 +837,7 @@ static my_bool allocate_tail(MARIA_FILE_BITMAP *bitmap, uint size, ...@@ -837,6 +837,7 @@ static my_bool allocate_tail(MARIA_FILE_BITMAP *bitmap, uint size,
if (bitmap->used_size == bitmap->total_size) if (bitmap->used_size == bitmap->total_size)
DBUG_RETURN(1); DBUG_RETURN(1);
/* Allocate data at end of bitmap */ /* Allocate data at end of bitmap */
best_data= end;
bitmap->used_size+= 6; bitmap->used_size+= 6;
best_pos= best_bits= 0; best_pos= best_bits= 0;
} }
......
...@@ -3753,7 +3753,15 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param) ...@@ -3753,7 +3753,15 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param)
} }
else else
{ {
/* Scan on clean table */ /*
Scan on clean table.
It requires a reliable data_file_length so we set it.
*/
my_off_t dfile_len= my_seek(info->dfile.file, 0, SEEK_END,
MYF(MY_WME));
if (dfile_len == MY_FILEPOS_ERROR)
DBUG_RETURN(my_errno);
info->state->data_file_length= dfile_len;
flag= _ma_scan_block_record(info, sort_param->record, flag= _ma_scan_block_record(info, sort_param->record,
info->cur_row.nextpos, 1); info->cur_row.nextpos, 1);
} }
......
...@@ -1045,15 +1045,28 @@ int maria_create(const char *name, enum data_file_type datafile_type, ...@@ -1045,15 +1045,28 @@ int maria_create(const char *name, enum data_file_type datafile_type,
log record log record
- data file must be created after log record, so that "missing log - data file must be created after log record, so that "missing log
record" implies "unusable table"). record" implies "unusable table").
When we wrote the state, we hadn't called ma_initialize_data_file(), so
the data_file_length is 0!
Thus, we below create a 8192-byte data file, but its recorded size is 0, Thus, we below create a 8192-byte data file, but its recorded size is 0,
so next time we read the bitmap (a maria_write() for example) we'll so next time we read the bitmap (a maria_write() for example) we'll
overwrite the bitmap we just created below. overwrite the bitmap we just created below.
It's not very efficient. Though there is no bug. It's not very efficient.
It also makes maria_chk_size() print
Size of datafile is: 8192 Should be: 0
on a freshly created table (run "check.test" with a Maria table).
Why do we absolutely want to create a 8192-byte page for a freshly Why do we absolutely want to create a 8192-byte page for a freshly
created, empty table? Why don't we leave the data file empty? created, empty table? Why don't we leave the data file empty?
Removing the call below at least removes the maria_chk_size() issue.
Monty wrote on IRC, about a size of 0:
"This basically ok; The first block is a bitmap that may or may not
exists", but later he asked that the first block always exists.???
*/ */
#ifdef ASK_MONTY
if (_ma_initialize_data_file(&share, dfile)) if (_ma_initialize_data_file(&share, dfile))
goto err; goto err;
#endif
} }
/* Enlarge files */ /* Enlarge files */
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#ifdef HAVE_SYS_MMAN_H #ifdef HAVE_SYS_MMAN_H
#include <sys/mman.h> #include <sys/mman.h>
#endif #endif
#include "ma_blockrec.h"
static void maria_extra_keyflag(MARIA_HA *info, static void maria_extra_keyflag(MARIA_HA *info,
enum ha_extra_function function); enum ha_extra_function function);
...@@ -279,6 +280,8 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function, ...@@ -279,6 +280,8 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function,
Don't we wait for all instances to be closed before dropping the table? Don't we wait for all instances to be closed before dropping the table?
Do we ever do something useful here? Do we ever do something useful here?
BUG? BUG?
FLUSH_IGNORE_CHANGED: we are also throwing away unique index blocks?
Does ENABLE KEYS rebuild them too?
*/ */
if (flush_pagecache_blocks(share->pagecache, &share->kfile, if (flush_pagecache_blocks(share->pagecache, &share->kfile,
FLUSH_IGNORE_CHANGED)) FLUSH_IGNORE_CHANGED))
...@@ -488,3 +491,60 @@ int _ma_sync_table_files(const MARIA_HA *info) ...@@ -488,3 +491,60 @@ int _ma_sync_table_files(const MARIA_HA *info)
return (my_sync(info->dfile.file, MYF(0)) || return (my_sync(info->dfile.file, MYF(0)) ||
my_sync(info->s->kfile.file, MYF(0))); my_sync(info->s->kfile.file, MYF(0)));
} }
/**
@brief flushes the data and/or index file of a table
This is useful when one wants to read a table using OS syscalls (like
my_copy()) and first wants to be sure that MySQL-level caches go down to
the OS so that OS syscalls can see all data. It can flush rec_cache,
bitmap, pagecache of data file, pagecache of index file.
@param info table
@param flush_data_or_index one or two of these flags:
MARIA_FLUSH_DATA, MARIA_FLUSH_INDEX
@param flush_type_for_data
@param flush_type_for_index
@note does not sync files (@see _ma_sync_table_files()).
@note Progressively this function will be used in all places where we flush
the index but not the data file (probable bugs).
@return Operation status
@retval 0 OK
@retval 1 Error
*/
int _ma_flush_table_files(MARIA_HA *info, uint flush_data_or_index,
enum flush_type flush_type_for_data,
enum flush_type flush_type_for_index)
{
MARIA_SHARE *share= info->s;
/* flush data file first because it's more critical */
if (flush_data_or_index & MARIA_FLUSH_DATA)
{
if (info->opt_flag & WRITE_CACHE_USED)
{
if (end_io_cache(&info->rec_cache))
goto err;
info->opt_flag&= ~WRITE_CACHE_USED;
}
if (share->data_file_type == BLOCK_RECORD)
{
if(_ma_flush_bitmap(share) ||
flush_pagecache_blocks(share->pagecache, &info->dfile,
flush_type_for_data))
goto err;
}
}
if ((flush_data_or_index & MARIA_FLUSH_INDEX) &&
flush_pagecache_blocks(share->pagecache, &share->kfile,
flush_type_for_index))
goto err;
return 0;
err:
maria_print_error(info->s, HA_ERR_CRASHED);
maria_mark_crashed(info);
return 1;
}
...@@ -902,6 +902,11 @@ MARIA_RECORD_POS _ma_write_init_default(MARIA_HA *info, const uchar *record); ...@@ -902,6 +902,11 @@ MARIA_RECORD_POS _ma_write_init_default(MARIA_HA *info, const uchar *record);
my_bool _ma_write_abort_default(MARIA_HA *info); my_bool _ma_write_abort_default(MARIA_HA *info);
C_MODE_START C_MODE_START
#define MARIA_FLUSH_DATA 1
#define MARIA_FLUSH_INDEX 2
int _ma_flush_table_files(MARIA_HA *info, uint flush_data_or_index,
enum flush_type flush_type_for_data,
enum flush_type flush_type_for_index);
/* Functions needed by _ma_check (are overrided in MySQL) */ /* Functions needed by _ma_check (are overrided in MySQL) */
volatile int *_ma_killed_ptr(HA_CHECK *param); volatile int *_ma_killed_ptr(HA_CHECK *param);
void _ma_check_print_error _VARARGS((HA_CHECK *param, const char *fmt, ...)); void _ma_check_print_error _VARARGS((HA_CHECK *param, const char *fmt, ...));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment