Hi, Sergei!
On Wed, Oct 07, 2009 at 07:01:32PM +0200, Sergei Golubchik wrote:
> Hi, Sergey!
>
> On Oct 06, Sergey Vojtovich wrote:
> > #At file:///home/svoj/devel/bzr-mysql/mysql-5.1-bugteam-bug47073/ based on
> revid:mattias.jonsson@stripped
> >
> > 3146 Sergey Vojtovich 2009-10-06
> > BUG#47073 - valgrind errs, corruption,failed repair of partition,
> > low myisam_sort_buffer_size
> >
> > Repair by sort (default) or parallel repair of a MyISAM table
> > (doesn't matter partitioned or not) as well as bulk inserts
> > and enable indexes some times didn't failover to repair with
> > key cache.
> >
> > The problem was that after unsuccessful attempt, data file was
> > closed. Whereas repair with key cache requires open data file.
> > Fixed by reopening data file.
>
> Few question.
>
> Why, as bug report says, "partitioned tables get corrupted during a
> repair table. non-partitioned tables didn't" ?
The above statement is wrong. Provided test case successfully falls back
to repair with key cache with pure MyISAM table, since during repair
by sort it fails to fix first (shorter) index. The reason is larger
number of rows.
> Shouldn't you remove Ingo's fix for bug#25289 ?
> Looks like it becomes a dead code after your changes.
I'd leave this for safety. There is still a tiny chance that we fail to
re-open data file.
> > @ storage/myisam/sort.c
> > When performing a copy of IO_CACHE structure, current_pos and
> > current_end must be updated separatly to point to memory we're
> > copying to (not to memory we're copying from).
> >
> > As t_file2 is always WRITE cache, proper members are write_pos
> > and write_end accordingly.
> >
> > === modified file 'storage/myisam/sort.c'
> > --- a/storage/myisam/sort.c 2009-08-28 16:21:54 +0000
> > +++ b/storage/myisam/sort.c 2009-10-06 17:35:26 +0000
> > @@ -788,7 +788,11 @@ static int NEAR_F merge_many_buff(MI_SOR
> > cleanup:
> > close_cached_file(to_file); /* This holds old result */
> > if (to_file == t_file)
> > + {
> > *t_file=t_file2; /* Copy result file */
> > + t_file->current_pos= &t_file->write_pos;
> > + t_file->current_end= &t_file->write_end;
> > + }
>
> Okay.
>
> I think I'd rather add flush_io_cache() instead, but your solution is
> also ok.
I don't understand how flush_io_cache() can affect this. Besides it gets
called _very_ soon after this point of execution.
Regards,
Sergey
--
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
