The query cache protected all operations with one exclusive lock
(m_cache_lock_status, guarded by structure_guard_mutex). This
serialized every lookup, even though a lookup mainly reads the
query cache structures.
Added a new global variable, query_cache_use_rw_lock (default OFF).
When set, send_result_to_client() takes a shared (read) lock instead
of an exclusive one, which allows lookups to run concurrently. Having
it as an option makes it possible to benchmark both alternatives with
the same binary and then decide which one to keep.
Implementation:
- try_lock() has a new 'read_lock' argument. The new inline function
try_read_lock() sets it from query_cache_use_rw_lock.
- Readers are counted in m_readers and leave m_cache_lock_status as
UNLOCKED. Writers wait until m_readers is 0 and are counted in
m_waiting_writers. Waiting writers have priority over new readers,
which ensures that invalidations are not starved by cache hits.
- Readers wait on the new COND_cache_read_lock. wake_up_waiters()
wakes one waiting writer, or all waiting readers if there is none.
- unlock() is split into unlock() and unlock_internal(). The kind of
lock to release is deduced from m_cache_lock_status, so none of the
unlock() callers had to be changed.
- send_result_to_client() updates the query list and the statistics
under structure_guard_mutex when it only holds a read lock. With a
write lock this is done as before, without taking the mutex.
- If an engine requests invalidation during a lookup done with a read
lock, the table key is copied and the invalidation is done after
the lock is released, as invalidation requires a write lock.
- Added the DEBUG_SYNC point "in_query_cache_hit" used by the new test.
With query_cache_use_rw_lock=0 the code works as before.
Did a simple sysbench run with 64 threads and 200000 simple select queries,
all served from query cache. query_cache_use_rw_lock=1 gave a 2.25x
speedup. The difference to an unmodied MariaDB version for the same
test is 2.3 x faster.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Added query_cache_type == ALWAYS_OFF to turn of query cache permanently
for new open tables. Without this option storage engines has to call
query cache for all update querys and invalidate used tables that has
a small overhead. When running benchmarks without query cache, this
is the option to use!
- Updated MyISAM, Aria, MYISAM_MRG and InnoDB to support ALWAYS_OFF.
- Setting query_cache_size in a config file will not automatically enable
the query cache.
- Added query_cache_available() that functions can check if query cache
can be used or if it is permanently disabled.
- Moved things around in Query_cache::store_query() to get more work done
outside of query cache mutex.
- Same for Query_cache::send_result_to_client()
Co-author: Thirunarayanan Balathandayuthapani <thiru@mariadb.com>
- ÌnnoDB changes reviewed and improved
- Do checks inline before calling query_cache.send_result_to_client()
Things tried that failed:
- I tried removing calling lex_start() and reset_for_next_command() if
query is cached, but this is would cause duplicated, hard to maintain
code and would require double resets of things if query cache was
not used. I have documented in sql_parse.cc what would need to be
reset for this approach to work.
Add SQL_CACHE=0|1 option for tables to disable/enable query caches if
table is part of the query.
SQL_CACHE option can be disable for a table by using SQL_CACHE=DEFAULT
Also provide a workaround for the problem described at:
MDEV-6631 "No cache directive(SQL_CACHE) query still wait for
query cache lock" on query_cache_type=DEMAND mode"
by adding DEMAND_STRICT mode.
The new mode is needed as modifying how DEMAND works would made the
change backward incompatible.
query_cache_type has now following options (0-2 are same as before)
OFF (0) = Query cache off
ON (1) = Query cache on, all queries are cached except queries marked
with SQL_NO_CACHE
DEMAND (2) = Query cache on, only queries marked with SQL_CACHE are cached
DEMAND_STRICT
(3) = Query cache on, only queries starting with SELECT SQL_CACHE
are cached. This avoids a query cache lock+lookup for every
query.
TABLES (4) = Query cache on, only cache queries marked with SQL_CACHE or
where all used tables are using the create option SQL_CACHE=1
Note that if any of the tables used in the query has the create option
SQL_CACHE=0 or is a system table, temporary table or other non
cacheable table, then the query will not be cached.
SHOW CREATE TABLE prints SQL_CACHE=0|1 when the option is set. For tables
where the option can never apply (temporary tables, tables in the mysql
database, and engines that do not support the query cache) it is printed
as /* SQL_CACHE=1 */.
co-author: Seunguck Lee
- Took some changes related to MDEV-6631 from his patch.
Caused by Bug#988 in 702b5ad.
How it works
QUERY blocks has all its tables in the array coming after
Query_cache_block header. Each such table element
(Query_cache_block_table) is also an element of doubly-linked circular
list associated with TABLE block used to find all queries by a
table. TABLE block data() is actually Query_cache_table: identificator
for table elements by real name. Every element in the circular list
has `parent` set to such Query_cache_table, except the list root. The
list root is the only Query_cache_block_table element owned by a TABLE
block, all other elements are owned by QUERY blocks. Thus TABLE block
n_tables is always 1.
FREE blocks are transformed into gap pointed by `border`. All the
QUERY and TABLE blocks are moved in place of the gap providing free
space defragmentation. Moving is done by creating new block at the
location of the gap and then doing memmove() from an old block to new
block. This may involve blocks overlap when the distance between the
blocks is less than the block size.
What was broken
When the old and the new QUERY blocks overlap, pointer can be in range
of both of them and checking the range does not guarantee affiliation
of the pointer with the old block. Such intra-block checks added by
Bug#988 can wrongly work on an already updated pointer.
In the test case small gap allows both tables 16 and 17 to be in the
overlapping region of old and new blocks. Iteration 16 updated
`prev->next` on the normal manner and the `prev` was table 17.
Iteration 17 sees this `next` as a pointer coming from an old block
though really it is already for a new block (as small gap shift allows
table 16 to be in both blocks), and it shifts it to the wrong
location. That have broken TABLE block circular chain.
How it was fixed
The fix modifies intra-block checks to work only in the forward
direction. F.ex., current table jx references intra-block by `next`
another table jy in later iteration (jx < jy) and we shift this `next`
pointer. Then by normal code (pre-Bug#988) we update `next->prev`. At
iteration jy we skip intra-block check for `prev` as it is in the past
iteration and was updated by iteration jx. The same happens for the
opposite direction, there is no semantic difference between `next` and
`prev` pointers.
When allocate_block() have failed Query_cache::insert_table() misses
to init node->parent which is then accessed with outdated data. There
is check in Query_cache::register_all_tables():
if (block_table->parent)
unlink_table(block_table);
So unlink_table() is avoided when parent is 0.
Partial commit of the greater MDEV-34348 scope.
MDEV-34348: MariaDB is violating clang-16 -Wcast-function-type-strict
Change the type of my_hash_get_key to:
1) Return const
2) Change the context parameter to be const void*
Also fix casting in hash adjacent areas.
Reviewed By:
============
Marko Mäkelä <marko.makela@mariadb.com>
What's happening:
1. Query_cache::insert() locks the QC and verifies that it's enabled
2. parallel thread tries to disable it. trylock fails (QC is locked)
so the status becomes DISABLE_REQUEST
3. Query_cache::insert() calls Query_cache::write_result_data()
which allocates a new block and unlocks the QC.
4. Query_cache::unlock() notices there are no more QC users and a
pending DISABLE_REQUEST so it disables the QC and frees all the
memory, including the new block that was just allocated
5. Query_cache::write_result_data() proceeds to write into the freed block
Fix: change m_cache_status under a mutex.
Approved by Oleksandr Byelkin <sanja@mariadb.com>
- Changing the global function ok_for_lower_case_names()
into a method in class Lex_ident_fs.
- Changing a few functions/methods to get the database name
as a "const LEX_CSTRING" instead of a "const char *".
All these functions/methods use ok_for_lower_case_names()
inside. This change helps to avoid new strlen() calls, and also
removes a few old strlen() calls.
In commit 28325b0863
a compile-time option was introduced to disable the macros
DBUG_ENTER and DBUG_RETURN or DBUG_VOID_RETURN.
The parameter name WITH_DBUG_TRACE would hint that it also
covers DBUG_PRINT statements. Let us do that: WITH_DBUG_TRACE=OFF
shall disable DBUG_PRINT() as well.
A few InnoDB recovery tests used to check that some output from
DBUG_PRINT("ib_log", ...) is present. We can live without those checks.
Reviewed by: Vladislav Vaintroub
Making changes to wsrep_mysqld.h causes large parts of server code to
be recompiled. The reason is that wsrep_mysqld.h is included by
sql_class.h, even tough very little of wsrep_mysqld.h is needed in
sql_class.h. This commit introduces a new header file, wsrep_on.h,
which is meant to be included from sql_class.h, and contains only
macros and variable declarations used to determine whether wsrep is
enabled.
Also, header wsrep.h should only contain definitions that are also
used outside of sql/. Therefore, move WSREP_TO_ISOLATION* and
WSREP_SYNC_WAIT macros to wsrep_mysqld.h.
Reviewed-by: Jan Lindström <jan.lindstrom@mariadb.com>
The problem happened because the the new client capability flag
CLIENT_EXTENDED_METADATA was not put into the cache entry key.
So results cached by a new client were sent to the old client (and vica versa)
with a mis-matching metadata, which made the client abort the connection on
an unexpected result set metadata packet format.
The problem was caused by the patch for:
MDEV-17832 Protocol: extensions for Pluggable types and JSON, GEOMETRY
which forgot to adjust the query cache code.
Fix:
- Adding a new member Query_cache_query_flags::client_extended_metadata,
so only clients with equal CLIENT_EXTENDED_METADATA flag values can
reuse results.
- Adding a new column CLIENT_EXTENDED_METADATA into
INFORMATION_SCHEMA.QUERY_CACHE_INFO (privided by the qc_info plugin).
Use < TL_FIRST_WRITE for determining a READ transaction.
Use TL_FIRST_WRITE as the relative operator replacing TL_WRITE_ALLOW_WRITE
as the minimium WRITE lock type.
- Rewrote bool Query_compressed_log_event::write() to make it more readable
(no logic changes).
- Changed DBUG_PRINT of 'is_error:' to 'is_error():' to make it easier to
find error: in traces.
- Ensure that 'db' is never null in Query_log_event (Simplified code).
