Hi!

On Thu, Apr 16, 2020 at 1:17 PM Sergei Golubchik <s...@mariadb.org> wrote:
>
> Hi, Michael!
>
> On Apr 13, Michael Widenius wrote:
> > revision-id: 3cbe15bd78c (mariadb-10.5.2-120-g3cbe15bd78c)
> > parent(s): d4d332d196d
> > author: Michael Widenius <mo...@mariadb.com>
> > committer: Michael Widenius <mo...@mariadb.com>
> > timestamp: 2020-04-09 01:37:01 +0300
> > message:
> >
> > Fixed core dump in alter table if ADD PARTITION fails
> >
> > I didn't add a test case as to reproduce this we need to
> > have a failed write in on of the engines. Bug found and bug fix
> > verified while debugging S3 and partitions.
> >
> > ---
> >  sql/sql_partition.cc | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
> > index ef8ef5114a8..4e984fa775d 100644
> > --- a/sql/sql_partition.cc
> > +++ b/sql/sql_partition.cc
> > @@ -6817,7 +6817,8 @@ static void 
> > handle_alter_part_error(ALTER_PARTITION_PARAM_TYPE *lpt,
> >    DBUG_ENTER("handle_alter_part_error");
> >    DBUG_ASSERT(table->m_needs_reopen);
> >
> > -  if (close_table)
> > +  /* The table may not be open if ha_partition::change_partitions() failed 
> > */
> > +  if (close_table && !table->file->is_open())
>
> This looks *very* confusing, like you close the table only if it's *not*
> open. But then the whole if() is very confusing, as it closes in both
> branches, so the meaning of "close_table" is totally not clear.

Agree it looks confusing but I am reasonable sure it did stop a crash.
I can't verify that as it was very hard to repeat.

> May be instead of this your fix you'd better cherry-pick
> 9c02b7d6670b069866 from MySQL? It removes close_table completely,
> so I expect it to work for S3 just as you wanted.

I ported the above to MariaDB and I agree it looks this is a better solution.

Regards,
Monty

_______________________________________________
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

Reply via email to