Re: [Maria-developers] [Commits] 1cd9319: myisam/aria: don't mess with IO_CACHE::file
Hi, Sergey! On May 27, Sergey Vojtovich wrote:
Hi Sergei,
looks good. Ok to push, a few minor suggestions inline.
diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index 8c012d2..d50528e 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -4267,20 +4267,22 @@ int maria_repair_parallel(HA_CHECK *param, register MARIA_HA *info,
if (!(sort_info.key_block= alloc_key_blocks(param, (uint) param->sort_key_blocks, - share->base.max_key_block_length)) || - init_io_cache(¶m->read_cache, info->dfile.file, + share->base.max_key_block_length))) + goto err; + + if (init_io_cache(¶m->read_cache, info->dfile.file, (uint) param->read_buffer_length, - READ_CACHE, share->pack.header_length, 1, MYF(MY_WME)) || - (!rep_quick && - (init_io_cache(&info->rec_cache, info->dfile.file, - (uint) param->write_buffer_length, - WRITE_CACHE, new_header_length, 1, - MYF(MY_WME | MY_WAIT_IF_FULL) & param->myf_rw) || - init_io_cache(&new_data_cache, -1, - (uint) param->write_buffer_length, - READ_CACHE, new_header_length, 1, - MYF(MY_WME | MY_DONT_CHECK_FILESIZE))))) + READ_CACHE, share->pack.header_length, 1, MYF(MY_WME))) goto err; + + if (!rep_quick) + { + if (init_io_cache(&new_data_cache, -1, + (uint) param->write_buffer_length, + READ_CACHE, new_header_length, 1, + MYF(MY_WME | MY_DONT_CHECK_FILESIZE))) + goto err; + } Why not to move new_data_cache initialization down along with rec_cache?
I've simply kept it where it was, the goal was to avoid io_cache.file=new_file_descriptor; for example, there was info->rec_cache.file=new_file; and I moved info->rec_cache initialization down where new_file was known and could be passed as an argument to init_io_cache(). There was no such problem with new_data_cache. Regards, Sergei
Hi Sergei, my concern was that we do if (!rep_quick) twice in a very short distance. Anyway, this was just a minor wish. Final decision is up to you. Regards, Sergey On Wed, May 27, 2015 at 09:02:03PM +0200, Sergei Golubchik wrote:
Hi, Sergey!
On May 27, Sergey Vojtovich wrote:
Hi Sergei,
looks good. Ok to push, a few minor suggestions inline.
diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index 8c012d2..d50528e 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -4267,20 +4267,22 @@ int maria_repair_parallel(HA_CHECK *param, register MARIA_HA *info,
if (!(sort_info.key_block= alloc_key_blocks(param, (uint) param->sort_key_blocks, - share->base.max_key_block_length)) || - init_io_cache(¶m->read_cache, info->dfile.file, + share->base.max_key_block_length))) + goto err; + + if (init_io_cache(¶m->read_cache, info->dfile.file, (uint) param->read_buffer_length, - READ_CACHE, share->pack.header_length, 1, MYF(MY_WME)) || - (!rep_quick && - (init_io_cache(&info->rec_cache, info->dfile.file, - (uint) param->write_buffer_length, - WRITE_CACHE, new_header_length, 1, - MYF(MY_WME | MY_WAIT_IF_FULL) & param->myf_rw) || - init_io_cache(&new_data_cache, -1, - (uint) param->write_buffer_length, - READ_CACHE, new_header_length, 1, - MYF(MY_WME | MY_DONT_CHECK_FILESIZE))))) + READ_CACHE, share->pack.header_length, 1, MYF(MY_WME))) goto err; + + if (!rep_quick) + { + if (init_io_cache(&new_data_cache, -1, + (uint) param->write_buffer_length, + READ_CACHE, new_header_length, 1, + MYF(MY_WME | MY_DONT_CHECK_FILESIZE))) + goto err; + } Why not to move new_data_cache initialization down along with rec_cache?
I've simply kept it where it was, the goal was to avoid
io_cache.file=new_file_descriptor;
for example, there was
info->rec_cache.file=new_file;
and I moved info->rec_cache initialization down where new_file was known and could be passed as an argument to init_io_cache(). There was no such problem with new_data_cache.
Regards, Sergei
Hi, Sergey! On May 27, Sergey Vojtovich wrote:
Hi Sergei,
my concern was that we do if (!rep_quick) twice in a very short distance. Anyway, this was just a minor wish. Final decision is up to you.
I've already fixed it. Regards, Sergei
Why not to move new_data_cache initialization down along with rec_cache?
I've simply kept it where it was, the goal was to avoid
io_cache.file=new_file_descriptor;
for example, there was
info->rec_cache.file=new_file;
and I moved info->rec_cache initialization down where new_file was known and could be passed as an argument to init_io_cache(). There was no such problem with new_data_cache.
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich