| Bug #37416 | When SQL_NO_CACHE is used, MySQL still lookup into the query cache | ||
|---|---|---|---|
| Submitted: | 16 Jun 2008 0:56 | Modified: | 19 Mar 4:07 |
| Reporter: | jocelyn fournier (Silver Quality Contributor) | ||
| Status: | Closed | ||
| Category: | Server: Query Cache | Severity: | S5 (Performance) |
| Version: | 5.0.51a,5.1 | OS: | Any |
| Assigned to: | Kristofer Pettersson | Target Version: | 5.0+ |
| Tags: | qc | ||
| Triage: | Triaged: D3 (Medium) | ||
[16 Jun 2008 0:56]
jocelyn fournier
[16 Jun 2008 9:14]
jocelyn fournier
Hi,
The cache lookup is confirmed in debug mode :
dispatch_command: query: SELECT SQL_NO_CACHE * FROM t2 WHERE a=1
alloc_root: enter: root: 0x2354eb8
alloc_root: exit: ptr: 0x238d8b8
Query_cache::send_result_to_client: qcache: database: 'test' length: 4
Query_cache::send_result_to_client: qcache: long 1, 4.1: 1, more results 0, pkt_nr: 1, CS
client: 8, CS result: 8, CS conn: 8, limit: 18446744073709551615, TZ: 0xbed380, sql mode:
0x0, sort len: 1024, conncat len: 1024, div_precision: 4, def_week_frmt: 0
Query_cache::send_result_to_client: qcache: No query in query hash or no results
Query_cache::send_result_to_client: lock: 1349 struct unlock...
Query_cache::send_result_to_client: lock: struct unlock OK
However later in the process we have :
Query_cache::is_cacheable: qcache: not interesting query: 0 or not cacheable, options 40
0 type: 1
It seems to correspond to following comment in the code in sql_parse.cc / mysql_parse :
"/*
Warning.
The purpose of query_cache_send_result_to_client() is to lookup the
query in the query cache first, to avoid parsing and executing it.
So, the natural implementation would be to:
- first, call query_cache_send_result_to_client,
- second, if caching failed, initialise the lexical and syntactic parser.
The problem is that the query cache depends on a clean initialization
of (among others) lex->safe_to_cache_query and thd->server_status,
which are reset respectively in
- lex_start()
- mysql_reset_thd_for_next_command()
So, initializing the lexical analyser *before* using the query cache
is required for the cache to work properly.
FIXME: cleanup the dependencies in the code to simplify this.
*/
lex_start(thd);
mysql_reset_thd_for_next_command(thd);
if (query_cache_send_result_to_client(thd, (char*) inBuf, length) <= 0)"
Here lex_start(thd) sets thd->lex->safe_to_cache_query to 1, so even if we have a
SQL_NO_CACHE statement, since the lexical parser has not yet run at this point, MySQL
checks if the result is in its cache.
Would it be possible to slightly modify lex_start to check if "thd->query" contains the
"SELECT SQL_NO_CACHE" string and initialize thd->lex->safe_to_cache_query accordingly ?
(assuming it will not impact too much performances of standard queries)
[19 Jun 2008 14:27]
Susanne Ebrecht
Verified as described by using test from above and 5.1 bzr tree. SELECT SQL_NO_CACHE * FROM t2 WHERE a=1; show profile; | checking query cache for query | 0.000074 | "checking query chache for query" shouldn't be here. SELECT * FROM t2 WHERE a=1; show profile; | checking query cache for query | 0.000067 | SELECT * FROM t2 WHERE a=1; show profile; | checking query cache for query | 0.000017 | When you disable query cache totally by setting query_cache_size=0 then show profiling don't has a "checking query cache for query".
[3 Jul 2008 22:28]
jocelyn fournier
Hi, Note this issue also occurs if the qcache if configured in "DEMAND" mode (ie, SQL_NO_CACHE keyword or no keyword still *does* a lookup in the cache). Thanks, Jocelyn
[4 Jul 2008 16:10]
Sergei Golubchik
The server looks in the query cache *before the parser* - it cannot know that the query contains SQL_NO_CACHE modifier.
[4 Jul 2008 16:16]
jocelyn fournier
Hi Sergei, That's why I added this comment : "Would it be possible to slightly modify lex_start to check if "thd->query" contains the "SELECT SQL_NO_CACHE" string and initialize thd->lex->safe_to_cache_query accordingly ? (assuming it will not impact too much performances of standard queries)" Doing this modification (detecting SQL_NO_CACHE / SQL_CACHE keyword during lex_start) would perhaps make more sense for the "ON DEMAND" mode ? (since there should probably be a higher probability to have a normal query than a "SQL_CACHE" query). Thanks, Jocelyn
[8 Oct 2008 11:22]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/55708 2693 Kristofer Pettersson 2008-10-08 Bug#37416 When SQL_NO_CACHE is used, MySQL still lookup into the query cache The query cache module did not check for the SQL_NO_CACHE keyword before attempting to query the hash lookup table. This had a small performance impact. By introducing a check on the query string before obtaining the hash mutex we can gain some performance if the SQL_NO_CACHE directive is used often.
[14 Oct 2008 16:07]
Kristofer Pettersson
Email system is refusing connection so I'm pasting the new commit below:
=== modified file 'sql/sql_cache.cc'
--- sql/sql_cache.cc 2008-07-24 14:14:34 +0000
+++ sql/sql_cache.cc 2008-10-14 13:38:32 +0000
@@ -363,6 +363,43 @@
array_elements(query_cache_type_names)-1,"", query_cache_type_names, NULL
};
+
+/**
+ Helper function for determine if a SELECT statement has a SQL_NO_CACHE
+ directive.
+
+ @param sql A pointer to the first white space character after SELECT
+
+ @return
+ @retval TRUE The character string contains SQL_NO_CACHE
+ @retval FALSE No directive found.
+*/
+
+static bool has_no_cache_directive(char *sql)
+{
+ int i=0;
+ while (sql[i] == ' ')
+ ++i;
+
+ if (my_toupper(system_charset_info, sql[i]) == 'S' &&
+ my_toupper(system_charset_info, sql[i+1]) == 'Q' &&
+ my_toupper(system_charset_info, sql[i+2]) == 'L' &&
+ my_toupper(system_charset_info, sql[i+3]) == '_' &&
+ my_toupper(system_charset_info, sql[i+4]) == 'N' &&
+ my_toupper(system_charset_info, sql[i+5]) == 'O' &&
+ my_toupper(system_charset_info, sql[i+6]) == '_' &&
+ my_toupper(system_charset_info, sql[i+7]) == 'C' &&
+ my_toupper(system_charset_info, sql[i+8]) == 'A' &&
+ my_toupper(system_charset_info, sql[i+9]) == 'C' &&
+ my_toupper(system_charset_info, sql[i+10]) == 'H' &&
+ my_toupper(system_charset_info, sql[i+11]) == 'E' &&
+ my_toupper(system_charset_info, sql[i+12]) == ' ')
+ return TRUE;
+
+ return FALSE;
+}
+
+
/*****************************************************************************
Query_cache_block_table method(s)
*****************************************************************************/
@@ -1085,6 +1122,16 @@
DBUG_PRINT("qcache", ("The statement is not a SELECT; Not cached"));
goto err;
}
+
+ if (query_length > 20 && has_no_cache_directive(&sql[i+6]))
+ {
+ /*
+ We do not increase 'refused' statistics here since it will be done
+ later when the query is parsed.
+ */
+ DBUG_PRINT("qcache", ("The statement has a SQL_NO_CACHE directive"));
+ goto err;
+ }
}
#ifdef __WIN__
[2 Mar 17:09]
Guillaume Giroux
would the above patch also work for queries using the "no comment" mode, i.e.: SELECT /*!SQL_NO_CACHE*/ foo from bar; ?
[3 Mar 9:58]
Kristofer Pettersson
No, the query cache intercepts the incoming queries before the parser engine. The patch is not "complete" in a sense that it works in a generic way for all kinds of different modes, but on the other hand I don't see why that should be part of the requirements. If a more advanced analysis is needed I think we seriously need to think about implementing a hook for a parser engine plugin in QC. Not because it is difficult to iteratively add new features, but because it is doomed to evolve into blobware really fast if we don't make it more structured.
[5 Mar 11:28]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/68338 2825 Kristofer Pettersson 2009-03-05 Bug#37416 When SQL_NO_CACHE is used, MySQL still lookup into the query cache The query cache module did not check for the SQL_NO_CACHE keyword before attempting to query the hash lookup table. This had a small performance impact. By introducing a check on the query string before obtaining the hash mutex we can gain some performance if the SQL_NO_CACHE directive is used often. @ sql/sql_cache.cc * Introduced new helper function, has_no_cache_directive, for checking the existance of a SQL_NO_CACHE directive before actual parsing of the query.
[9 Mar 9:43]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/68591 2837 He Zhenxing 2009-03-09 [merge] Auto merge
[9 Mar 9:44]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/68592 2768 He Zhenxing 2009-03-09 [merge] Auto merge
[9 Mar 15:14]
Bugs System
Pushed into 5.0.79 (revid:joro@sun.com-20090309135922-a0di9ebkxoj4d4wv) (version source revid:zhenxing.he@sun.com-20090309084418-d3q0f21q2f8jv09i) (merge vers: 5.0.79) (pib:6)
[13 Mar 3:01]
Paul DuBois
Noted in 5.0.79 changelog. The parser now checks whether a SELECT statement begins with SQL_NO_CACHE to determine whether it can skip checking for the query result in the query cache. Setting report to NDI pending push into 5.1.x/6.0.x.
[13 Mar 20:05]
Bugs System
Pushed into 5.1.33 (revid:joro@sun.com-20090313111355-7bsi1hgkvrg8pdds) (version source revid:zhou.li@sun.com-20090311061050-ihp0g77znonq1tuq) (merge vers: 5.1.33) (pib:6)
[13 Mar 20:31]
Guillaume Giroux
I think the /*!{version} SQL_NO_CACHE */ syntax should be considered for this optimization
as well - because mysqldump uses this syntax unconditionally - so it could speed it up
slightly, expecially when a large quantity of tables are dumped.
Projects that aim for multiple db-backends support also use that syntax frequently.
[13 Mar 21:31]
Paul DuBois
Noted in 5.1.33 changelog. Setting report to NDI pending push into 6.0.x.
[18 Mar 14:17]
Bugs System
Pushed into 6.0.11-alpha (revid:joro@sun.com-20090318122208-1b5kvg6zeb4hxwp9) (version source revid:matthias.leich@sun.com-20090310140952-gwtoq87wykhji3zi) (merge vers: 6.0.11-alpha) (pib:6)
[18 Mar 15:31]
Paul DuBois
Noted in 6.0.11 changelog.
[19 Mar 1:10]
James Day
Paul, please change the release note from: The parser now checks whether a SELECT statement begins with SQL_NO_CACHE to determine whether it can skip checking for the query result in the query cache. to: The query cache now checks whether a SELECT statement begins with SQL_NO_CACHE to determine whether it can skip checking for the query result in the query cache. This is not supported when SQL_NO_CACHE is within a comment. The reason for this is that the parser part would imply that full parser rules would be used, while the query cache is before the parser, so gives some clue that full rules may not apply. Then I added the lack of support in comment to specify just what doesn't work.
[19 Mar 4:07]
Paul DuBois
Revised changelog entry per James Day wording.
[9 May 18:44]
Bugs System
Pushed into 5.1.34-ndb-6.2.18 (revid:jonas@mysql.com-20090508185236-p9b3as7qyauybefl) (version source revid:jonas@mysql.com-20090508100057-30ote4xggi4nq14v) (merge vers: 5.1.33-ndb-6.2.18) (pib:6)
[9 May 19:41]
Bugs System
Pushed into 5.1.34-ndb-6.3.25 (revid:jonas@mysql.com-20090509063138-1u3q3v09wnn2txyt) (version source revid:jonas@mysql.com-20090508175813-s6yele2z3oh6o99z) (merge vers: 5.1.33-ndb-6.3.25) (pib:6)
[9 May 20:38]
Bugs System
Pushed into 5.1.34-ndb-7.0.6 (revid:jonas@mysql.com-20090509154927-im9a7g846c6u1hzc) (version source revid:jonas@mysql.com-20090509073226-09bljakh9eppogec) (merge vers: 5.1.33-ndb-7.0.6) (pib:6)
