maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei,

On Sun, Apr 11, 2021 at 7:21 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Apr 11, Aleksey Midenkov wrote:
> > On Sat, Apr 10, 2021 at 11:12 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> > > On Apr 09, Aleksey Midenkov wrote:
> > > > > >
> > > > > > --echo # Increment from 3 to 5
> > > > > > --echo # Increment from 3 to 6, manual names, LOCK TABLES
> > > > > > --echo # Multiple increments in single command
> > > > > >
> > > > > > Besides "increment" is correct because PARTITIONS number is
> > > > > > incremented.
> > > > >
> > > > > Sure.
> > > > > "Increment the number of partitions", this is fine.
> > > > > "Auto create partitions" is also fine.
> > > > > "Increment partitions" is meaningless.
> > > >
> > > > It is obvious from the context that we are talking about the number,
> > > > not partitions themselves. Treat "partitions" as PARTITIONS keyword
> > > > and the increment is attached to a number right next to it. That's
> > > > quite a sense, isn't it?
> > >
> > > No, I think it's a meaningless combination of words.
> > > But let's ask native speakers, shall we?
> >
> > The ability to imply things and make special terms is the freedom of
> > any language I suppose. Slang (or "special language") helps us to
> > describe technical entities more concisely.
>
> :)
> I feel it's just incorrect use of words and I shudder every time I see
> it.

It was a conscious choice. Quantitative characteristic is implied.

>
> 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.

>
> > 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

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



-- 
All the best,

Aleksey Midenkov
@midenok