Rafal,
> Why local value forces all threads to use the same timeout?
> Local means that each thread (each connection) has a separate
> copy of the timespec structure and
> con2 can not reset timeout of con1. Your argument looks like
> if you are talking about a global timeout value...
>
> > per-thread so that the timeouts can occur at the correct
> moments for
> > each thread. This also holds true for when the global
> timeout is set.
> >
>
> The change which I have in mind is as follows:
>
> > my_bool DDL_blocker_class::check_DDL_blocker(THD *thd) > {
> > int ret = 0;
> > bool timeout= FALSE;
> + struct timespec ddl_timeout;
>
> <...>
>
> > if (thd->variables.ddl_wait_timeout > 0)
> > {
> > + set_timespec(ddl_timeout, thd->variables.ddl_wait_timeout);
> > timeout= TRUE;
> > }
> > else if (global_system_variables.ddl_wait_timeout > 0)
> > {
> > + set_timespec(ddl_timeout,
> global_system_variables.ddl_wait_timeout);
> > timeout= TRUE;
> > }
>
> <...>
>
> > while (DDL_blocked && !thd->DDL_exception && (ret == 0))
> > {
> > if (!timeout)
> > pthread_cond_wait(&COND_DDL_blocker,
> &THR_LOCK_DDL_is_blocked);
> > else
> > ret= pthread_cond_timedwait(&COND_DDL_blocker,
> &THR_LOCK_DDL_is_blocked,
> > + &ddl_timeout);
> > }
>
> <...>
>
> > DBUG_RETURN(FALSE);
> > }
>
> Thus the ddl_timeout structure is local to the
> check_DDL_blocker() method. It is filled there and used to
> time the pthread_cond_wait() call. Obviously, it is
> completely independent from the same structures used in any
> other invocations of
> check_DDL_blocker() which might run concurrently in other
> threads. Therefore the timeouts in different threads will be
> independent from each other without polluting the THD
> structure with new members.
You make a sound agrument and I see your point. However, this is one where I
think neither implementation is wrong and therefore I will leave in THD. My
only (remaining) reason to place it in THD is because that's where the other
timeout variables are defined and I personally think that is where it should
be. I don't like to rely on local copies of variables -- it's just a
practice of mine that is borne from past (bad) experiences.
>
> (cut)
> >>> (uchar**) &opt_date_time_formats[MYSQL_TIMESTAMP_DATETIME],
> >>> (uchar**) &opt_date_time_formats[MYSQL_TIMESTAMP_DATETIME],
> >>> 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
> >>> - { "default_week_format", OPT_DEFAULT_WEEK_FORMAT,
> >>> + {"ddl_wait_timeout", OPT_DDL_WAIT_TIMEOUT,
> >>> + "The number of seconds a DDL can be blocked before
> cancelled.",
> >>> + (uchar**) &global_system_variables.ddl_wait_timeout,
> >>> + (uchar**) &max_system_variables.ddl_wait_timeout, 0,
> GET_ULONG,
> >>> + REQUIRED_ARG, DDL_WAIT_TIMEOUT, 0, 1024 * 1024 *
> 1024, 0, 1, 0},
> >> Could you shortly explain what the entries mean? This is
> to help me
> >> learn something here and to check that you know what you are doing.
> >
> > See my_getopt.h @line#43:
> >
> > struct my_option
> > {
> > const char *name; /* Name of the option */
> > int id; /* unique id or
> short option */
> > const char *comment; /* option comment,
> for autom. --help
> > */
> > uchar **value; /* The variable value */
> > uchar **u_max_value; /* The user def.
> max variable value
> > */
> > struct st_typelib *typelib; /* Pointer to
> possible values */
> > ulong var_type;
> > enum get_opt_arg_type arg_type;
> > longlong def_value; /* Default value */
> > longlong min_value; /* Min allowed value */
> > longlong max_value; /* Max allowed value */
> > longlong sub_size; /* Subtract this
> from given value */
> > long block_size; /* Value should be
> a mult. of this
> > */
> > void *app_type; /* To be used by an
> application */
> > };
> >
>
> Thanks for reference. After seeing this I have two more questions.
>
> 1. Why do you use REQUIRED_ARG as the value of arg_type
> member? Shouldn't it be NO_ARG instead?
So that when specified on the command line a value is given. See:
c:\mysql\bin>mysqld -uroot --ddl_wait_timeout
080311 8:50:09 [ERROR] mysqld: option '--ddl_wait_timeout' requires an
argument
...but this may need to change pending Peter's suggestion and our mutual
acceptance of the desired behaviour.
> 2. Where max_system_variables.ddl_wait_timeout is initialized?
See line#3431 in mysqld.cc.
