Percona Server with XtraDB

Suspicious logic in XtraDB extra slow log stats collection

Reported by Alexey Kopytov on 2013-02-13
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Server
Medium
Alexey Kopytov
5.1
Medium
Unassigned
5.5
Medium
Alexey Kopytov

Bug Description

innobase_trx_init() enables stats collection just based on the log_slow_verbosity=innodb:

#ifdef EXTENDED_SLOWLOG
 if (thd_log_slow_verbosity(thd) & (1ULL << SLOG_V_INNODB)) {
  trx->take_stats = TRUE;
 } else {
  trx->take_stats = FALSE;
 }
#else
 trx->take_stats = FALSE;
#endif

Then all the code that wants to update stats check if both the slow log is enabled and trx->take_stats is TRUE:

  if (innobase_get_slow_log() && trx && trx->take_stats && start_time)
  {
   ut_usectime(&sec, &ms);
   finish_time = (ib_uint64_t)sec * 1000000 + ms;
   trx->io_reads_wait_timer += (ulint)(finish_time - start_time);
  }

Why don't we check if the slow log is enabled before setting trx->take_stats? The only downside is a rather pathological case when we have long running transactions, and want to collect stats for them with log_slow_verbosity=innodb without writing to the slow log, and then want all previously collected stats to be reflected in the slow log once it is enabled in the middle of those long-runnign transactions.

The upside is that we would also remove calls to to innobase_get_slow_log(), which are in some cases on critical code paths.

Related branches

lp:~akopytov/percona-server/bug1123921
Merged into lp:percona-server/5.5 at revision 459
Laurynas Biveinis: Approve on 2013-02-26
Stewart Smith (community): Needs Fixing on 2013-02-26
Sergei Glushchenko: Approve (g2) on 2013-02-25
tags: added: slow-extended

The pathological case has its inverse case: when we want to stop collecting stats and have some long running transactions.

The change not to support this is more than reasonable IMHO.

Alexey Kopytov (akopytov) wrote :

Assigning to myself, as this needs to be fixed for internal tests.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers