MySQL Bugs: #100186: some useless latch on Btree search and modify path

Bug #100186	some useless latch on Btree search and modify path
Submitted:	11 Jul 2020 21:16	Modified:	17 Aug 2020 12:09
Reporter:	Zongzhi Chen (OCA)	Email Updates:
Status:	Can't repeat	Impact on me:	None
Category:	MySQL Server: InnoDB storage engine	Severity:	S3 (Non-critical)
Version:	8.0.*	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
After reading the btree search and modify codebase in detail. I find some block's latch add twice. Even InnoDB's rw_lock support recursion add lock, avoid the useless latch can make the code readable

1. in the btr_cur_search_to_nth_level() function, if the latch_mode == BTR_CONT_MODIFY_TREE, InnoDB get the father block with RW_X_LATCH

```
      if (latch_mode == BTR_CONT_MODIFY_TREE) {
        child_block = btr_block_get(page_id, page_size, RW_X_LATCH, index, mtr);
```

However, before go into btr_cur_search_to_nth_level with latch_mode == BTR_CONT_MODIFY_TREE, the previous search must have latch the father block in it's tree_savepoints

```
      /* x-latch the branch blocks not released yet. */
      for (ulint i = n_releases; i <= n_blocks; i++) {
        mtr_block_x_latch_at_savepoint(mtr, tree_savepoints[i], tree_blocks[i]);
      }
```

I have test and find InnoDB add the latch twice.

2. in btr_cur_search_to_nth_level(), if the latch_mode== BTR_MODIFY_TREE, and the modify node is leaf-node, then it need to latch it's previous and next node.
```
      if (rw_latch == RW_NO_LATCH || srv_read_only_mode) {
        btr_cur_latch_leaves(block, page_id, page_size, latch_mode, cursor,
                             mtr);
      }
```

And in btr_page_split_and_insert => btr_attach_half_pages(), it latch it's previous and next page again
```
  /* for consistency, both blocks should be locked, before change */
  if (prev_page_no != FIL_NULL && direction == FSP_DOWN) {
    prev_block = btr_block_get(page_id_t(space, prev_page_no), block->page.size,
                               RW_X_LATCH, index, mtr);
  }
  if (next_page_no != FIL_NULL && direction != FSP_DOWN) {
    next_block = btr_block_get(page_id_t(space, next_page_no), block->page.size,
                               RW_X_LATCH, index, mtr);
  }
```

Another question is that can we only latch the target node with previous node or next node, since btr_cur_search_to_nth_level can get the direction as btr_attach_half_pages. 

How to repeat:
read the code and test

Hi Mr. zongzhi,

Thank you for your bug report.

You wrote that a test can prove an unnecessary latch lock.

Can you send us that simple test case ???

Thanks in advance.

simple test such as oltp_read_write can trigger unnecessary latch. Actually, every SMO(struct modify operation) will cause the father node unnecessary latch, however, as what I said before, since InnoDB's rw_lock support recursion
add lock, it won't cause deadlock issue..

If you want get the information, you need print the latch information before the code..

Hi Mr. zongzhi,

Can you provide us with a proper patch for 8.0.21 that will show the problem ???

Also, have you prepared a patch for this bug, even an ancillary one ???

Both would be welcome.

No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".

We can't repeat this bug report. Furthermore, we can do without a test case if the report contained a fully detailed code analysis. Some benchmarks would be welcome, as well as a patch, which would allow us to verify this report, without any test case.