maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Aleksey!

On Apr 11, Aleksey Midenkov wrote:
> 
> It was a conscious choice. Quantitative characteristic is implied.

This isn't going anywhere.
I've asked Ian (KB technical writer) for his opinion and we'll do what
he'll say.

> > True, I know that my feeling of the language is not perfect, I'm not a
> > native speaker. But it works both ways, you're not a native speaker
> > either.
> >
> > So, if you don't want to change to correct English, there's no point in
> > arguing, let's just ask a native speaker.
> >
> > > > I'd suggest to rewrite as
> > > >
> > > >    if (ot_ctx->vers_create_count)
> > > >      /*
> > > >        already tried to add a partition to this table and failed
> > > >        (because of e.g. lock conflict). Don't try again
> > > >      */
> > > >    else if (table->vers_need_hist_part(thd, table_list))
> > > >     ... and here your code ...
> > >
> > > The comment is wrong. Added correct comment.
> >
> > The comment describes what I've seen in gdb, looking how
> > ot_ctx->vers_create_count could be non-zero. It was your
> >
> >   update t1 set x= x + 122 where x = 1
> >
> > test case, "Locking timeout caused by auto-creation affects original DML"
> > You try to auto-create, it fails because of the lock timeout, you
> > clear_error(), the table is reopened. But because vers_create_count is
> > not reset, it does not try to auto-create again, thus avoiding an
> > infinite loop.
> >
> > What do you thing was wrong there?
> 
> Okay, it is partly true. Failing case is the secondary one.

May be, but it's the only case where ot_ctx->vers_create_count>0 there.
So if there's a "primary" case too, your tests never trigger it.
Add a test for it, please.

> > > I also have to revert DBUG_ASSERT() you suggested which fails
> > > drop_table_force on winx64-debug and add error code condition instead:
> > >
> > > @@ -3268,12 +3271,12 @@ Open_table_context::recover_from_failed_open()
> > >     case OT_DISCOVER:
> > >     case OT_REPAIR:
> > >     case OT_ADD_HISTORY_PARTITION:
> > > -      DBUG_ASSERT(!m_thd->get_stmt_da()->is_set());
> > >       result= lock_table_names(m_thd, m_thd->lex->create_info, m_failed_table,
> > >                                NULL, get_timeout(), 0);
> > >       if (result)
> > >       {
> > > -        if (m_action == OT_ADD_HISTORY_PARTITION)
> > > +        if (m_action == OT_ADD_HISTORY_PARTITION &&
> > > +            m_thd->get_stmt_da()->sql_errno() == ER_LOCK_WAIT_TIMEOUT)
> > >         {
> > >           // MDEV-23642 Locking timeout caused by auto-creation affects original DML
> > >           m_thd->clear_error();
> >
> > What happens on winx64-debug?
> 
> Please look yourself at
> 
> http://buildbot.askmonty.org/buildbot/builders/winx64-debug/builds/24345/steps/test/logs/stdio

Thanks. But either I don't know how to read it correctly or it doesn't
have enough info.

Could you explain what happens on this builder? What error do you see in
stmt_da in recover_from_failed_open() that triggers the assert?

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx