Hi, Sergey!
On May 27, Sergey Vojtovich wrote:
> On Tue, May 26, 2015 at 03:37:36PM +0200, s...@mariadb.org 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
> >
> > remove some 14-year old code that added support for
> > LOAD DATA replication to IO_CACHE:
> > * three callbacks, of which only two were actually used and that
> > were only needed for LOAD DATA replication but were
> > tested in every IO_CACHE instance
> > * an additional opaque void * argument in IO_CACHE, also only
> > used for LOAD DATA replication, but present everywhere
> > * the code to close IO_CACHE prematurely in LOAD DATA to have
> > these callbacks called in the correct order and a long
> > comment explaining what will happen if IO_CACHE is not
> > closed prematurely
> > * a variable to track whether IO_CACHE was closed prematurely
> > (to avoid double-closing it)
> >
> > this also fixes a bug with replication of LOAD DATA LOCAL
> > (IO_CACHE of READ_NET type was not calling these callbacks at all)
> I couldn't track this down. That is GET/my_b_get/pre_read was called in both
> cases as well as end_io_cache/pre_close. Could you elaborate?
Right, that part is incorrect. I'll remove it.
> ...skip...
>
> > diff --git a/sql/sql_load.cc b/sql/sql_load.cc
> > index 18982c5..62b6e17 100644
> > --- a/sql/sql_load.cc
> > +++ b/sql/sql_load.cc
> > @@ -1410,20 +1372,15 @@ READ_INFO::READ_INFO(File file_par, uint
> > tot_length, CHARSET_INFO *cs,
> > }
> > else
> > {
> > - /*
> > - init_io_cache() will not initialize read_function member
> > - if the cache is READ_NET. So we work around the problem with a
> > - manual assignment
> > - */
> > - need_end_io_cache = 1;
> > -
> > #ifndef EMBEDDED_LIBRARY
> > if (get_it_from_net)
> > cache.read_function = _my_b_net_read;
> >
> > if (mysql_bin_log.is_open())
> > - cache.pre_read = cache.pre_close =
> > - (IO_CACHE_CALLBACK) log_loaded_block;
> > + {
> > + cache.real_read_function= cache.read_function;
> > + cache.read_function= log_loaded_block;
> > + }
> > #endif
> > }
> > }
> Will this compile with replication disabled? OTOH I guess it didn't compile
> even
> before: log_loaded_block() seem to be unavailable when replication is
> disabled.
Yes, it compiles for embedded (which has replication disabled). I don't
know if the server can be compiled with replication disabled anymore.
> 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.
> > @@ -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.
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
