From cd15ea74f77c24ad875a5ea5992b276d984fee54 Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 19 Jan 2008 04:51:38 +0100 Subject: [PATCH] - fix for bug when creating bitmaps - fix for bug seen when running test "type_datetime" with Maria (wrong data_file_length in maria_repair()) - fix for bug seen when running test "repair" with Maria (myisam_sort_buffer_size was influencing Maria) sql/handler.cc: Sounds illogical to store myisam_sort_buffer_size into a structure used by all engines. There are only MyISAM and Maria which used sort_buffer_size: they can get their value from their respective system variable (myisam|maria_sort_buffer_size). Using MyISAM's value for all engines was wrong (myisam_sort_buffer_size influenced Maria). sql/handler.h: not needed storage/maria/ha_maria.cc: check_opt->sort_buffer_size was myisam_sort_buffer_size; Maria must use maria_sort_buffer_size instead. storage/maria/ma_bitmap.c: don't use my_chsize() now that Monty re-explained the problem to me :) storage/maria/ma_check.c: making maria_repair() work like maria_repair_by_sort(): sort_param.filepos must be set at start then possibly corrected by create_new_data_handle(); in the opposite order, filepos is finally set to 0, and if the table has no records, it stays 0 and this causes state.data_file_length to be 0 which is incorrect for a BLOCK_RECORD table having always at least one bitmap page. storage/maria/ma_pagecache.c: Comments storage/myisam/ha_myisam.cc: check_opt->sort_buffer_size is gone --- sql/handler.cc | 1 - sql/handler.h | 1 - storage/maria/ha_maria.cc | 6 +++--- storage/maria/ma_bitmap.c | 25 +++++++++++++++++-------- storage/maria/ma_check.c | 8 ++++---- storage/maria/ma_pagecache.c | 7 ++++++- storage/myisam/ha_myisam.cc | 4 ++-- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/sql/handler.cc b/sql/handler.cc index d70eca512ef..6489041eaf1 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -2735,7 +2735,6 @@ int ha_create_table_from_engine(THD* thd, const char *db, const char *name) void st_ha_check_opt::init() { flags= sql_flags= 0; - sort_buffer_size = current_thd->variables.myisam_sort_buff_size; } diff --git a/sql/handler.h b/sql/handler.h index d7183449ad5..75c56e8339e 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -890,7 +890,6 @@ typedef class Item COND; typedef struct st_ha_check_opt { st_ha_check_opt() {} /* Remove gcc warning */ - ulong sort_buffer_size; uint flags; /* isam layer flags (e.g. for myisamchk) */ uint sql_flags; /* sql layer flags - for something myisamchk cannot do */ KEY_CACHE *key_cache; /* new key cache when changing key cache */ diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index 5ccb2799950..53ee96cd048 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -1149,7 +1149,7 @@ int ha_maria::repair(THD * thd, HA_CHECK_OPT *check_opt) param.testflag= ((check_opt->flags & ~(T_EXTEND)) | T_SILENT | T_FORCE_CREATE | T_CALC_CHECKSUM | (check_opt->flags & T_EXTEND ? T_REP : T_REP_BY_SORT)); - param.sort_buffer_length= check_opt->sort_buffer_size; + param.sort_buffer_length= THDVAR(thd, sort_buffer_size); start_records= file->state->records; while ((error= repair(thd, param, 0)) && param.retry_repair) { @@ -1196,7 +1196,7 @@ int ha_maria::zerofill(THD * thd, HA_CHECK_OPT *check_opt) param.thd= thd; param.op_name= "zerofill"; param.testflag= check_opt->flags | T_SILENT | T_ZEROFILL; - param.sort_buffer_length= check_opt->sort_buffer_size; + param.sort_buffer_length= THDVAR(thd, sort_buffer_size); error=maria_zerofill(¶m, file, file->s->open_file_name); return error; @@ -1214,7 +1214,7 @@ int ha_maria::optimize(THD * thd, HA_CHECK_OPT *check_opt) param.op_name= "optimize"; param.testflag= (check_opt->flags | T_SILENT | T_FORCE_CREATE | T_REP_BY_SORT | T_STATISTICS | T_SORT_INDEX); - param.sort_buffer_length= check_opt->sort_buffer_size; + param.sort_buffer_length= THDVAR(thd, sort_buffer_size); if ((error= repair(thd, param, 1)) && param.retry_repair) { sql_print_warning("Warning: Optimize table got errno %d on %s.%s, retrying", diff --git a/storage/maria/ma_bitmap.c b/storage/maria/ma_bitmap.c index 3751895ec75..b54d7d12b97 100644 --- a/storage/maria/ma_bitmap.c +++ b/storage/maria/ma_bitmap.c @@ -2617,6 +2617,8 @@ void _ma_bitmap_set_pagecache_callbacks(PAGECACHE_FILE *file, /** Extends data file with zeroes and creates new bitmap pages into page cache. + Writes all bitmap pages in [from, to]. + Non-bitmap pages of zeroes are correct as they are marked empty in bitmaps. Bitmap pages will not be zeroes: they will get their CRC fixed when flushed. And if there is a crash before flush (so they are zeroes at @@ -2632,17 +2634,24 @@ _ma_bitmap_create_missing_into_pagecache(MARIA_SHARE *share, { pgcache_page_no_t i; /* - We use my_chsize() to not rely on OS filling gaps with zeroes and to have - sequential order in the file (allocate all new data and bitmap pages from - the filesystem). + We do not use my_chsize() because there can be a race between when it + reads the physical size and when it writes (assume data_file_length is 10, + physical length is 8 and two data pages are in cache, and here we do a + my_chsize: my_chsize sees physical length is 8, then the two data pages go + to disk then my_chsize writes from page 8 and so overwrites the two data + pages, wrongly). + We instead rely on the filesystem filling gaps with zeroes. */ - if (my_chsize(bitmap->file.file, (to + 1) * bitmap->block_size, 0, - MYF(MY_WME))) - goto err; - /* Write all bitmap pages in [from, to] */ for (i= from; i <= to; i+= bitmap->pages_covered) { - /* no need to keep them pinned, they are new so flushable */ + /** + No need to keep them pinned, they are new so flushable. + @todo but we may want to keep them pinned, as an optimization: if they + are not pinned they may go to disk before the data pages go (so, the + physical pages would be in non-ascending "sparse" order on disk), or the + filesystem may fill gaps with zeroes physically which is a waste of + time. + */ if (pagecache_write(share->pagecache, &bitmap->file, i, 0, zeroes, PAGECACHE_PLAIN_PAGE, diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index 79bd33586a8..5541840db6c 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -2249,8 +2249,8 @@ int maria_repair(HA_CHECK *param, register MARIA_HA *info, if (reenable_logging) _ma_tmp_disable_logging_for_table(info, 0); - new_header_length= ((param->testflag & T_UNPACK) ? 0L : - share->pack.header_length); + sort_param.filepos= new_header_length= + ((param->testflag & T_UNPACK) ? 0L : share->pack.header_length); if (!rep_quick) { @@ -2321,7 +2321,6 @@ int maria_repair(HA_CHECK *param, register MARIA_HA *info, sort_param.read_cache=param->read_cache; sort_param.pos=sort_param.max_pos=share->pack.header_length; - sort_param.filepos=new_header_length; param->read_cache.end_of_file= sort_info.filelength; sort_param.master=1; sort_info.max_records= ~(ha_rows) 0; @@ -3414,7 +3413,7 @@ int maria_repair_by_sort(HA_CHECK *param, register MARIA_HA *info, /* for external plugin parser we cannot tell anything at all :( so, we'll use all the sort memory and start from ~10 buffpeks. - (see _create_index_by_sort) + (see _ma_create_index_by_sort) */ sort_info.max_records= 10*param->sort_buffer_length/sort_param.key_length; @@ -5967,6 +5966,7 @@ static my_bool create_new_data_handle(MARIA_SORT_PARAM *param, File new_file) if (_ma_initialize_data_file(new_info->s, new_file)) DBUG_RETURN(1); + /* Take into account any bitmap page created above: */ param->filepos= new_info->state->data_file_length; /* Use new virtual functions for key generation */ diff --git a/storage/maria/ma_pagecache.c b/storage/maria/ma_pagecache.c index 0ffd0ae2242..7e73a0b1abf 100644 --- a/storage/maria/ma_pagecache.c +++ b/storage/maria/ma_pagecache.c @@ -3396,6 +3396,7 @@ restart: DBUG_ASSERT(block->type == PAGECACHE_EMPTY_PAGE || block->type == PAGECACHE_READ_UNKNOWN_PAGE || block->type == type || + /* this is for when going to non-trans to trans */ (block->type == PAGECACHE_PLAIN_PAGE && type == PAGECACHE_LSN_PAGE)); block->type= type; @@ -4239,7 +4240,11 @@ my_bool pagecache_collect_changed_blocks_with_lsn(PAGECACHE *pagecache, */ DBUG_ASSERT(block->hash_link != NULL); DBUG_ASSERT(block->status & PCBLOCK_CHANGED); - /* Note that we don't store bitmap pages */ + /* + Note that we don't store bitmap pages, or pages from non-transactional + (like temporary) tables. Don't checkpoint during Recovery which uses + PAGECACHE_PLAIN_PAGE. + */ if (block->type != PAGECACHE_LSN_PAGE) continue; /* no need to store it */ stored_list_size++; diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index 42e3215403c..4e8f300fcf5 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -969,7 +969,7 @@ int ha_myisam::repair(THD* thd, HA_CHECK_OPT *check_opt) param.testflag= ((check_opt->flags & ~(T_EXTEND)) | T_SILENT | T_FORCE_CREATE | T_CALC_CHECKSUM | (check_opt->flags & T_EXTEND ? T_REP : T_REP_BY_SORT)); - param.sort_buffer_length= check_opt->sort_buffer_size; + param.sort_buffer_length= thd->variables.myisam_sort_buff_size; start_records=file->state->records; while ((error=repair(thd,param,0)) && param.retry_repair) { @@ -1015,7 +1015,7 @@ int ha_myisam::optimize(THD* thd, HA_CHECK_OPT *check_opt) param.op_name= "optimize"; param.testflag= (check_opt->flags | T_SILENT | T_FORCE_CREATE | T_REP_BY_SORT | T_STATISTICS | T_SORT_INDEX); - param.sort_buffer_length= check_opt->sort_buffer_size; + param.sort_buffer_length= thd->variables.myisam_sort_buff_size; if ((error= repair(thd,param,1)) && param.retry_repair) { sql_print_warning("Warning: Optimize table got errno %d on %s.%s, retrying",