Skip to content

Commit

Permalink
[bypass] Fix bypass thd->lock leak
Browse files Browse the repository at this point in the history
Summary:
In MySQL bypass, if we decide to fallback *after* lock_table (typically when we see complex conditions such as multiple AND field > N AND field > M, etc), the table will be locked again which and it'll leak lock and actually triggers assert in debug build.

MySQL relies on per-handler THR_LOCK_DATA (you can think it as the lock request) and often per table share THR_LOCK (the actual lock that tracks the read/read_wait/write/write_wait lists) to track more granular table level locks in addition to metadata lock.

If the lock gets leaked, what will happen is THR_LOCK_DATA on the table/handler (such as ha_rocksdb::m_db_lock) will be left on in the THR_LOCK (such as Rdb_table_handler::m_thr_lock), and this can cause many different issues:
* the THR_LOCK_DATA::next will point to itself if the same table is locked again on read lock
* the THR_LOCK_DATA::next will be updated to the write lock if the table is locked for write, essentially linking the read list and write list together, leading to further corruptions
* when the table and its THR_LOCK_DATA gets released, the THR_LOCK_DATA instance will obviously become dangling pointer

In practice this often leads to deadlock in replication thread and instance gets flagged by automation due to lagging behind too much.

Interestingly 5.6 doesn't have this issue because it has a check `if (is_query_tables_locked)` before the lock, so we are doing the same in 8.0 as well.

We should also see if the THR_LOCK are actually needed in MyRocks as InnoDB doesn't use it anymore since 5.7. See https://dev.mysql.com/worklog/task/?id=6671

Reviewed By: luqun

Differential Revision: D36765784

-----------------------------------------------------------------------

Clean up test output files for rocksdb.bypass_select_unsupported

Summary:
Running the test on repeat fails because output files were not cleaned
up after the run.

Reviewed By: bladepan

Differential Revision: D45715828
  • Loading branch information
yizhang82 authored and Herman Lee committed Nov 18, 2023
1 parent d2a18dd commit 7ff6bfc
Show file tree
Hide file tree
Showing 5 changed files with 1,108 additions and 406 deletions.
Loading

0 comments on commit 7ff6bfc

Please sign in to comment.