maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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.

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?

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

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