[Maria-developers] Rev 2731: BUG#39249 Maria:query cache returns out of date results in file:///Users/bell/maria/bzr/work-maria-5.1-querycache/
At file:///Users/bell/maria/bzr/work-maria-5.1-querycache/ ------------------------------------------------------------ revno: 2731 revision-id: sanja@askmonty.org-20090923065953-l23fsjfy8lx3hkm4 parent: knielsen@knielsen-hq.org-20090907131358-6bq1e3qfcgi30hu8 committer: sanja@askmonty.org branch nick: work-maria-5.1-querycache timestamp: Wed 2009-09-23 09:59:53 +0300 message: BUG#39249 Maria:query cache returns out of date results BUG#41098 Query Cache returns wrong result with concurent insert === modified file 'BUILD/SETUP.sh' --- a/BUILD/SETUP.sh 2009-05-11 15:31:30 +0000 +++ b/BUILD/SETUP.sh 2009-09-23 06:59:53 +0000 @@ -172,6 +172,7 @@ max_no_embedded_configs="$SSL_LIBRARY --with-plugins=max" +max_no_qc_configs="$SSL_LIBRARY --with-plugins=max --without-query-cache" max_no_ndb_configs="$SSL_LIBRARY --with-plugins=max-no-ndb --with-embedded-server --with-libevent" max_configs="$SSL_LIBRARY --with-plugins=max --with-embedded-server -with-libevent" # Disable NDB in maria max builds === added file 'BUILD/compile-pentium-debug-max-no-qc' --- a/BUILD/compile-pentium-debug-max-no-qc 1970-01-01 00:00:00 +0000 +++ b/BUILD/compile-pentium-debug-max-no-qc 2009-09-23 06:59:53 +0000 @@ -0,0 +1,9 @@ +#! /bin/sh + +path=`dirname $0` +. "$path/SETUP.sh" + +extra_flags="$pentium_cflags $debug_cflags" +extra_configs="$pentium_configs $debug_configs $max_no_qc_configs" + +. "$path/FINISH.sh" === modified file 'sql/mysql_priv.h' --- a/sql/mysql_priv.h 2009-09-03 14:56:46 +0000 +++ b/sql/mysql_priv.h 2009-09-23 06:59:53 +0000 @@ -933,7 +933,6 @@ #define query_cache_abort(A) #define query_cache_end_of_result(A) -#define query_cache_invalidate_by_MyISAM_filename_ref NULL #define query_cache_maybe_disabled(T) 1 #define query_cache_is_cacheable_query(L) 0 #endif /*HAVE_QUERY_CACHE*/ === modified file 'storage/maria/ha_maria.cc' --- a/storage/maria/ha_maria.cc 2009-06-29 21:03:30 +0000 +++ b/storage/maria/ha_maria.cc 2009-09-23 06:59:53 +0000 @@ -28,6 +28,7 @@ #include <my_bit.h> #include "ha_maria.h" #include "trnman_public.h" +#include "trnman.h" C_MODE_START #include "maria_def.h" @@ -907,6 +908,8 @@ if (!(file= maria_open(name, mode, test_if_locked | HA_OPEN_FROM_SQL_LAYER))) return (my_errno ? my_errno : -1); + file->s->chst_invalidator= query_cache_invalidate_by_MyISAM_filename_ref; + if (test_if_locked & (HA_OPEN_IGNORE_IF_LOCKED | HA_OPEN_TMP_TABLE)) VOID(maria_extra(file, HA_EXTRA_NO_WAIT_LOCK, 0)); @@ -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); + /* If a concurrent INSERT has happened just before the currently processed SELECT statement, the total size of the table is unknown. === modified file 'storage/maria/ma_delete.c' --- 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; - } DBUG_RETURN(0); err: === modified file 'storage/maria/ma_locking.c' --- 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) { === 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: === 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; - } /* Update status of the table. We need to do so after each row write === 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 @@ -83,6 +83,7 @@ pgcache_page_no_t first_bitmap_with_space; ulonglong auto_increment; TrID create_trid; /* Minum trid for file */ + TrID last_change_trn; /* selfdescriptive */ ulong update_count; /* Updated for each write lock */ ulong status; double *rec_per_key_part; @@ -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 */ 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 */ ulonglong last_auto_increment; /* auto value at start of statement */ ulong this_unique; /* uniq filenumber or thread */ ulong last_unique; /* last unique number */ === modified file 'storage/myisam/ha_myisam.cc' --- a/storage/myisam/ha_myisam.cc 2009-06-29 21:03:30 +0000 +++ b/storage/myisam/ha_myisam.cc 2009-09-23 06:59:53 +0000 @@ -660,6 +660,9 @@ if (!(file=mi_open(name, mode, test_if_locked | HA_OPEN_FROM_SQL_LAYER))) return (my_errno ? my_errno : -1); + + file->s->chst_invalidator= query_cache_invalidate_by_MyISAM_filename_ref; + if (!table->s->tmp_table) /* No need to perform a check for tmp table */ { if ((my_errno= table2myisam(table, &keyinfo, &recinfo, &recs))) === modified file 'storage/myisam/mi_locking.c' --- a/storage/myisam/mi_locking.c 2009-09-03 14:05:38 +0000 +++ b/storage/myisam/mi_locking.c 2009-09-23 06:59:53 +0000 @@ -329,6 +329,12 @@ #endif info->s->state.state= *info->state; info->state= &info->s->state.state; +#ifdef HAVE_QUERY_CACHE + DBUG_PRINT("info", ("invalidator... '%s' (status update)", + info->filename)); + DBUG_ASSERT(info->s->chst_invalidator != NULL); + (*info->s->chst_invalidator)((const char *)info->filename); +#endif } info->append_insert_at_end= 0; === modified file 'storage/myisam/myisamdef.h' --- a/storage/myisam/myisamdef.h 2009-04-25 09:04:38 +0000 +++ b/storage/myisam/myisamdef.h 2009-09-23 06:59:53 +0000 @@ -190,7 +190,10 @@ const uchar *record, my_off_t pos); size_t (*file_read) (MI_INFO *, uchar *, size_t, my_off_t, myf); size_t (*file_write) (MI_INFO *, const uchar *, size_t, my_off_t, myf); - invalidator_by_filename invalidator; /* query cache invalidator */ + /* query cache invalidator for merged tables */ + invalidator_by_filename invalidator; + /* query cache invalidator for changing state */ + invalidator_by_filename chst_invalidator; ulong this_process; /* processid */ ulong last_process; /* For table-change-check */ ulong last_version; /* Version on start */
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
participants (2)
-
Michael Widenius -
sanja＠askmonty.org