maria-discuss team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Monty,

> diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl
> index 55543e529a6..54dc0aa17cc 100755
> --- a/mysql-test/mysql-test-run.pl
> +++ b/mysql-test/mysql-test-run.pl
> @@ -4469,6 +4469,7 @@ sub extract_warning_lines ($$) {
>       qr/InnoDB: See also */,
>       qr/InnoDB: Cannot open .*ib_buffer_pool.* for reading: No such file or directory*/,
>       qr/InnoDB: Table .*mysql.*innodb_table_stats.* not found./,
> +     qr/Changed limits: max_open_files/,
I wonder how did you get this warning?

>       qr/InnoDB: User stopword table .* does not exist./
>
>      );
> diff --git a/sql/mysqld.cc b/sql/mysqld.cc
> index b21d50a20fe..b3414279ac9 100644
> --- a/sql/mysqld.cc
> +++ b/sql/mysqld.cc
> @@ -4446,11 +4446,18 @@ static int init_common_variables()
>
>    /* connections and databases needs lots of files */
>    {
> -    uint files, wanted_files, max_open_files;
> +    uint files, wanted_files, max_open_files, min_tc_size, extra_files,
> +      min_connections;
> +    ulong org_max_connections= max_connections, org_tc_size= tc_size;
>
> +    /* Number of files reserved for temporary files */
> +    extra_files= 30;
> +    min_connections= 10;
>      /* MyISAM requires two file handles per table. */
> -    wanted_files= (10 + max_connections + extra_max_connections +
> +    wanted_files= (extra_files + max_connections + extra_max_connections +
>                     tc_size * 2);
> +    min_tc_size= MY_MIN(tc_size, TABLE_OPEN_CACHE_MIN);
> +
>      /*
>        We are trying to allocate no less than max_connections*5 file
>        handles (i.e. we are trying to set the limit so that they will
I believe the following is easier to read:

    ulong org_max_connections= max_connections;
    ulong org_tc_size= tc_size;
    ulong min_tc_size= MY_MIN(tc_size, TABLE_OPEN_CACHE_MIN);
    /* Number of files reserved for temporary files */
    uint extra_files= 30;
    uint min_connections= 10;
    /* MyISAM requires two file handles per table. */
    uint wanted_files= extra_files + max_connections + extra_max_connections +
                       tc_size * 2;
    /*
      We are trying to allocate no less than max_connections*5 file
      handles (i.e. we are trying to set the limit so that they will
      be available).  In addition, we allocate no less than how much
      was already allocated.  However below we report a warning and
      recompute values only if we got less file handles than were
      explicitly requested.  No warning and re-computation occur if we
      can't get max_connections*5 but still got no less than was
      requested (value of wanted_files).
    */
    uint max_open_files= MY_MAX(MY_MAX(wanted_files, (max_connections +
                                                      extra_max_connections)*5),
                                open_files_limit);
    uint files= my_set_max_open_files(max_open_files);

> @@ -4463,43 +4470,43 @@ static int init_common_variables()
>      */
>      max_open_files= MY_MAX(MY_MAX(wanted_files,
>                              (max_connections + extra_max_connections)*5),
> -                        open_files_limit);
> +                           open_files_limit);
>      files= my_set_max_open_files(max_open_files);
>
> -    if (files < wanted_files)
> -    {
> -      if (!open_files_limit || IS_SYSVAR_AUTOSIZE(&open_files_limit))
> -      {
> -        /*
> -          If we have requested too much file handles than we bring
> -          max_connections in supported bounds.
> -        */
> -        SYSVAR_AUTOSIZE(max_connections,
> -           (ulong) MY_MIN(files-10-TABLE_OPEN_CACHE_MIN*2, max_connections));
> -        /*
> -          Decrease tc_size according to max_connections, but
> -          not below TABLE_OPEN_CACHE_MIN.  Outer MY_MIN() ensures that we
> -          never increase tc_size automatically (that could
> -          happen if max_connections is decreased above).
> -        */
> -        SYSVAR_AUTOSIZE(tc_size,
> -                        (ulong) MY_MIN(MY_MAX((files - 10 - max_connections) / 2,
> -                                              TABLE_OPEN_CACHE_MIN), tc_size));
> -	DBUG_PRINT("warning",
> -		   ("Changed limits: max_open_files: %u  max_connections: %ld  table_cache: %ld",
> -		    files, max_connections, tc_size));
> -	if (global_system_variables.log_warnings > 1)
> -	  sql_print_warning("Changed limits: max_open_files: %u  max_connections: %ld  table_cache: %ld",
> -			files, max_connections, tc_size);
> -      }
> -      else if (global_system_variables.log_warnings)
> -	sql_print_warning("Could not increase number of max_open_files to more than %u (request: %u)", files, wanted_files);
> -    }
> -    SYSVAR_AUTOSIZE(open_files_limit, files);
> +    if (files < wanted_files && global_system_variables.log_warnings)
> +      sql_print_warning("Could not increase number of max_open_files to more than %u (request: %u)", files, wanted_files);
> +
> +    /*
> +      If we have requested too much file handles than we bring
> +      max_connections in supported bounds. Still leave at least
> +      'min_connections' connections
> +    */
> +    SYSVAR_AUTOSIZE(max_connections,
> +                    (ulong) MY_MAX(MY_MIN(files- extra_files- min_tc_size*2,
> +                                          max_connections), min_connections));
> +    /*
> +      Decrease tc_size according to max_connections, but
> +      not below min_tc_size.  Outer MY_MIN() ensures that we
> +      never increase tc_size automatically (that could
> +      happen if max_connections is decreased above).
> +    */
> +    SYSVAR_AUTOSIZE(tc_size,
> +                    (ulong) MY_MIN(MY_MAX((files - extra_files -
> +                                           max_connections) / 2, min_tc_size),
> +                                   tc_size));
> +    DBUG_PRINT("warning",
> +               ("Current limits: max_open_files: %u  max_connections: %ld  table_cache: %ld",
> +                files, max_connections, tc_size));
> +    if (global_system_variables.log_warnings > 1 &&
> +        (max_connections < org_max_connections ||
> +         tc_size < org_tc_size))
> +      sql_print_warning("Changed limits: max_open_files: %u  max_connections: %lu (%lu)  table_cache: %lu (%lu)",
I'd say this format is quite confusing. Probably add something like "was %lu"
in brackets?

> +			files, max_connections, org_max_connections,
> +                        tc_size, org_tc_size);
I think the above code must be executed under "if (files < wanted_files)".
We don't really want to "autosize" max_connections and tc_size if we got
enough file descriptors.

Also you missed to "autosize" open_files_limit. I think it should be done
"if (open_files_limit != files) SYSVAR_AUTOSIZE(open_files_limit, files);".

>    }
>
>    /*
> -    Max_connections is now set.
> +    Max_connections and tc_cache is now set.
s/is/are/

>      Now we can fix other variables depending on this variable.
>    */
>
> diff --git a/sql/sql_const.h b/sql/sql_const.h
> index 65742235bee..e28a0649f04 100644
> --- a/sql/sql_const.h
> +++ b/sql/sql_const.h
> @@ -127,7 +127,7 @@
>  #define MAX_FIELDS_BEFORE_HASH	32
>  #define USER_VARS_HASH_SIZE     16
>  #define SEQUENCES_HASH_SIZE     16
> -#define TABLE_OPEN_CACHE_MIN    400
> +#define TABLE_OPEN_CACHE_MIN    200
>  #define TABLE_OPEN_CACHE_DEFAULT 2000
>  #define TABLE_DEF_CACHE_DEFAULT 400
>  /**
> diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
> index de9806ab289..41317a5c259 100644
> --- a/sql/sys_vars.cc
> +++ b/sql/sys_vars.cc
> @@ -3477,7 +3477,7 @@ static bool fix_table_open_cache(sys_var *, THD *, enum_var_type)
>  static Sys_var_ulong Sys_table_cache_size(
>         "table_open_cache", "The number of cached open tables",
>         GLOBAL_VAR(tc_size), CMD_LINE(REQUIRED_ARG),
> -       VALID_RANGE(1, 1024*1024), DEFAULT(TABLE_OPEN_CACHE_DEFAULT),
> +       VALID_RANGE(10, 1024*1024), DEFAULT(TABLE_OPEN_CACHE_DEFAULT),
Why? There're at least a few test cases that do table_open_cache=1:
partition_open_files_limit, sp, myisam.

>         BLOCK_SIZE(1), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
>         ON_UPDATE(fix_table_open_cache));
>

Regards,
Sergey