maria-developers team mailing list archive

[Michael Widenius <monty@xxxxxxxxxxxx>]

Hi!

>>>>> "knielsen" == knielsen  <knielsen@xxxxxxxxxxxxxxx> writes:

knielsen> #At lp:maria
knielsen>  2702 knielsen@xxxxxxxxxxxxxxx	2009-06-09
knielsen>       XtraDB after-merge fixes.
      
knielsen> === modified file 'mysys/thr_mutex.c'
knielsen> --- a/mysys/thr_mutex.c	2009-02-19 09:01:25 +0000
knielsen> +++ b/mysys/thr_mutex.c	2009-06-09 15:08:46 +0000
knielsen> @@ -149,6 +149,35 @@ static inline void remove_from_active_li
knielsen>    mp->prev= mp->next= 0;
knielsen>  }
 
knielsen> +/*
knielsen> +  We initialise the hashes for deadlock detection lazily.
knielsen> +  This greatly helps with performance when lots of mutexes are initiased but
knielsen> +  only a few of them are actually used (eg. XtraDB).
knielsen> +*/
knielsen> +static int safe_mutex_lazy_init_deadlock_detection(safe_mutex_t *mp)
knielsen> +{
knielsen> +  if (!my_multi_malloc(MY_FAE | MY_WME,
knielsen> +                       &mp->locked_mutex, sizeof(*mp->locked_mutex),
knielsen> +                       &mp->used_mutex, sizeof(*mp->used_mutex), NullS))
knielsen> +  {

Add:
   /* Disable deadlock handling for this mutex */
   mp->flags|= MYF_NO_DEADLOCK_DETECTION;

knielsen> +    return 1;                                   /* Error */
knielsen> +  }
knielsen> +
knielsen> +  pthread_mutex_lock(&THR_LOCK_mutex);
knielsen> +  mp->id= ++safe_mutex_id;
knielsen> +  pthread_mutex_unlock(&THR_LOCK_mutex);
knielsen> +  hash_init(mp->locked_mutex, &my_charset_bin,
knielsen> +            1000,
knielsen> +            offsetof(safe_mutex_deadlock_t, id),
knielsen> +            sizeof(mp->id),
knielsen> +            0, 0, HASH_UNIQUE);
knielsen> +  hash_init(mp->used_mutex, &my_charset_bin,
knielsen> +            1000,
knielsen> +            offsetof(safe_mutex_t, id),
knielsen> +            sizeof(mp->id),
knielsen> +            0, 0, HASH_UNIQUE);
knielsen> +  return 0;
knielsen> +}
 
knielsen>  int safe_mutex_init(safe_mutex_t *mp,
knielsen>  		    const pthread_mutexattr_t *attr __attribute__((unused)),
knielsen> @@ -167,35 +196,8 @@ int safe_mutex_init(safe_mutex_t *mp,
knielsen>    mp->line= line;
knielsen>    /* Skip the very common '&' prefix from the autogenerated name */
knielsen>    mp->name= name[0] == '&' ? name + 1 : name;
knielsen> +  /* Deadlock detection is initialised only lazily, on first use. */
 
knielsen> -  if (safe_mutex_deadlock_detector && !( my_flags & MYF_NO_DEADLOCK_DETECTION))
knielsen> -  {
knielsen> -    if (!my_multi_malloc(MY_FAE | MY_WME,
knielsen> -                         &mp->locked_mutex, sizeof(*mp->locked_mutex),
knielsen> -                         &mp->used_mutex, sizeof(*mp->used_mutex), NullS))
knielsen> -    {
knielsen> -      /* Disable deadlock handling for this mutex */
knielsen> -      my_flags|= MYF_NO_DEADLOCK_DETECTION;
knielsen> -    }
knielsen> -    else
knielsen> -    {
knielsen> -      pthread_mutex_lock(&THR_LOCK_mutex);
knielsen> -      mp->id= ++safe_mutex_id;
knielsen> -      pthread_mutex_unlock(&THR_LOCK_mutex);
knielsen> -      hash_init(mp->locked_mutex, &my_charset_bin,
knielsen> -                1000,
knielsen> -                offsetof(safe_mutex_deadlock_t, id),
knielsen> -                sizeof(mp->id),
knielsen> -                0, 0, HASH_UNIQUE);
knielsen> -      hash_init(mp->used_mutex, &my_charset_bin,
knielsen> -                1000,
knielsen> -                offsetof(safe_mutex_t, id),
knielsen> -                sizeof(mp->id),
knielsen> -                0, 0, HASH_UNIQUE);
knielsen> -    }
knielsen> -  }
knielsen> -  else
knielsen> -    my_flags|= MYF_NO_DEADLOCK_DETECTION;

You accidently removed checking of 'safe_mutex_deadlock_detector here.

Please add:

 if (safe_mutex_deadlock_detector)
   my_flags|= MYF_NO_DEADLOCK_DETECTION;

knielsen>    mp->create_flags= my_flags;

<cut>

knielsen> === modified file 'storage/xtradb/ibuf/ibuf0ibuf.c'
knielsen> --- a/storage/xtradb/ibuf/ibuf0ibuf.c	2009-03-26 06:11:11 +0000
knielsen> +++ b/storage/xtradb/ibuf/ibuf0ibuf.c	2009-06-09 15:08:46 +0000
knielsen> @@ -422,7 +422,12 @@ ibuf_init_at_db_start(void)
knielsen>  	grow in size, as the references on the upper levels of the tree can
knielsen>  	change */
 
knielsen> -	ibuf->max_size = ut_min( buf_pool_get_curr_size() / UNIV_PAGE_SIZE
knielsen> +        /* The default for ibuf_max_size is calculated from the requested
knielsen> +           buffer pool size srv_buf_pool_size, not the actual size as returned
knielsen> +           by buf_pool_get_curr_size(). The latter can differ from the former
knielsen> +           by one page due to alignment requirements, and we do not want a
knielsen> +           user-visible variable like INNODB_IBUF_MAX_SIZE to vary at random. */
knielsen> +	ibuf->max_size = ut_min( srv_buf_pool_size / UNIV_PAGE_SIZE
knielsen>  		/ IBUF_POOL_SIZE_PER_MAX_SIZE, (ulint) srv_ibuf_max_size / UNIV_PAGE_SIZE);
 
knielsen>  	srv_ibuf_max_size = (long long) ibuf->max_size * UNIV_PAGE_SIZE;

Not sure if the above is correct. From MySQL 5.1:


from ../srv/srv0start.c:


             if (srv_use_awe) {
             ....
                ret = buf_pool_init(srv_pool_size, srv_pool_size,
                                    srv_awe_window_size);
        } else {
                ret = buf_pool_init(srv_pool_size, srv_pool_size,
                                    srv_pool_size);

The last argument, n_frames, is used to set
buf_pool->n_frames = n_frames, which is returned from
buf_pool_get_curr_size().

This indicates that if srv_use_awe is used, it would be wrong to use
srv_buf_pool_size instead of buf_pool_get_curr_size().

My suggestion to fix this would be to change the buffer we allocate in
the test case to take into account that it may be rounded down a bit
by adding 4096 to it or just fix the test case to deal with it.

On IRC, we agreed to fix the test case.

Regards,
Monty