Description:
For InnoDB engine, when try to hit query cache, need to call row_search_check_if_query_cache_permitted.
This function use trx_id to check whether the cache result can be saw by current transaction.
```
6426 ibool
6427 row_search_check_if_query_cache_permitted(
6428 /*======================================*/
6429 trx_t* trx, /*!< in: transaction object */
6430 const char* norm_name) /*!< in: concatenation of database name,
6431 '/' char, table name */
6432 {
6433 dict_table_t* table;
6434 ibool ret = FALSE;
6435
6436 table = dict_table_open_on_name(
6437 norm_name, FALSE, FALSE, DICT_ERR_IGNORE_NONE);
6438
6439 if (table == NULL) {
6440
6441 return(FALSE);
6442 }
6443
6444 /* Start the transaction if it is not started yet */
6445
6446 trx_start_if_not_started(trx, false);
6447
6448 /* If there are locks on the table or some trx has invalidated the
6449 cache before this transaction started then this transaction cannot
6450 read/write from/to the cache.
6451
6452 If a read view has not been created for the transaction then it doesn't
6453 really matter what this transactin sees. If a read view was created
6454 then the view low_limit_id is the max trx id that this transaction
6455 saw at the time of the read view creation. */
6456
6457 if (lock_table_get_n_locks(table) == 0
6458 && ((trx->id != 0 && trx->id >= table->query_cache_inv_id)
6459 || !MVCC::is_view_active(trx->read_view)
6460 || trx->read_view->low_limit_id()
6461 >= table->query_cache_inv_id)) {
```
But this is wrong. We can not just use trx_id comparison with query_cache_inv_id which is the last committed trx id to check the visibility. Because the trx_id is the start of rw-transaction, may be when current transaction start, the rw-trx has not been committed.
So, the conditions
```
((trx->id != 0 && trx->id >= table->query_cache_inv_id)
```
and
```
6459 || !MVCC::is_view_active(trx->read_view)
6460 || trx->read_view->low_limit_id()
6461 >= table->query_cache_inv_id)
```
can not guarantee result right.
Another is the dict table may be remove from cache. The table->query_cache_inv_id may be 0 after dict_table_open_on_name. This also may cause result mismatch.
How to repeat:
query_cache_bugs-master.opt
```
--query_cache_type=1
```
query_cache_bugs.test
```
set GLOBAL query_cache_size=1355776;
CREATE TABLE t1(c1 INT);
CREATE TABLE t2(c1 INT);
SET @@GLOBAL.innodb_stats_auto_recalc=OFF;
connect (con1, localhost, root);
connect (con2, localhost, root);
--echo Case1. ReadView low_limit_id check with query_cache_inv_id can not guarantee result correct.
--enable_connect_log
connection con1;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
INSERT INTO t2 VALUES(1);
connection default;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
SELECT * FROM t1;
connection con1;
INSERT INTO t1 VALUES(1);
COMMIT;
connection con2;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
SELECT * FROM t1;
connection default;
SELECT * FROM t1;
SELECT * FROM t1 WHERE 1;
COMMIT;
connection con2;
COMMIT;
RESET QUERY CACHE;
--echo Case2. Trx id check with query_cache_inv_id can not guarantee result correct.
connection default;
BEGIN;
SELECT * FROM t1;
connection con1;
BEGIN;
INSERT INTO t1 VALUES(2);
COMMIT;
connection con2;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
SELECT * FROM t1;
connection default;
#Change to write trx. trx_id is allocated after INSERT.
INSERT INTO t2 VALUES(2);
SELECT * FROM t1;
SELECT * FROM t1 WHERE 1;
COMMIT;
connection con2;
COMMIT;
RESET QUERY CACHE;
#--echo Case3. dict_table remove from cache. The check with query_cache_inv_id can not guarantee result correct.
#set @@global.table_definition_cache=400;
#--let $table_count=405
#--let $table_number=1
#while ($table_number <= $table_count)
#{
# --eval CREATE TABLE tb$table_number (c1 int);
# --inc $table_number
#}
#
#connection default;
#BEGIN;
#SELECT * FROM t2;
#
#connection con1;
#BEGIN;
#INSERT INTO t1 VALUES(10);
#COMMIT;
#
#connection con2;
#BEGIN;
#SELECT * FROM t1;
#COMMIT;
#
#BEGIN;
#--let $table_number=1
#while ($table_number <= $table_count)
#{
# --eval SELECT * FROM tb$table_number;
# --inc $table_number
#}
#
#connection default;
#disconnect con1;
#disconnect con2;
##Wait dict_table removed from cache
#select sleep(5);
#SELECT * FROM t1;
#SELECT * FROM t1 WHERE 1;
#COMMIT;
#
#connection con2;
#COMMIT;
connection default;
disconnect con1;
disconnect con2;
DROP TABLE t1, t2;
```
query_cache_bugs.result
```
set GLOBAL query_cache_size=1355776;
Warnings:
Warning 1287 '@@query_cache_size' is deprecated and will be removed in a future release.
CREATE TABLE t1(c1 INT);
CREATE TABLE t2(c1 INT);
SET @@GLOBAL.innodb_stats_auto_recalc=OFF;
Case1. ReadView low_limit_id check with query_cache_inv_id can not guarantee result correct.
connection con1;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
INSERT INTO t2 VALUES(1);
connection default;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
SELECT * FROM t1;
c1
connection con1;
INSERT INTO t1 VALUES(1);
COMMIT;
connection con2;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
SELECT * FROM t1;
c1
1
connection default;
SELECT * FROM t1; --result mismatch
c1
1
SELECT * FROM t1 WHERE 1;
c1
COMMIT;
connection con2;
COMMIT;
RESET QUERY CACHE;
Warnings:
Warning 1681 'RESET QUERY CACHE' is deprecated and will be removed in a future release.
Case2. Trx id check with query_cache_inv_id can not guarantee result correct.
connection default;
BEGIN;
SELECT * FROM t1;
c1
1
connection con1;
BEGIN;
INSERT INTO t1 VALUES(2);
COMMIT;
connection con2;
SET TRANSACTION_ISOLATION='REPEATABLE-READ';
BEGIN;
SELECT * FROM t1;
c1
1
2
connection default;
INSERT INTO t2 VALUES(2);
SELECT * FROM t1; --result mismatch
c1
1
2
SELECT * FROM t1 WHERE 1;
c1
1
COMMIT;
connection con2;
COMMIT;
RESET QUERY CACHE;
Warnings:
Warning 1681 'RESET QUERY CACHE' is deprecated and will be removed in a future release.
connection default;
disconnect con1;
disconnect con2;
DROP TABLE t1, t2;
```
As I can not make the dict_table may remove from cache in mtr test happen. (Which could happen in independent server), so I disable case3. The reason why dict_table t1 not remove from cache is like below. Owned by thread id 0.
```
mysql> select * from table_handles;
+-------------+---------------+-------------+-----------------------+-----------------+----------------+---------------+---------------+
| OBJECT_TYPE | OBJECT_SCHEMA | OBJECT_NAME | OBJECT_INSTANCE_BEGIN | OWNER_THREAD_ID | OWNER_EVENT_ID | INTERNAL_LOCK | EXTERNAL_LOCK |
+-------------+---------------+-------------+-----------------------+-----------------+----------------+---------------+---------------+
| TABLE | sys | sys_config | 139862459739728 | 0 | 0 | NULL | NULL |
| TABLE | test | t2 | 139862254616416 | 0 | 0 | NULL | NULL |
| TABLE | test | t1 | 139862469611856 | 0 | 0 | NULL | NULL |
| TABLE | test | t1 | 139862254624512 | 0 | 0 | NULL
```
Suggested fix:
Use committed version which is changed when commit trx.
And load new value of trx_sys_get_committed_version when query_cache_inv_id is 0.
diff --git a/storage/innobase/include/read0types.h b/storage/innobase/include/read0types.h
index 49967f289f6..002b66036e8 100644
--- a/storage/innobase/include/read0types.h
+++ b/storage/innobase/include/read0types.h
@@ -249,6 +249,8 @@ public:
return(m_ids.empty());
}
+ ib_id_t get_committed_version() { return m_committed_version; }
+
#ifdef UNIV_DEBUG
/**
@param rhs view to compare with
@@ -326,6 +328,8 @@ private:
they can be removed in purge if not needed by other views */
trx_id_t m_low_limit_no;
+ ib_id_t m_committed_version;
+
/** AC-NL-RO transaction view that has been "closed". */
bool m_closed;
diff --git a/storage/innobase/include/trx0sys.h b/storage/innobase/include/trx0sys.h
index 5b82a9019f1..44e23158349 100644
--- a/storage/innobase/include/trx0sys.h
+++ b/storage/innobase/include/trx0sys.h
@@ -173,6 +173,8 @@ UNIV_INLINE
trx_id_t
trx_sys_get_max_trx_id(void);
/*========================*/
+UNIV_INLINE
+ib_id_t trx_sys_get_committed_version(void);
#ifdef UNIV_DEBUG
/* Flag to control TRX_RSEG_N_SLOTS behavior debugging. */
@@ -579,6 +581,8 @@ struct trx_sys_t {
volatile because it can be accessed
without holding any mutex during
AC-NL-RO view creation. */
+ volatile ib_uint64_t committed_version;
+
trx_ut_list_t serialisation_list;
/*!< Ordered on trx_t::no of all the
currenrtly active RW transactions */
diff --git a/storage/innobase/include/trx0sys.ic b/storage/innobase/include/trx0sys.ic
index 4f3642b4719..69783945023 100644
--- a/storage/innobase/include/trx0sys.ic
+++ b/storage/innobase/include/trx0sys.ic
@@ -474,6 +474,10 @@ trx_sys_get_max_trx_id(void)
#endif /* UNIV_WORD_SIZE < DATA_TRX_ID_LEN */
}
+inline ib_id_t trx_sys_get_committed_version() {
+ return(trx_sys->committed_version);
+}
+
/*****************************************************************//**
Get the number of transaction in the system, independent of their state.
@return count of transactions in trx_sys_t::rw_trx_list */
diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index 1f46f944e08..dadeb103465 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -4473,7 +4473,6 @@ lock_release(
{
lock_t* lock;
ulint count = 0;
- trx_id_t max_trx_id = trx_sys_get_max_trx_id();
ut_ad(lock_mutex_own());
ut_ad(!trx_mutex_own(trx));
@@ -4500,7 +4499,7 @@ lock_release(
block the use of the MySQL query cache for
all currently active transactions. */
- table->query_cache_inv_id = max_trx_id;
+ table->query_cache_inv_id = trx_sys_get_committed_version();
}
lock_table_dequeue(lock);
diff --git a/storage/innobase/read/read0read.cc b/storage/innobase/read/read0read.cc
index 305fd5dc381..8ca6ad646c0 100644
--- a/storage/innobase/read/read0read.cc
+++ b/storage/innobase/read/read0read.cc
@@ -333,7 +333,8 @@ ReadView::ReadView()
m_up_limit_id(),
m_creator_trx_id(),
m_ids(),
- m_low_limit_no()
+ m_low_limit_no(),
+ m_committed_version()
{
ut_d(::memset(&m_view_list, 0x0, sizeof(m_view_list)));
}
@@ -473,6 +474,7 @@ ReadView::prepare(trx_id_t id)
m_low_limit_no = trx->no;
}
}
+ m_committed_version = trx_sys->committed_version;
}
/**
diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc
index c37e7908bc5..bc15f7d80d8 100644
--- a/storage/innobase/row/row0sel.cc
+++ b/storage/innobase/row/row0sel.cc
@@ -6441,6 +6441,8 @@ row_search_check_if_query_cache_permitted(
return(FALSE);
}
+ if (table->query_cache_inv_id == 0)
+ table->query_cache_inv_id = trx_sys_get_committed_version();
/* Start the transaction if it is not started yet */
trx_start_if_not_started(trx, false);
@@ -6455,9 +6457,8 @@ row_search_check_if_query_cache_permitted(
saw at the time of the read view creation. */
if (lock_table_get_n_locks(table) == 0
- && ((trx->id != 0 && trx->id >= table->query_cache_inv_id)
- || !MVCC::is_view_active(trx->read_view)
- || trx->read_view->low_limit_id()
+ && (!MVCC::is_view_active(trx->read_view)
+ || trx->read_view->get_committed_version()
>= table->query_cache_inv_id)) {
ret = TRUE;
diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc
index 30885ae02a0..edc4fb9f116 100644
--- a/storage/innobase/trx/trx0trx.cc
+++ b/storage/innobase/trx/trx0trx.cc
@@ -1905,6 +1905,7 @@ trx_erase_lists(
}
trx_sys->rw_trx_set.erase(TrxTrack(trx->id));
+ trx_sys->committed_version++;
trx_sys_mutex_exit();
}
diff --git a/storage/innobase/trx/trx0sys.cc b/storage/innobase/trx/trx0sys.cc
index 1e553d91373..825451bc60d 100644
--- a/storage/innobase/trx/trx0sys.cc
+++ b/storage/innobase/trx/trx0sys.cc
@@ -533,6 +533,7 @@ trx_sys_init_at_db_start(void)
ib::info() << "Trx id counter is " << trx_sys->max_trx_id;
}
+ trx_sys->committed_version = 0;
trx_sys_mutex_exit();