Hi Varun,


On Sun, Apr 12, 2020 at 09:08:37PM +0530, Varun wrote:
> revision-id: 3ead2cea95c32b7ceaf6e6ec81f7afbd9137cfe9 (mariadb-10.2.31-103-g3ead2cea95c)
> parent(s): d565895bbd8e2a163be48b9bac51fccbf3949c80
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2020-04-12 21:05:36 +0530
> message:
>
> MDEV-13266: Race condition in ANALYZE TABLE / statistics collection
>
> Fixing a race condition while collecting the engine independent statistics.
> The issue here was when the statistics was collected on specific indexes
> then because of some race condition the statistics for indexes was not
> collected. The TABLE::keys_in_use_for_query was set to 0 in such cases.
>
> This happens when the table is opened from TABLE_SHARE instead of the
> table share stored in table_cache.

It would be nice to write down the race condition that we've caught (in case we
need this info later)

Is my understanding correct: the error scenario is as follows:

Thread1> start running "ANALYZE TABLE t PERISTENT FOR COLUMNS (..) INDEXES ($list)"
Thread1> Walk through $list and save it in TABLE::keys_in_use_for_query

Thread1> Close/re-open tables
Thread2> Make some use of table t. This involves taking table t from
Thread2> the table cache, and putting it back (with
         TABLE::keys_in_use_for_query reset to 0)

Thread1> continue collecting EITS stats. Since TABLE::keys_in_use_for_query we
will not collect statistics for indexes in $list.

Please confirm (and if not, describe the race condition).

The patch also introduces this behaviour:

analyze table ... persistent for ... indexes(no_such_index);

will now cause engine statistics to be still collected. Before the patch
it exited with an error.

But then, this is consistent with what happens for

analyze table ... persistent for columns(no_such_column) ...

So I guess this is ok.

> ---
>  sql/sql_admin.cc | 46 +++++++++++++++++++++-------------------------
>  1 file changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc
> index ab95fdc340c..0613495f202 100644
> --- a/sql/sql_admin.cc
> +++ b/sql/sql_admin.cc
> @@ -769,31 +769,6 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
>          (table->table->s->table_category == TABLE_CATEGORY_USER &&
>           (get_use_stat_tables_mode(thd) > NEVER ||
>            lex->with_persistent_for_clause));
> -
> -
> -      if (!lex->index_list)
> -      {
> -        tab->keys_in_use_for_query.init(tab->s->keys);
> -      }
> -      else
> -      {
> -        int pos;
> -        LEX_STRING *index_name;
> -        List_iterator_fast<LEX_STRING> it(*lex->index_list);
> -   
> -        tab->keys_in_use_for_query.clear_all(); 
> -        while ((index_name= it++))
> -     {
> -          if (tab->s->keynames.type_names == 0 ||
> -              (pos= find_type(&tab->s->keynames, index_name->str,
> -                              index_name->length, 1)) <= 0)
> -          {
> -            compl_result_code= result_code= HA_ADMIN_INVALID;
> -            break;
> -          }
> -          tab->keys_in_use_for_query.set_bit(--pos);
> -        } 
> -      }
>      }
> 
>      if (result_code == HA_ADMIN_OK)
> @@ -878,6 +853,27 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
>            }
>            tab->file->column_bitmaps_signal();
>          }
> +        if (!lex->index_list)
> +          tab->keys_in_use_for_query.init(tab->s->keys);
> +        else
> +        {
> +          int pos;
> +          LEX_STRING *index_name;
> +          List_iterator_fast<LEX_STRING> it(*lex->index_list);
> +
> +          tab->keys_in_use_for_query.clear_all();
> +          while ((index_name= it++))
> +          {
> +            if (tab->s->keynames.type_names == 0 ||
> +                (pos= find_type(&tab->s->keynames, index_name->str,
> +                                index_name->length, 1)) <= 0)
> +            {
> +              compl_result_code= result_code= HA_ADMIN_INVALID;
> +              break;
> +            }
> +            tab->keys_in_use_for_query.set_bit(--pos);
> +          }
> +        }
>          if (!(compl_result_code=
>                alloc_statistics_for_table(thd, table->table)) &&
>              !(compl_result_code=
> _______________________________________________
> commits mailing list
> commits@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

BR
 Sergei
--
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog