List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:May 17 2011 11:58am
Subject:Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3620) Bug#38813
Bug#11749345
View as plain text  
Hi Dmitry,

On 5/16/11 7:17 AM, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/mysql-5.1-bug11749345/ based on
> revid:tatjana.nuernberg@stripped
>
>   3620 Dmitry Shulga	2011-05-16
>        Fixed bug#11749345 (formerly bug#38813) - increasing memory consumption
>        when selecting from I_S and views exist, in SP.
>
>        Symptoms: re-execution of prepared statement (or statement in a stored
>        routine) which read from one of I_S tables and which in order to fill
>        this I_S table had to open a view led to increasing memory consumption.
>
>        What happened in this situation was that during the process of view
>        opening for purpose of I_S filling view-related structures (like its
>        LEX) were allocated on persistent MEM_ROOT of prepared statement (or
>        stored routine). Since this MEM_ROOT is not freed until prepared
>        statement deallocation (or expulsion of stored routine from the cache)
>        and code responsible for filling I_S is not able to re-use results of
>        view opening from previous executions this allocation ended up in
>        memory hogging.
>
>        This patch solves the problem by ensuring that when a view opened
>        for the purpose of I_S filling all its structures are allocated on
>        non-persistent runtime MEM_ROOT. This is achieved by activating a
>        temporary Query_arena bound to this MEM_ROOT.
>        Since this step makes impossible linking of view structures into
>        LEX of our prepared statement (or stored routine statement) this
>        patch also changes code filling I_S table to install a proxy LEX
>        before trying to open a view or a table. Consequently some code
>        which was responsible for backing-up/restoring parts of LEX when
>        view/table was opened during filling of I_S table became redundant
>        and was removed.
>

OK to push. Some minor comments below. Thanks for working on this.

[...]

> -  lex->all_selects_list= tables->schema_select_lex;
>     /*
> -    Restore thd->temporary_tables to be able to process
> -    temporary tables(only for 'show index'&  'show columns').
> -    This should be changed when processing of temporary tables for
> -    I_S tables will be done.
> +    When a view is opened its structures are allocated on permanent

"on a permanent"

> +    statement arena and linked into LEX tree for the current statement

"into the LEX"

> +    (this happens even in cases when view is handled through TEMPTABLE
> +    algorithm).
> +
> +    To prevent this process from unnecessary hogging of memory in permanent

"in the permanent"

> +    arena of our I_S query and to avoid damaging its LEX we use temporary
> +    arena and LEX for table/view opening.
> +
> +    Use temporary arena instead of statement permanent arena. Also make
> +    it active arena and save original one for successive restoring.
>     */
> -  thd->temporary_tables= open_tables_state_backup->temporary_tables;
> +  old_arena= thd->stmt_arena;
> +  thd->stmt_arena=&i_s_arena;
> +  thd->set_n_backup_active_arena(&i_s_arena,&backup_arena);
> +
> +  /* Prepare temporary LEX. */
> +  thd->lex= lex=&temp_lex;
> +  lex_start(thd);
> +
> +  /* Disable constant subquery evaluation as we won't be locking tables. */
> +  lex->context_analysis_only= CONTEXT_ANALYSIS_ONLY_VIEW;
> +
> +  /*
> +    Some of process_table() functions rely on wildcard being passed from
> +    old LEX (or at least being initialized).
> +  */
> +  lex->wild= old_lex->wild;
> +
> +  /*
> +    Strings in db_name and table_name might be changed by make_table_list()
> +    function. In order to pass their unaltered values to process_table()
> +    call we save them here.
> +  */
> +  if (!thd->make_lex_string(&orig_db_name, db_name->str,
> +                            db_name->length, FALSE) ||
> +      !thd->make_lex_string(&orig_table_name, table_name->str,
> +                            table_name->length, FALSE))
> +    goto end;
> +
> +  /*
> +    Create table list element for table to be open. Link it with the
> +    temporary LEX. The latter is required to correctly open views and
> +    produce table describing their structure.
> +  */
> +  if (make_table_list(thd,&lex->select_lex, db_name, table_name))
> +    goto end;
> +
> +  table_list= lex->select_lex.table_list.first;
> +
> +  if (is_show_fields_or_keys)
> +  {
> +    /*
> +      Restore thd->temporary_tables to be able to process
> +      temporary tables(only for 'show index' & 'show columns').

"tables (only"

> +      This should be changed when processing of temporary tables for
> +      I_S tables will be done.
> +    */
> +    thd->temporary_tables= open_tables_state_backup->temporary_tables;
> +  }
> +  else
> +  {
> +    /*
> +      Apply optimization flags for table opening which are relevant for
> +      this I_S table. We can't do this for SHOW COLUMNS/KEYS because of
> +      backward compatibility.
> +    */
> +    table_list->i_s_requested_object= schema_table->i_s_requested_object;
> +  }
> +

[...]

> +  if (!is_show_fields_or_keys&&  result&& 
> thd->is_error()&&
> +      thd->main_da.sql_errno() == ER_NO_SUCH_TABLE)
> +  {
> +    /*
> +      Hide error for not existing table.

"for a non-existing table"

> +      This error can occur for example when we use
> +      where condition with db name and table name and this
> +      table does not exist.

"For example, this error can occur when we use a where condition with a 
db name and table, but the table does not exist."

> +
> +  /*
> +    For safety reset list of open temporary tables before closing
> +    all tables open within this Open_tables_state.
> +  */
> +  thd->temporary_tables= 0;

0 -> NULL

> +  close_thread_tables(thd);
> +

Otherwise, looks good.

Regards,

Davi
Thread
bzr commit into mysql-5.1 branch (Dmitry.Shulga:3620) Bug#38813 Bug#11749345Dmitry Shulga16 May
  • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3620)Bug#38813 Bug#11749345Dmitry Lenev16 May
  • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3620) Bug#38813Bug#11749345Davi Arnaut19 May