Bug #98949 The count_os_wait in rw_lock_s_lock_spin could be wrong

• Submitted: 13 Mar 2020 21:20
• Modified: 16 Mar 2020 9:00
• Reporter: Zongzhi Chen (OCA)
• Status: Not a Bug
• Category: MySQL Server: InnoDB storage engine
• Severity: S3 (Non-critical)
• Version: 8.0.*
• OS: Any
• CPU Architecture: Any

[13 Mar 2020 21:20] Zongzhi Chen

Description:
In the code  rw_lock_s_lock_spin

when we count the number of wait before we actually get the rw_lock, in the code below:

```
 328   while (i < srv_n_spin_wait_rounds && lock->lock_word <= 0) {
 329     if (srv_spin_wait_delay) {
 330       ut_delay(ut_rnd_interval(0, srv_spin_wait_delay));
 331     }
 332
 333     i++;
 334   }

   //  comment here:
  //  In this case, it possible that i >= srv_n_spin_wait_rounds, and lock->lock_word > 0
  //  The in our logic, even it has yield, but the count_os_wait won't increase.
  //  Then in the next line, if rw_lock_s_lock_low success, then it find that count_os_wait == 0, 
  //  and don't add the count to lock->count_os_wait..

 336   if (i >= srv_n_spin_wait_rounds) {
 337     os_thread_yield();
 338   }
 339
 340   ++spin_count;
 341
 342   /* We try once again to obtain the lock */
 343   if (rw_lock_s_lock_low(lock, pass, file_name, line)) {
 344     if (count_os_wait > 0) {
 345       lock->count_os_wait += static_cast<uint32_t>(count_os_wait);
 346       rw_lock_stats.rw_s_os_wait_count.add(count_os_wait);
 347     }
 348
 349     rw_lock_stats.rw_s_spin_round_count.add(spin_count);
 350
 351     return; /* Success */
 352   } else {
 353     if (i < srv_n_spin_wait_rounds) {
 354       goto lock_loop;
 355     }
 356
 357     ++count_os_wait;

```

How to repeat:
Read the code and test

Suggested fix:
  add the count_os_wait++ in this brace

  if (i >= srv_n_spin_wait_rounds) {
    os_thread_yield();
    count_os_wait++
  }

and delete the count_os_wait++ in line 357

[14 Mar 2020 12:08] Marcin Babij

Thank You for your report.
The "os_thread_yield" is not the "os wait" the counter counts.
It is supposed to count the calls to "os_event_wait_low" called from "sync_array_wait_event" called from the branch that starts with the "++count_os_wait" you specified to be moved.
Therefore it's not a bug.

[16 Mar 2020 9:00] MySQL Verification Team

Closing as !bug according developer prior comment.