[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mysql-internals
Subject:    Re: bk commit into 4.1 tree (marko:1.2164)
From:       "Jocelyn Fournier" <joc () presence-pc ! com>
Date:       2004-11-30 20:10:44
Message-ID: 00ef01c4d718$acd0ab80$0100a8c0 () ppc913nmgprirv
[Download RAW message or body]

Hi Marko,

It seems there is a typo in the lock0lock.h declaration :

you've declared

ibool
lock_table_exclusive(

instead of

ibool
lock_is_table_exclusive(

This prevent MySQL latest bktree to compile.

Thanks !
  Jocelyn

----- Original Message ----- 
From: "Marko Mäkelä" <marko@mysql.com>
To: <internals@lists.mysql.com>
Sent: Tuesday, November 30, 2004 4:34 PM
Subject: bk commit into 4.1 tree (marko:1.2164)


> Below is the list of changes that have just been committed into a local
> 4.1 repository of marko. When marko does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://www.mysql.com/doc/I/n/Installing_source_tree.html
>
> ChangeSet
>   1.2164 04/11/30 17:34:37 marko@hundin.mysql.fi +3 -0
>   InnoDB: Allow ALTER TABLE to do intermediate COMMIT also when the table
>   contains auto_increment columns.  (Bug #6633)
>
>   sql/ha_innodb.cc
>     1.174 04/11/30 17:31:28 marko@hundin.mysql.fi +27 -24
>     ha_innobase::write_row(): Improve the ALTER TABLE optimization
>     (do intermediate COMMIT also if table contains auto_increment columns)
>
>   innobase/lock/lock0lock.c
>     1.46 04/11/30 17:31:22 marko@hundin.mysql.fi +109 -26
>     Replaced lock_get_table()
>     with lock_get_src_table() and lock_is_table_exclusive()
>
>   innobase/include/lock0lock.h
>     1.16 04/11/30 17:29:11 marko@hundin.mysql.fi +24 -9
>     Replaced lock_get_table()
>     with lock_get_src_table() and lock_is_table_exclusive()
>
> # This is a BitKeeper patch.  What follows are the unified diffs for the
> # set of deltas contained in the patch.  The rest of the patch, the part
> # that BitKeeper cares about, is below these diffs.
> # User: marko
> # Host: hundin.mysql.fi
> # Root: /home/marko/k/mysql-4.1
>
> --- 1.15/innobase/include/lock0lock.h Sat Nov 27 00:36:59 2004
> +++ 1.16/innobase/include/lock0lock.h Tue Nov 30 17:29:11 2004
> @@ -463,17 +463,32 @@
>   ulint space, /* in: space */
>   ulint page_no);/* in: page number */
>
/*************************************************************************
> -Gets the table covered by an IX or IS table lock, if there are no
> -other locks on the table. */
> +Gets the source table of an ALTER TABLE transaction.  The table must be
> +covered by an IX or IS table lock. */
>
>  dict_table_t*
> -lock_get_table(
> -/*===========*/
> - /* out: the table covered by the lock,
> - or NULL if it is not an IX or IS table lock,
> - or there are other locks on the table */
> - lock_t* lock, /* in: lock */
> - ulint* mode); /* out: lock mode of table */
> +lock_get_src_table(
> +/*===============*/
> + /* out: the source table of transaction,
> + if it is covered by an IX or IS table lock;
> + dest if there is no source table, and
> + NULL if the transaction is locking more than
> + two tables or an inconsistency is found */
> + trx_t* trx, /* in: transaction */
> + dict_table_t* dest, /* in: destination of ALTER TABLE */
> + ulint* mode); /* out: lock mode of the source table */
>
+/*************************************************************************
> +Determine if the given table is exclusively "owned" by the given
> +transaction, i.e., transaction holds LOCK_IX and possibly LOCK_AUTO_INC
> +on the table. */
> +
> +ibool
> +lock_table_exclusive(
> +/*=================*/
> + /* out: TRUE if table is only locked by trx,
> + with LOCK_IX, and possibly LOCK_AUTO_INC */
> + dict_table_t* table, /* in: table */
> + trx_t* trx); /* in: transaction */
>
/*************************************************************************
>  Checks that a transaction id is sensible, i.e., not in the future. */
>
>
> --- 1.45/innobase/lock/lock0lock.c Sat Nov 27 00:38:16 2004
> +++ 1.46/innobase/lock/lock0lock.c Tue Nov 30 17:31:22 2004
> @@ -602,42 +602,125 @@
>  }
>
>
/*************************************************************************
> -Gets the table covered by an IX or IS table lock, if there are no
> -other locks on the table. */
> +Gets the source table of an ALTER TABLE transaction.  The table must be
> +covered by an IX or IS table lock. */
>
>  dict_table_t*
> -lock_get_table(
> -/*===========*/
> - /* out: the table covered by the lock,
> - or NULL if it is not an IX or IS table lock,
> - or there are other locks on the table */
> - lock_t* lock, /* in: lock */
> - ulint* mode) /* out: lock mode of table */
> +lock_get_src_table(
> +/*===============*/
> + /* out: the source table of transaction,
> + if it is covered by an IX or IS table lock;
> + dest if there is no source table, and
> + NULL if the transaction is locking more than
> + two tables or an inconsistency is found */
> + trx_t* trx, /* in: transaction */
> + dict_table_t* dest, /* in: destination of ALTER TABLE */
> + ulint* mode) /* out: lock mode of the source table */
>  {
> - dict_table_t* table;
> - ulint lock_mode;
> + dict_table_t* src;
> + lock_t* lock;
>
> - table = NULL;
> + src = NULL;
>   *mode = LOCK_NONE;
>
> - if (lock_get_type(lock) != LOCK_TABLE) {
> - return(table);
> + for (lock = UT_LIST_GET_FIRST(trx->trx_locks);
> +      lock;
> +      lock = UT_LIST_GET_NEXT(trx_locks, lock)) {
> + lock_table_t* tab_lock;
> + ulint lock_mode;
> + if (!(lock_get_type(lock) & LOCK_TABLE)) {
> + /* We are only interested in table locks. */
> + continue;
> + }
> + tab_lock = &lock->un_member.tab_lock;
> + if (dest == tab_lock->table) {
> + /* We are not interested in the destination table. */
> + continue;
> + } else if (!src) {
> + /* This presumably is the source table. */
> + src = tab_lock->table;
> + if (UT_LIST_GET_LEN(src->locks) != 1 ||
> +     UT_LIST_GET_FIRST(src->locks) != lock) {
> + /* We only support the case when
> + there is only one lock on this table. */
> + return(NULL);
> + }
> + } else if (src != tab_lock->table) {
> + /* The transaction is locking more than
> + two tables (src and dest): abort */
> + return(NULL);
> + }
> +
> + /* Check that the source table is locked by
> + LOCK_IX or LOCK_IS. */
> + lock_mode = lock_get_mode(lock);
> + switch (lock_mode) {
> + case LOCK_IX:
> + case LOCK_IS:
> + if (*mode != LOCK_NONE && *mode != lock_mode) {
> + /* There are multiple locks on src. */
> + return(NULL);
> + }
> + *mode = lock_mode;
> + break;
> + }
>   }
>
> - lock_mode = lock_get_mode(lock);
> - switch (lock_mode) {
> - case LOCK_IS:
> - case LOCK_IX:
> - *mode = lock_mode;
> - table = lock->un_member.tab_lock.table;
> - if (UT_LIST_GET_LEN(table->locks) != 1 ||
> -     UT_LIST_GET_FIRST(table->locks) != lock) {
> - /* We only support the case when
> - there is only one lock on this table. */
> - table = NULL;
> + if (!src) {
> + /* No source table lock found: flag the situation to caller */
> + src = dest;
> + }
> +
> + return(src);
> +}
> +
>
+/*************************************************************************
> +Determine if the given table is exclusively "owned" by the given
> +transaction, i.e., transaction holds LOCK_IX and possibly LOCK_AUTO_INC
> +on the table. */
> +
> +ibool
> +lock_is_table_exclusive(
> +/*====================*/
> + /* out: TRUE if table is only locked by trx,
> + with LOCK_IX, and possibly LOCK_AUTO_INC */
> + dict_table_t* table, /* in: table */
> + trx_t* trx) /* in: transaction */
> +{
> + lock_t* lock;
> + bool ok = FALSE;
> +
> + ut_ad(table && trx);
> +
> + for (lock = UT_LIST_GET_FIRST(table->locks);
> +      lock;
> +      lock = UT_LIST_GET_NEXT(locks, &lock->un_member.tab_lock)) {
> + if (lock->trx != trx) {
> + /* A lock on the table is held
> + by some other transaction. */
> + return(FALSE);
> + }
> +
> + if (!(lock_get_type(lock) & LOCK_TABLE)) {
> + /* We are interested in table locks only. */
> + continue;
> + }
> +
> + switch (lock_get_mode(lock)) {
> + case LOCK_IX:
> + ok = TRUE;
> + break;
> + case LOCK_AUTO_INC:
> + /* It is allowed for trx to hold an
> + auto_increment lock. */
> + break;
> + default:
> + /* Other table locks than LOCK_IX are not allowed. */
> + return(FALSE);
>   }
>   }
> - return(table);
> +
> + return(ok);
>  }
>
>
/*************************************************************************
>
> --- 1.173/sql/ha_innodb.cc Sat Nov 27 00:39:38 2004
> +++ 1.174/sql/ha_innodb.cc Tue Nov 30 17:31:28 2004
> @@ -2325,29 +2325,44 @@
>   intermediate COMMIT, since writes by other transactions are
>   being blocked by a MySQL table lock TL_WRITE_ALLOW_READ. */
>
> - dict_table_t* table;
> + dict_table_t* src_table;
>   ibool mode;
>
>   num_write_row = 0;
>
>   /* Commit the transaction.  This will release the table
>   locks, so they have to be acquired again. */
> - switch (prebuilt->trx->mysql_n_tables_locked) {
> - case 1:
> +
> + /* Altering an InnoDB table */
> + /* Get the source table. */
> + src_table = lock_get_src_table(
> + prebuilt->trx, prebuilt->table, &mode);
> + if (!src_table) {
> + no_commit:
> + /* Unknown situation: do not commit */
> + /*
> + ut_print_timestamp(stderr);
> + fprintf(stderr,
> + "  InnoDB error: ALTER TABLE is holding lock"
> + " on %lu tables!\n",
> + prebuilt->trx->mysql_n_tables_locked);
> + */
> + ;
> + } else if (src_table == prebuilt->table) {
> + /* Source table is not in InnoDB format:
> + no need to re-acquire locks on it. */
> +
>   /* Altering to InnoDB format */
>   innobase_commit(user_thd, prebuilt->trx);
>   /* Note that this transaction is still active. */
>   user_thd->transaction.all.innodb_active_trans = 1;
>   /* We will need an IX lock on the destination table. */
>           prebuilt->sql_stat_start = TRUE;
> - break;
> - case 2:
> - /* Altering an InnoDB table */
> - ut_a(UT_LIST_GET_LEN(prebuilt->trx->trx_locks) >= 2);
> - table = lock_get_table(
> - UT_LIST_GET_FIRST(prebuilt->trx->trx_locks),
> - &mode);
> - if (!table) {
> + } else {
> + /* Ensure that there are no other table locks than
> + LOCK_IX and LOCK_AUTO_INC on the destination table. */
> + if (!lock_is_table_exclusive(prebuilt->table,
> + prebuilt->trx)) {
>   goto no_commit;
>   }
>
> @@ -2357,21 +2372,9 @@
>   /* Note that this transaction is still active. */
>   user_thd->transaction.all.innodb_active_trans = 1;
>   /* Re-acquire the table lock on the source table. */
> - row_lock_table_for_mysql(prebuilt, table, mode);
> + row_lock_table_for_mysql(prebuilt, src_table, mode);
>   /* We will need an IX lock on the destination table. */
>           prebuilt->sql_stat_start = TRUE;
> - break;
> - default:
> - no_commit:
> - /* Unknown situation: do nothing (no commit) */
> - /*
> - ut_print_timestamp(stderr);
> - fprintf(stderr,
> - "  InnoDB error: ALTER TABLE is holding lock"
> - " on %lu tables!\n",
> - prebuilt->trx->mysql_n_tables_locked);
> - */
> - break;
>   }
>   }
>
>
> -- 
> MySQL Internals Mailing List
> For list archives: http://lists.mysql.com/internals
> To unsubscribe:
http://lists.mysql.com/internals?unsub=joc@presence-pc.com
>
>


-- 
MySQL Internals Mailing List
For list archives: http://lists.mysql.com/internals
To unsubscribe:    http://lists.mysql.com/internals?unsub=mysql-internals@progressive-comp.com

[prev in list] [next in list] [prev in thread] [next in thread]