Suspicious logic in XtraDB extra slow log stats collection

Bug #1123921 reported by Alexey Kopytov on 2013-02-13
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Server moved to
Fix Released
Alexey Kopytov
Won't Fix
Fix Released
Alexey Kopytov

Bug Description

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

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

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

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.

Percona now uses JIRA for bug reports so this bug report is migrated to:

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

Other bug subscribers