On Mon, Jul 07, 2008 at 04:32:13PM +0200, Ingo Strüwing wrote:
> Hi Guilhem,
>
> thank you.
>
> Guilhem Bichot, 07.07.2008 16:06:
> ...
> > Thanks for this enormous hack. Good comments.
> > I'd put a "return;" instead of the "do {} while (0);",
> > or I'd remove "end:" as well.
>
> Yes, the hack can be better integrated with the function. I wanted to
> keep it together in one section so that it can be removed easier, when
> the issue is fixed cleanly.
>
> Btw, the change in mi_examine_log.c can also be beautified. Since we
> don't suppress updating of the index file on close any more, the boolean
> "update_index_on_close" is no longer named properly. And a couple of
> comments are misleading. When a final decision about restore locking is
> made, this may need more updates. So we can defer it for now.
Ok to push.
> > I wonder: Table_restore::post_restore() (which may include a repair)
> > runs after the pieces which you fixed above. So, does a last bug lurk
> > there?
>
> I don't know if I understand the question correctly. But as far as I get
> it, there should be no problem as close() should be called before
> post_restore(). The share and other instances are fixed in close() now.
> So later open()/close() should always work on the final data.
But repair may change the state, so the states updated in close() by
your patch, won't it become out-of-date again after repair() has made
changes (like index_file_length, key_root etc) ?
> Btw, if we stick with open_and_lock_tables() for the tables in restore,
> we should refactor the driver to work on the M_INFO instance that the
> backup kernel provides. This would save us some mi_open()/mi_close() and
> the search loop in my hack.
Yes.
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Mr. Guilhem Bichot <guilhem@stripped>
/ /|_/ / // /\ \/ /_/ / /__ MySQL France, Lead Software Engineer
/_/ /_/\_, /___/\___\_\___/ Bordeaux, France
<___/ www.mysql.com
