← Back to team overview

maria-developers team mailing list archive

Re: Rev 2731: BUG#39249 Maria:query cache returns out of date results in file:///Users/bell/maria/bzr/work-maria-5.1-querycache/

 

Hi!

Here folllows the review of this patch.

> +++ b/storage/maria/ha_maria.cc 2009-09-23 06:59:53 +0000
<cut>
> @@ -3227,6 +3230,9 @@
>    */
>    *engine_data= 0;
> 
> +  if (file->s->now_transactional && file->s->have_versioning)
> +    return (file->trn->trid != file->s->state.last_change_trn);
> +

Should probably be:

(file->trn->trid >= file->s->state.last_change_trn)

This is becasue the 'trn->trid' will be incremented for each
transaction as long as it's bigger than last_change_trn, the table is
not changed and the current transaction sees that same thing as any
new transaction and it's thus safe to put in query cache.


> --- a/storage/maria/ma_delete.c	2009-01-12 11:12:00 +0000
> +++ b/storage/maria/ma_delete.c	2009-09-23 06:59:53 +0000
> @@ -118,13 +118,6 @@
>    mi_sizestore(lastpos, info->cur_row.lastpos);
>    VOID(_ma_writeinfo(info,WRITEINFO_UPDATE_KEYFILE));
>    allow_break();			/* Allow SIGHUP & SIGINT */
> -  if (info->invalidator != 0)
> -  {
> -    DBUG_PRINT("info", ("invalidator... '%s' (delete)",
> -                        share->open_file_name.str));
> -    (*info->invalidator)(share->open_file_name.str);
> -    info->invalidator=0;
> -  }

Invalidator is used by merge tables;  We don't have merge tables for
Maria yet, but we will have this sooner or later. So why remove the
above ?

IRC talk follows...

Ok, Sanja thought that we will not have Merge tables for Maria, but
this is not the case;  The plan is to replace MyISAM tables with Maria
tables and to do that, we need MERGE tables for Maria too. The main
reason that is not done is just not enough time.

<cut>

> --- a/storage/maria/ma_locking.c	2009-02-09 21:52:42 +0000
> +++ b/storage/maria/ma_locking.c	2009-09-23 06:59:53 +0000
> @@ -214,7 +214,6 @@
>        VOID(_ma_test_if_changed(info));
>  
>        info->lock_type=lock_type;
> -      info->invalidator=share->invalidator;
>        share->w_locks++;
>        share->tot_locks++;
>        break;
> @@ -269,7 +268,6 @@
>      }
>      if (check_keybuffer)
>        VOID(_ma_test_if_changed(info));
> -    info->invalidator=share->invalidator;
>    }
>    else if (lock_type == F_WRLCK && info->lock_type == F_RDLCK)
>    {

Please revert the above and everything else related to invalidator

> === modified file 'storage/maria/ma_state.c'
> --- a/storage/maria/ma_state.c	2008-12-27 02:05:16 +0000
> +++ b/storage/maria/ma_state.c	2009-09-23 06:59:53 +0000
> @@ -318,6 +318,13 @@
>      DBUG_ASSERT(!info->s->base.born_transactional);
>      share->state.state= *info->state;
>      info->state= &share->state.state;
> +#ifdef HAVE_QUERY_CACHE
> +    DBUG_PRINT("info", ("invalidator... '%s' (status update)",
> +                        info->s->data_file_name.str));
> +    DBUG_ASSERT(info->s->chst_invalidator != NULL);
> +    (*info->s->chst_invalidator)((const char *)info->s->data_file_name.str);
> +#endif
> +
>    }
>    info->append_insert_at_end= 0;
>  }
> @@ -469,6 +476,8 @@
>                                       tables->state_start.checksum);
>            history->trid= trn->commit_trid;
>  
> +          share->state.last_change_trn= trn->commit_trid;
> +
>            if (history->next)
>            {
>              /* Remove not visible states */
> 
> === modified file 'storage/maria/ma_update.c'
> --- a/storage/maria/ma_update.c	2008-10-31 23:14:58 +0000
> +++ b/storage/maria/ma_update.c	2009-09-23 06:59:53 +0000
> @@ -187,13 +187,6 @@
>    */
>    VOID(_ma_writeinfo(info, WRITEINFO_UPDATE_KEYFILE));
>    allow_break();				/* Allow SIGHUP & SIGINT */
> -  if (info->invalidator != 0)
> -  {
> -    DBUG_PRINT("info", ("invalidator... '%s' (update)",
> -                        share->open_file_name.str));
> -    (*info->invalidator)(share->open_file_name.str);
> -    info->invalidator=0;
> -  }
>    DBUG_RETURN(0);
>  
>  err:

Revert

> 
> === modified file 'storage/maria/ma_write.c'
> --- a/storage/maria/ma_write.c	2009-02-19 09:01:25 +0000
> +++ b/storage/maria/ma_write.c	2009-09-23 06:59:53 +0000
> @@ -295,13 +295,6 @@
>  
>    info->cur_row.lastpos= filepos;
>    VOID(_ma_writeinfo(info, WRITEINFO_UPDATE_KEYFILE));
> -  if (info->invalidator != 0)
> -  {
> -    DBUG_PRINT("info", ("invalidator... '%s' (update)",
> -                        share->open_file_name.str));
> -    (*info->invalidator)(share->open_file_name.str);
> -    info->invalidator=0;
> -  }

revert


> === modified file 'storage/maria/maria_def.h'
> --- a/storage/maria/maria_def.h	2009-02-19 09:01:25 +0000
> +++ b/storage/maria/maria_def.h	2009-09-23 06:59:53 +0000

<cut>

> @@ -337,7 +338,7 @@
>    /* Mapings to read/write the data file */
>    size_t (*file_read)(MARIA_HA *, uchar *, size_t, my_off_t, myf);
>    size_t (*file_write)(MARIA_HA *, const uchar *, size_t, my_off_t, myf);
> -  invalidator_by_filename invalidator;	/* query cache invalidator */
> +  invalidator_by_filename chst_invalidator; /* query cache invalidator */

I assume you need the old one for Merge tables

>    my_off_t key_del_current;		/* delete links for index pages */
>    ulong this_process;			/* processid */
>    ulong last_process;			/* For table-change-check */
> @@ -507,7 +508,6 @@
>    uint int_nod_flag;			/* -""- */
>    uint32 int_keytree_version;		/* -""- */
>    int (*read_record)(MARIA_HA *, uchar*, MARIA_RECORD_POS);
> -  invalidator_by_filename invalidator;	/* query cache invalidator */

revert.

<cut>

Regards,
Monty



References