Details
-
Bug
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
-
10.5, 10.6, 10.11, 11.4
Description
The data structure trx_t::autoinc_locks is of type ib_vector_t*. It is unconditionally allocated in trx_create() even when no AUTO_INCREMENT locks will be used by the transaction, even for read-only transactions.
We had better make use of a template that was introduced in MDEV-30289:
small_vector<lock_t*, 4> autoinc_locks;
|
The data elements are trivially assignable and allocated separately; the first 8 table locks of a transaction would use the trx->lock.table_pool in lock_table_create(). Any data-modifying transaction will typically also require a table IX lock in addition to the auto-increment lock. That is why 4 would be a reasonable fixed size.
Only if a transaction involved more than 4 auto-increment locks, we would dynamically allocate a larger vector, similar to how the 16-element mtr_t::m_memo works.
Attachments
Issue Links
- is blocked by
-
-
- Closed
-
- relates to
-
-
- Open
-
Activity
| Field | Original Value | New Value |
|---|---|---|
| Link |
This issue is blocked by |
| Link | This issue relates to MDEV-35702 [ MDEV-35702 ] |
| Status | Open [ 1 ] | In Progress [ 3 ] |
| Assignee | Marko Mäkelä [ marko ] | Debarun Banerjee [ JIRAUSER54513 ] |
| Status | In Progress [ 3 ] | In Review [ 10002 ] |
| Assignee | Debarun Banerjee [ JIRAUSER54513 ] | Marko Mäkelä [ marko ] |
| Status | In Review [ 10002 ] | Stalled [ 10000 ] |
| Status | Stalled [ 10000 ] | In Progress [ 3 ] |
| Assignee | Marko Mäkelä [ marko ] | Debarun Banerjee [ JIRAUSER54513 ] |
| Status | In Progress [ 3 ] | In Review [ 10002 ] |
Mainly we are replacing ` ib_vector_t* autoinc_locks; ` with `small_vector`. So, the possible impact mainly would depend on the stability of `small_vector` implementation. I think we need to test it well. I added one comment on passing R value while adding the lock entry which doesn't seem completely right. I need a little more time to complete the review.
markoI am done with the review and should be able to approve once you check/reply/address the comments. I think the chances of regression is from the small_vector implementation.
| Assignee | Debarun Banerjee [ JIRAUSER54513 ] | Marko Mäkelä [ marko ] |
| Status | In Review [ 10002 ] | Stalled [ 10000 ] |
| Status | Stalled [ 10000 ] | In Progress [ 3 ] |
Thank you for the first review. I spent quite some time in an attempt to improve code coverage. I am able to occasionally (nondeterministically) cover the out-of-order lock release in lock_table_remove_autoinc_lock(), but not any AUTO_INCREMENT lock release in lock_cancel_waiting_and_release().
| Assignee | Marko Mäkelä [ marko ] | Debarun Banerjee [ JIRAUSER54513 ] |
| Status | In Progress [ 3 ] | In Review [ 10002 ] |
| Status | In Review [ 10002 ] | Stalled [ 10000 ] |
| Assignee | Debarun Banerjee [ JIRAUSER54513 ] | Marko Mäkelä [ marko ] |
| Fix Version/s | 10.6.21 [ 29953 ] | |
| Fix Version/s | 10.11.11 [ 29954 ] | |
| Fix Version/s | 11.4.5 [ 29956 ] | |
| Fix Version/s | 10.6 [ 24028 ] | |
| Fix Version/s | 10.11 [ 27614 ] | |
| Fix Version/s | 11.4 [ 29301 ] | |
| Resolution | Fixed [ 1 ] | |
| Status | Stalled [ 10000 ] | Closed [ 6 ] |
I was too optimistic; there are some test failures that I will need to address.