[MDEV-31185] rw_trx_hash_t::find() unpins pins too early Created: 2023-05-04 Updated: 2023-09-21 Resolved: 2023-05-19 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Storage Engine - InnoDB |
| Affects Version/s: | 10.5 |
| Fix Version/s: | 10.4.31, 10.5.22, 10.6.15, 10.9.8, 10.10.6, 10.11.5, 11.0.3, 11.1.2 |
| Type: | Bug | Priority: | Major |
| Reporter: | Vladislav Lesin | Assignee: | Vladislav Lesin |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | parallelslave | ||
| Attachments: |
|
||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||
| Description |
|
Take a look rw_trx_hash_t::find():
It acquires element->mutex, then unpins transaction pins. After that the "element" can be deallocated and reused by some other thread. If we take a look rw_trx_hash_t::insert()->lf_hash_insert()->lf_alloc_new() calls, we will not find any element->mutex acquisition, as it was not initialized yet before it's allocation. My assumption is that rw_trx_hash_t::insert() can easily reuse the chunk, unpinned in rw_trx_hash_t::find(). The scenario is the following: 1. Thread 1 have just executed lf_hash_search() in rw_trx_hash_t::find(), but have not acquired element->mutex yet. The fix is to invoke "lf_hash_search_unpin(pins);" after "mutex_exit(&element->mutex);" call in rw_trx_hash_t::find(). The above scenario is indirectly confirmed with the following trick. If we set one my_sleep(1) before mutex_enter(&element->mutex) call in rw_trx_hash_t::find(), another my_sleep(1) after lf_hash_search_unpin(pins) call in rw_trx_hash_t::find(), then the assertion failure is reproduced much more faster with the test case rpl_debug.test To reproduce it, jut run the test with several instances in a loop, like:
Without the trick, described above, it can take up to 12 hours to reproduce it, with the trick it's reproduces with several minutes. The following comments can also be useful for bug analyses: 1, 2. UPD: the scenario is completely confirmed with rr trace, recorded with the above delays and rpl_debug.test |
| Comments |
| Comment by Vladislav Lesin [ 2023-05-11 ] | ||||||||||||||||
|
The following stable test case proves the above scenario: trx_sys_t_find_lf_hash_error.diff | ||||||||||||||||
| Comment by Vladislav Lesin [ 2023-05-11 ] | ||||||||||||||||
|
Some comment from Marko how it can influence | ||||||||||||||||
| Comment by Vladislav Lesin [ 2023-05-12 ] | ||||||||||||||||
|
The bug is quite ancient, and the question is why we still did not catch it during RQG testing. The explanation can be the following. lf_pinbox_free() invokes lf_pinbox_real_free() only on "if (pins->purgatory_count % LF_PURGATORY_SIZE == 0)" condition, i.e. once per 100 elements in the purgatory. And the probability that trx_sys.rw_trx_hash.hash.alloc.top points to the same element as were unpinned in rw_trx_hash_t::find() is quite low, as lf_pinbox_real_free() pushes pointer of the first element of linked list of unpinned elements gathered from all the thread purgatories at trx_sys.rw_trx_hash.hash.alloc.top(see alloc_free() and pinbox->free_func(first, last, pinbox->free_func_arg) call in lf_pinbox_real_free()). I.e. the unpinned in rw_trx_hash_t::find() element can more likely be not at the head of the list. At the other hand, lf_pinbox_real_free() can be invoked not only from lf_pinbox_free(), but also from lf_pinbox_put_pins(). The rpl_debug.test
I.e. the bug is more reproducible in the case when "XA commit" is executed by slave thread. But our RQG testing does not test replication well. | ||||||||||||||||
| Comment by Marko Mäkelä [ 2023-05-16 ] | ||||||||||||||||
|
The fix looks OK to me. I did not try to run the test with and without the fix. In the test, you could write CREATE TABLE…STATS_PERSISTENT=0 so that there is no need to SET GLOBAL innodb_stats_persistent=OFF. |