Re: [Maria-developers] [Commits] 1cd9319: myisam/aria: don't mess with IO_CACHE::file

[Sergey Vojtovich]

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(&param->read_cache, info->dfile.file,
> > > +                  share->base.max_key_block_length)))
> > > +    goto err;
> > > +
> > > +  if (init_io_cache(&param->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

_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to     : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp