Comment 17 for bug 791030

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

It is not clear why the committed fix for this bug gathers a bit mask all of X waiters.

The original 5.5 code is

 if (UNIV_UNLIKELY(rw_lock_get_writer(&btr_search_latch) != RW_LOCK_NOT_LOCKED)
     && trx->has_search_latch) {

  /* There is an x-latch request on the adaptive hash index:
  release the s-latch to reduce starvation and wait for
  BTR_SEA_TIMEOUT rounds before trying to keep it again over
  calls from MySQL */

rw_lock_s_unlock(&btr_search_latch);
trx->has_search_latch = FALSE;

The current XtraDB 5.5 code is

 should_release = 0;
 for (i = 0; i < btr_search_index_num; i++) {
  /* we should check all latches (fix Bug#791030) */
  if (UNIV_UNLIKELY(rw_lock_get_writer(btr_search_latch_part[i])
      != RW_LOCK_NOT_LOCKED)) {
   should_release |= ((ulint)1 << i);
  }
 }

 if (UNIV_UNLIKELY(should_release)) {

  for (i = 0; i < btr_search_index_num; i++) {
   /* we should release all s-latches (fix Bug#791030) */
   if (trx->has_search_latch & ((ulint)1 << i)) {
    rw_lock_s_unlock(btr_search_latch_part[i]);
    trx->has_search_latch &= (~((ulint)1 << i));
   }
  }

Thus, it checks all the latches for the X waiters and collects a bitmask of them. That bitmask is only used if any bit is set, and if yes, the S latches are released for which the current transaction thinks it has the latch for. The individual bit info in the should_release is not used at all. The should_release and trx->has_search_latch bitmasks are not identical.

It can be rewritten as

 should_release = false;
 for (i = 0; i < btr_search_index_num; i++) {
  /* we should check all latches (fix Bug#791030) */
  if (UNIV_UNLIKELY(rw_lock_get_writer(btr_search_latch_part[i])
      != RW_LOCK_NOT_LOCKED)) {
   should_release = true;
   break;
  }
 }

        if (UNIV_UNLIKELY(should_release) {
                trx_search_latch_release_if_reserved();