RE: bk commit into 6.0 tree (cbell:1.2613) BUG#36323

[Chuck Bell]

Hi,

Thanks for the review. New patch is ready for another look:

http://lists.mysql.com/commits/46463

Chuck

> > +result_t Backup::cleanup()
> > +{
> > +  DBUG_ENTER("Default_backup::cleanup()");
> > +  DBUG_PRINT("backup",("Default driver - stop backup"));
> > +  mode= CANCEL;
> > +  if (hdl)
> > +    end_tbl_read();
> > +    hdl= NULL;
> Don't you need braces here? Anyway as you set hdl to NULL in 
> Backup::end_tbl_read(), you don't need to do it here again.

I removed this one and left the one in end_tbl_read().

> > +  if (locking_thd)
> > +    locking_thd->kill_locking_thread();
> > +  DBUG_RETURN(OK);
> >  }
> Great! I believe that's exactly what I requested when I was 
> reviewing BUG#35249. So there is no need to report new bug anymore.

Ok. New patch for BUG#35249 coming later.

> > @@ -363,6 +399,9 @@ result_t Backup::get_data(Buffer &buf)
> >      cur_blob= 0;
> >      cur_table->use_all_columns();
> >      last_read_res = hdl->rnd_next(cur_table->record[0]);
> > +    while ((last_read_res == HA_ERR_RECORD_DELETED) &&
> > +           (last_read_res != HA_ERR_END_OF_FILE))
> > +      last_read_res = hdl->rnd_next(cur_table->record[0]);
> >      DBUG_EXECUTE_IF("SLEEP_DRIVER", sleep(4););
> >      /*
> >        If we are end of file, stop the read process and signal the
> Hey! This is subject of another bug, why do you change it 
> with this patch?
> See my comments regarding this change in review for BUG#35249.

What the?!?!? Great catch! I've had BK issues lately...a patch must've
gotten crossed. Either that or it was operator error. :)

> > +  if (tables_open)
> > +  {
> > +    if (hdl)
> > +    {
> > +      default_backup::Backup::end_tbl_read();
> > +      hdl= NULL;
> > +    }
> Don't you set hdl to NULL in end_tbl_read()?

No, because snapshot inherits from default driver. So I removed this
assignment.