← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 8a595a9: cleanup: LOAD DATA replication support in IO_CACHE

 

Hi Sergei,

ok to push.

On Wed, May 27, 2015 at 08:14:59PM +0200, Sergei Golubchik wrote:
> Hi, Sergey!
> 
> On May 27, Sergey Vojtovich wrote:
> > On Tue, May 26, 2015 at 03:37:36PM +0200, serg@xxxxxxxxxxx wrote:
> > > revision-id: 8a595a9825de286ad073f6f043db4c0dce88a2ea
> > > parent(s): 3e55ef26d49a900782d2c2bb2c03470faed6ec9d
> > > committer: Sergei Golubchik
> > > branch nick: maria
> > > timestamp: 2015-05-26 15:09:25 +0200
> > > message:
> > > 
> > > cleanup: LOAD DATA replication support in IO_CACHE
...skip...

> > IOCACHE::read_function may change, e.g. on error in _my_b_async_read().
> > Shouldn't we handle this?
> 
> async is dead code, but theoretically it might change.
> how would you suggest to handle that?
> 
> I'd leave it as is until we have a test case.
Maybe add an assertion if you can foresee some practical cases when
read_function may change. Up to you.

> > > @@ -1432,8 +1389,7 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs,
> > >  
> > >  READ_INFO::~READ_INFO()
> > >  {
> > > -  if (need_end_io_cache)
> > > -    ::end_io_cache(&cache);
> > > +  ::end_io_cache(&cache);
> > >    my_free(buffer);
> > >    List_iterator<XML_TAG> xmlit(taglist);
> > >    XML_TAG *t;
> > Hmm... I may miss something, but shouldn't we call log_loaded_block() for last
> > successfully read block?
> 
> Yes, I do that by calling log_loaded_block() explicitly.
> 
> But now end_io_cache() only closes the IO_CACHE it does not log
> anything. Which means I don't need to call end_io_cache() early. And
> don't need 'need_end_io_cache' variable - I can unconditionally call
> end_io_cache() at the end.
Somehow I thought that the second log_loaded_block() was inside "if (error)"
too. Sorry for confusion.

Thanks,
Sergey


References