maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Nirbhay!

On Sep 24, Nirbhay Choubey wrote:
> revision-id: 12ffe70831c9c69c3d6ef83431443696455c332b (mariadb-10.1.17-23-g12ffe70)
> parent(s): 8ffded0a78c0a4912b32acac4c3f58f04c3bcd87
> author: Nirbhay Choubey
> committer: Nirbhay Choubey
> timestamp: 2016-09-24 00:27:38 -0400
> message:
> 
> MDEV-9416: MariaDB galera got signal 11 when altering table add unique index
> 
> When a BF thread attempts to abort a victim thread's transaction,
> the victim thread is not locked and thus its not safe to rely on
> its data structures like htons registered for the trx.
> 
> So, instead of getting the registered htons from victim, innodb's
> hton can be looked up directly from installed_htons[] and used to
> abort the transaction. (Same technique is used in older versions)
> 
> diff --git a/sql/handler.cc b/sql/handler.cc
> index 4e4c8fa..6a32fec 100644
> --- a/sql/handler.cc
> +++ b/sql/handler.cc
> @@ -6108,29 +6108,16 @@ int ha_abort_transaction(THD *bf_thd, THD *victim_thd, my_bool signal)
>      DBUG_RETURN(0);
>    }
>  
> -  /* Try statement transaction if standard one is not set. */
> -  THD_TRANS *trans= (victim_thd->transaction.all.ha_list) ?
> -    &victim_thd->transaction.all : &victim_thd->transaction.stmt;
> -
> -  Ha_trx_info *ha_info= trans->ha_list, *ha_info_next;
> -
> -  for (; ha_info; ha_info= ha_info_next)
> +  handlerton *hton= installed_htons[DB_TYPE_INNODB];
> +  if (hton && hton->abort_transaction)
>    {
> -    handlerton *hton= ha_info->ht();
> -    if (!hton->abort_transaction)
> -    {
> -      /* Skip warning for binlog & wsrep. */
> -      if (hton->db_type != DB_TYPE_BINLOG && hton != wsrep_hton)
> -      {
> -        WSREP_WARN("Cannot abort transaction.");
> -      }
> -    }
> -    else
> -    {
> -      hton->abort_transaction(hton, bf_thd, victim_thd, signal);
> -    }
> -    ha_info_next= ha_info->next();
> +    hton->abort_transaction(hton, bf_thd, victim_thd, signal);

Okay, but please add a comment that it's safe to abort from one thread
(bf_thd) the transaction, running in another thread (victim_thd),
because innodb's lock_sys and trx_mutex guarantee the necessary protection.

And that it's not safe to access victim_thd->transaction, because it's
not protected from concurrent accesses. And it's an overkill to take
LOCK_plugin and iterate the whole installed_htons[] array every time.

Then ok to push

>    }
> +  else
> +  {
> +    WSREP_WARN("Cannot abort InnoDB transaction");
> +  }
> +
>    DBUG_RETURN(0);
>  }

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx