1. 07 Aug, 2008 2 commits
    • vasil's avatar
      branches/zip: · 48216724
      vasil authored
      The cardinality of every index (the number of different key values) is
      calculated when the table is opened, at SHOW TABLE STATUS,
      ANALYZE TABLE and on other circumstances (like when the table has
      changed too much). Note that if the mysql client is running with the
      auto-rehash setting turned on (default) this causes all tables to be
      opened when it starts.
      
      Previously InnoDB sampled 8 random pages from the index to get an
      estimate of the cardinality. Now the number of sampled pages can be
      changed via the global parameter innodb_stats_sample_pages which can
      be tuned at runtime. The default value for this parameter is 8.
      
      If the value of this parameter is changed, there may be serious problems:
      
      - small values (say, 1) can cause an error in table stats;
      - values much larger than 8 (say, 100), can cause a big slowdown in
        table opening time, SHOW TABLE status, etc.
      - query plans may be different from the old ones.
      
      Approved by:	Heikki
      48216724
    • vasil's avatar
      branches/zip: · a97b0cc3
      vasil authored
      Fix Mantis issue#66.
      
      Do not try to dereference the MySQL thread object if it is NULL.
      
      Approved by:	Sunny (via IM)
      a97b0cc3
  2. 06 Aug, 2008 6 commits
    • marko's avatar
      branches/zip: Correct some typos in comments. · 0ce4e51d
      marko authored
      0ce4e51d
    • marko's avatar
      branches/zip: Fix terminology that was introduced in r2569: · 046c1d91
      marko authored
      replace rbmode and "rollback mode"
      with rb_ctx and "rollback context".
      Suggested by Sunny.
      046c1d91
    • marko's avatar
      branches/zip: In the rollback of incomplete transactions after crash · 8a73d356
      marko authored
      recovery, tolerate clustered index records whose externally stored
      columns have not been written.  This should remove the assertion failures
      that were reported as Mantis issue#58, issue#62, issue#64.
      
      trx_is_recv(): New function: TRUE if this transaction is rolling back
      an incomplete transaction in crash recovery.
      
      enum trx_rbmode: Rollback modes: no rollback, normal rollback, crash recovery.
      
      btr_cur_pessimistic_delete(), btr_free_externally_stored_field(),
      btr_rec_free_externally_stored_fields():
      Replace the ibool parameter with enum trx_rbmode.
      
      btr_free_externally_stored_field(): If field_ref is zero, return
      but assert ut_a(rbmode == RB_RECOVERY).  Unless InnoDB has crashed
      while inserting a clustered index record, field_ref should not be zero.
      
      btr_rec_free_updated_extern_fields(): Add the parameter enum trx_rbmode.
      
      btr_cur_pessimistic_update(): Pass the rbmode parameter to
      btr_rec_free_updated_extern_fields().
      
      row_undo_ins(), row_undo_mod_upd_del_sec(): If row_build_index_entry()
      fails, assert trx_is_recv() and skip this secondary index.
      
      row_undo_mod_upd_del_sec(): Empty the heap at the end of each loop
      iteration in order to conserve memory and to reduce the number of
      low-level memory allocations.
      8a73d356
    • marko's avatar
      branches/zip: Improve the diagnostics for tracking down issue#63 and issue#65. · 94a7cee3
      marko authored
      buf_flush_init_for_writing(), buf_LRU_block_remove_hashed_page():
      Dump the page frame featuring the incorrect FIL_PAGE_TYPE along with the
      page_zip->data that might contain an earlier version of the page.
      94a7cee3
    • marko's avatar
    • marko's avatar
      branches/zip: Disable two debug assertions that attempt to call · a9b6c6bc
      marko authored
      buf_block_align() on a non-file page frame that was created in
      btr_cur_pessimistic_insert(), to see if a record fits on a compressed
      page by itself.  These assertions caused an assertion failure in
      buf_block_align() in innodb_bug36172.test.
      
      page_zip_write_rec(), page_zip_write_header(): Remove the assertion
      that calls buf_frame_get_page_zip().
      a9b6c6bc
  3. 04 Aug, 2008 4 commits
  4. 01 Aug, 2008 3 commits
  5. 30 Jul, 2008 1 commit
    • inaam's avatar
      branches/zip: issue#57 · aa87f6bd
      inaam authored
      Fixes a race in recovery where the recovery thread recovering a
      PREPARED trx and the background rollback thread can both try
      to free the trx after its status is set to COMMITTED_IN_MEMORY.
      
      trx->is_recovered flag was introduced in r2040.
      
      Reviewed by: Sunny
      aa87f6bd
  6. 29 Jul, 2008 1 commit
    • inaam's avatar
      branches/zip issue# 33 · 3468767c
      inaam authored
      This fix makes two basic changes in blob handling:
      (The bug was introduced in r2252)
      
      1) The blob prefixes are no longer stored in the undo if
        a) We are modifying a delete marked record and
        b) The record was delete marked by an already committed trx.
      
      2) When building old row version to check if one of these versions
      can hold an implicit lock on the record we stop our probe if
        a) The version is delete marked and
        b) The delete marking is done by a trx which is different from
           the current active trx on the record.
      
      Reviewed by: Heikki
      3468767c
  7. 21 Jul, 2008 1 commit
  8. 15 Jul, 2008 2 commits
    • inaam's avatar
      branches/zip: · 47e159a3
      inaam authored
      Fixed a mistake in r2539 where I changed UNIV_INLINE to UNIV_INTERN
      for the wrong function in btr0sea.h
      47e159a3
    • inaam's avatar
      branches/zip: Merge 2524:2537 from branches/5.1: · f2ad32be
      inaam authored
        ------------------------------------------------------------------------
        r2537 | inaam | 2008-07-15 20:46:03 +0300 (Tue, 15 Jul 2008) | 12 lines
        
        branches/5.1   issue# 4
        
        Fixed a timing hole where a thread dropping an index can free the
        in-memory index struct while another thread is still using
        that structure to remove entries from adaptive hash index belonging
        to one of the pages that belongs to the index being dropped.
        
        The fix is to have a reference counter in the index struct and to
        wait for this counter to drop to zero beforing freeing the struct.
        
        Reviewed by: Heikki
        
        ------------------------------------------------------------------------
      f2ad32be
  9. 04 Jul, 2008 1 commit
  10. 03 Jul, 2008 2 commits
    • calvin's avatar
      branches/zip: · 6e41b961
      calvin authored
      Add a space in a warning message. No functional change to the code.
      The original message is "invalid innodb_file_format_checkvalue;...".
      The new one is "invalid innodb_file_format_check value;..."
      6e41b961
    • calvin's avatar
      branches/zip: · 8b449133
      calvin authored
      Fixes issue# 51 - by removing the negative test cases that produce
      warnings. The warnings cause mysql-test-run to fail.
      
      Those negative test cases will be moved to separate test/result files.
      
      Approved by:    Sunny
      8b449133
  11. 02 Jul, 2008 1 commit
    • vasil's avatar
      branches/zip: · f66c5e5d
      vasil authored
      Disable part of innodb-index test because MySQL changed its behavior and
      is not calling ::add_index() anymore in the following ALTER TABLE:
      
      CREATE TABLE t (a INT) ENGINE=INNODB;
      INSERT INTO t VALUES (NULL);
      ALTER TABLE t ADD PRIMARY KEY (a);  -- adding primary index on non-NULL column
      
      Previously, in ALTER TABLE, MySQL would call ::add_index() which would
      fail with a "primary key cannot contain NULL" error.
      
      The change occured in:
      
        mysql-5.1$ bzr log -v -r2667
        ------------------------------------------------------------
        revno: 2667
        committer: Davi Arnaut <davi@mysql.com>
        branch nick: 33873-5.1
        timestamp: Tue 2008-06-17 11:12:21 -0300
        message:
          Bug#33873: Fast ALTER TABLE doesn't work with multibyte character sets
          
          The problem was that when comparing tables for a possible
          fast alter table, the comparison was being performed using
          the parsed information and not the final definition.
                
          The solution is to use the possible final table layout to
          compare if a fast alter is possible or not.
        modified:
          mysql-test/include/mix1.inc
          mysql-test/r/alter_table.result
          mysql-test/r/innodb_mysql.result
          mysql-test/t/alter_table.test
          sql/sql_table.cc
        mysql-5.1$
      f66c5e5d
  12. 01 Jul, 2008 1 commit
    • vasil's avatar
      branches/zip: Merge 2489:2524 from branches/5.1: · ab4a89c6
      vasil authored
        ------------------------------------------------------------------------
        r2519 | sunny | 2008-06-26 16:55:43 +0300 (Thu, 26 Jun 2008) | 5 lines
        
        branches/5.1: Add test cases and fix a bug where the last AUTOINC cached value
        was not reset to 0 when the table was truncated.
        
        Bug #37531 : After truncate, auto_increment behaves incorrectly for InnoDB
        
        ------------------------------------------------------------------------
        r2520 | vasil | 2008-06-26 17:38:02 +0300 (Thu, 26 Jun 2008) | 7 lines
        
        branches/5.1:
         
        Fix Bug#36941 Performance problem in ha_print_info (SHOW INNODB STATUS)
        by disabling some of the code in ha_print_info() in production builds.
         
        Approved by:	Heikki (via IM)
        
        ------------------------------------------------------------------------
        r2521 | vasil | 2008-06-26 17:39:01 +0300 (Thu, 26 Jun 2008) | 8 lines
        
        branches/5.1:
         
        Fix Bug#36942 Performance problem in lock_get_n_rec_locks (SHOW INNODB STATUS)
        by not calling lock_get_n_rec_locks() from lock_print_info_summary() on
        production builds.
         
        Approved by:	Heikki (via IM)
        
        ------------------------------------------------------------------------
        r2524 | vasil | 2008-07-01 10:37:34 +0300 (Tue, 01 Jul 2008) | 29 lines
        
        branches/5.1:
        
        Merge a change from MySQL (this fixes the failing innodb-replace test):
        
          revno: 2659
          committer: Mattias Jonsson <mattiasj@mysql.com>
          branch nick: b31210-51-bugteam
          timestamp: Tue 2008-06-03 13:25:41 +0200
          message:
            Bug#31210: INSERT DELAYED crashes server when used on partitioned tables
              
            Problem was an unclear error message since it could suggest that
            MyISAM did not support INSERT DELAYED.
            Changed the error message to say that DELAYED is not supported by the
            table, instead of the table's storage engine.
            The confusion is that a partitioned table is in somewhat sense using
            the partitioning storage engine, which in turn uses the ordinary
            storage engine. By saying that the table does not support DELAYED we
            do not give any extra informantion about the storage engine or if it
            is partitioned.
          modified:
            mysql-test/r/innodb-replace.result
            mysql-test/t/innodb-replace.test
            mysql-test/t/merge.test
            mysql-test/t/partition_hash.test
            sql/share/errmsg.txt
            sql/sql_insert.cc
      
      
      ------------------------------------------------------------------------
      ab4a89c6
  13. 26 Jun, 2008 1 commit
  14. 19 Jun, 2008 3 commits
    • vasil's avatar
      branches/zip: · 232d8337
      vasil authored
       
      Rewrite the function innodb_plugin_init() to support parameters in
      different order (in static and dynamic InnoDB) and to support more
      parameters in the static InnoDB.
       
      The previous implementation traversed both lists at the same time,
      skipping parameters in the dynamic list that do not exist in the static
      list. E.g. both lists were allowed to be static=(a, b, c),
      dynamic=(a, b, c) or static=(a, b, c), dynamic=(a, b, x, c).
       
      With the new implementation they are allowed to be
      static=(a, b, c), dynamic=(b, a, c) or
      static=(a, b, x, c), dynamic=(b, a, c) in addition.
      
      The new implementation has complexity O(N^2) while the old one was O(N),
      but this is acceptable sacrifice provided that innodb_plugin_init() is
      called once per InnoDB lifetime and that N=39 currently, and N is not
      going to increase much in the future, N=number of the InnoDB parameters.
      
      Suggested by:	Sunny
      Approved by:	Sunny
      232d8337
    • vasil's avatar
      branches/zip: · 6ef84d75
      vasil authored
      Fix the rpl_ddl mysql-test that produces a warnings:
      
        master.err: rpl.rpl_ddl: 080611 16:27:01 [Warning] MySQL is closing a connection that has an active InnoDB transaction.  13 row modifications will roll back.
      
      the bug was introduced in r2256, which adds "prebuilt->trx->active_trans = 1;"
      in ::add_index() and ::final_drop_index() but never calls innobase_commit() or
      innobase_rollback() to set back active_trans to 0.
      
      Also, per Heikki's suggestion we remove the calls to trans_register_ha() from
      ::add_index() and ::final_drop_index(). Following is the IM discussion that
      took place about this:
      
        --- cut ---
        (17:59:05) vasil: 
                unsigned        active_trans:2; /* 1 - if a transaction in MySQL
                                                is active. 2 - if prepare_commit_mutex
                                                was taken */
        
        (17:59:13) vasil: what about 0?
        (17:59:26) Heikki: Hmm
        (17:59:42) Heikki: Maybe in the other cases :)
        (18:01:00) vasil: 
           2   1369  handler/ha_innodb.cc <<<unknown>>>
                     if (trx->active_trans == 0) {
           3   1372  handler/ha_innodb.cc <<<unknown>>>
                     trx->active_trans = 1;
        
        (18:01:05) vasil: yes, it can be 0
        (18:01:38) Heikki: Maybe no active transaction => the value is 0
        (18:02:03) vasil: probably yes
        (18:09:09) vasil: 
                /* The flag trx->active_trans is set to 1 in
        
                1. ::external_lock(),
                2. ::start_stmt(),
                3. innobase_query_caching_of_table_permitted(),
                4. innobase_savepoint(),
                5. ::init_table_handle_for_HANDLER(),
                6. innobase_start_trx_and_assign_read_view(),
                7. ::transactional_table_lock()
        
                and it is only set to 0 in a commit or a rollback.
        (18:09:14) vasil: this is no longer true
        (18:09:35) vasil: it is also set to 1 in ::final_drop_index
        (18:09:56) vasil: and in ::add_index()
        (18:12:31) Heikki: Please correct the comment :(
        (18:13:01) vasil: it is not only the comment, I am trying to understand what's wrong
        (18:13:28) Heikki: What is the symptom?
        (18:14:37) vasil: can you find this email and the subsequent one:
        
        From: Vasil Dimov <vasil.dimov@oracle.com>
        To: InnoDB Dev <innodb_dev_ww@oracle.com>
        Subject: Re: innodb - marko - r2256 - in branches/zip: handler include lock
                trx
        Date: Wed, 11 Jun 2008 16:33:47 +0300
        
        (18:15:02) vasil: the symptom is 
        
        master.err: rpl.rpl_ddl: 080611 16:27:01 [Warning] MySQL is closing a connection that has
        +an active InnoDB transaction.  13 row modifications will roll back.
        
        (18:16:22) vasil: My understanding till now is that active_trans is set to 1 in ::add_index() and forgotten like this, never set to 0; I wonder if it should be set to 0 somewhere at the end or not set to 1 at all... or there is more...
        (18:16:31) Heikki: Vasil, ok the problem is that Marko forgets to commit the transaction
        (18:16:55) Heikki: An active transaction has to be committed or rolled back
        (18:17:09) vasil: but i can find this in ::add_index():
        
                /* Commit the data dictionary transaction in order to release
                the table locks on the system tables.  Unfortunately, this
                means that if MySQL crashes while creating a new primary key
                inside row_merge_build_indexes(), indexed_table will not be
                dropped on crash recovery.  Thus, it will become orphaned. */
                trx_commit_for_mysql(trx);
        
        (18:17:27) vasil: but this function does not set active_trans to 0
        (18:17:43) Heikki: Let me look
        (18:19:20) vasil: it is only set to 0 in innobase_commit() and in innobase_rollback()
        (18:20:10) Heikki: Ok
        (18:20:59) Heikki: Hmm...
        (18:21:08) vasil: innobase_commit() calls innobase_commit_low() calls trx_commit_for_mysql()
        innobase_commit() sets active_trans=0
        (18:21:26) Heikki: active_trans is strictly a ha_innodb.cc variable
        (18:21:53) Heikki: It is intended to be used in transactions visible to MySQL
        (18:22:25) Heikki: I think Marko should call innobase_commit()
        (18:23:51) vasil: but still set active_trans=1 inside :add_index() inside handler/handler0alter.cc?
        (18:24:12) Heikki: Hmm
        (18:24:56) Heikki: I think the transaction is NOT really visible to MySQL
        (18:24:57) vasil: 
        > Author: marko
        > Date: 2008-01-25 16:26:07 +0200 (Fri, 25 Jan 2008)
        > New Revision: 2256
        >
        > Log:
        > branches/zip: Fast index creation: Release locks on system tables before
        > creating indexes.  Lock the user table inside the user transaction.
        >
        > enum trx_dict_op: Remove TRX_OP_INDEX_MAY_WAIT.
        >
        > ha_innobase::add_index(): Lock the user tables within prebuilt->trx.
        > Commit the data dictionary transaction before creating indexes.
        >
        > ha_innobase::final_drop_index(): Lock the user table within prebuilt->trx.
        
        (18:26:41) Heikki: Let me look at the old ::create_table()
        (18:27:25) Heikki: Ok, in the old code, the transactions are internal to InnoDB, not visible to MySQL
        (18:27:46) Heikki: I think it is an error to set active_trans=1 inside :add_index() i
        (18:28:11) Heikki: MySQL is not really interested in HOW the engine adds the index. In a transaction or not.
        (18:28:26) vasil: this is the change inside drop_index(), that is supposed to "Lock the user table within prebuilt->trx":
        (18:28:37) vasil: 
        @@ -1093,27 +1101,25 @@ ha_innobase::final_drop_index(
          /* Create a background transaction for the operations on
          the data dictionary tables. */
          trx = trx_allocate_for_mysql();
          trx_start_if_not_started(trx);
         
          trans_register_ha(user_thd, FALSE, ht);
        + prebuilt->trx->active_trans = 1;
         
          trx->mysql_thd = user_thd;
          trx->mysql_query_str = thd_query(user_thd);
         
          /* Flag this transaction as a dictionary operation, so that
        - the data dictionary will be locked in crash recovery.  Prevent
        - warnings if row_merge_lock_table() results in a lock wait,
        - i.e., when another transaction is holding a conflicting lock
        - on the table, e.g., because of SELECT ... FOR UPDATE. */
        - trx_set_dict_operation(trx, TRX_DICT_OP_INDEX_MAY_WAIT);
        + the data dictionary will be locked in crash recovery. */
        + trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);
         
          /* Lock the table exclusively, to ensure that no active
          transaction depends on an index that is being dropped. */
          err = convert_error_code_to_mysql(
        -  row_merge_lock_table(trx, prebuilt->table, LOCK_X),
        +  row_merge_lock_table(prebuilt->trx, prebuilt->table, LOCK_X),
           user_thd);
         
          if (UNIV_UNLIKELY(err)) {
         
           /* Unmark the indexes to be dropped. */
           row_mysql_lock_data_dictionary(trx);
        @@ -1125,14 +1131,12 @@ ha_innobase::final_drop_index(
           }
         
           row_mysql_unlock_data_dictionary(trx);
           goto func_exit;
          }
         
        - trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);
        -
          /* Drop indexes marked to be dropped */
         
          row_mysql_lock_data_dictionary(trx);
         
          index = dict_table_get_first_index(prebuilt->table);
         
        @@ -1159,12 +1163,13 @@ ha_innobase::final_drop_index(
          dict_table_check_for_dup_indexes(prebuilt->table);
         #endif
          row_mysql_unlock_data_dictionary(trx);
         
         func_exit:
          trx_commit_for_mysql(trx);
        + trx_commit_for_mysql(prebuilt->trx);
         
          /* Flush the log to reduce probability that the .frm files and
          the InnoDB data dictionary get out-of-sync if the user runs
          with innodb_flush_log_at_trx_commit = 0 */
         
          log_buffer_flush_to_disk();
        
        (18:28:43) Heikki: Let me look
        (18:29:05) vasil: this is the change inside drop_index(), that is supposed to "Lock the user table within prebuilt->trx"
        (18:29:26) Heikki:   trans_register_ha(user_thd, FALSE, ht);
        + prebuilt->trx->active_trans = 1;
        (18:29:34) Heikki: I think the above code should be removed
        (18:29:53) Heikki: MySQL is not interested in InnoDB's transaction in this case
        (18:30:33) Heikki: Alternatively, we should use innobase_commit() instead of trx_commit_...
        (18:31:01) vasil: but there are many places in ::add_index() where it could return before reaching the trx_commit...
        (18:31:12) vasil: leaving active_traans=1
        (18:31:56) Heikki: Does Marko handle the errors with trx_rollback()?
        (18:32:08) vasil: let me see..
        (18:32:30) Heikki: It is risky to leave error handling to MySQL :(
        (18:32:35) vasil: 
         651         if (UNIV_UNLIKELY(error)) {
         652 err_exit:
         653                 mem_heap_free(heap);
         654                 trx_general_rollback_for_mysql(trx, FALSE, NULL);
         655                 trx_free_for_mysql(trx);
         656                 trx_commit_for_mysql(prebuilt->trx);
         657                 DBUG_RETURN(error);
         658         }
        
        (18:32:43) Heikki: Ok
        (18:33:25) Heikki: Please remove the ha_register... and active_trx =1 ....
        (18:33:42) Heikki: Then check if it prints something
        (18:34:08) Heikki: I believe MySQL is not interested in how the engine does the index build
        (18:34:11) vasil: why remove ha_register?
        (18:34:28) Heikki: Because there is no transaction visible to MySQL
        (18:34:53) vasil: the ha_register call was there from before
        (18:35:09) Heikki: Registers that InnoDB takes part in an SQL statement, so that MySQL knows to
        roll back the statement if the statement results in an error. This MUST be
        called for every SQL statement that may be rolled back by MySQL. Calling this
        several times to register the same statement is allowed, too. 
        (18:36:01) Heikki: Hmm
        (18:36:17) Heikki: Obviously, MySQL does NOT call commit at the end
        (18:36:36) Heikki: Because then innobase_commit() would set active_trx=0!
        (18:37:00) Heikki: Ok, maybe MySQL thinks CREATE INDEX is NOT a transactional statement
        (18:37:21) Heikki: Then, .._register_.. should be removed
        (18:37:51) Heikki: It is logical this way. If it also passes test, even better
        (18:38:21) vasil: ha_registrer in add_index() was added in r1845 and in drop_index() in r2128, let me read the comments..
        (18:38:55) vasil: 
        ------------------------------------------------------------------------
        r1845 | marko | 2007-09-13 13:31:54 +0300 (Thu, 13 Sep 2007) | 3 lines
        
        branches/zip: Move the code related to fast index creation
        (smart ALTER TABLE) from ha_innodb.cc to a separate module, handler0alter.cc.
        
        (18:39:05) Heikki: One thing you must check is that MySQL does write the CREATE INDEX in the binlog, even if we remove the _register_
        (18:39:41) Heikki: I assume Marko did not know if MySQL thinks CREATE INDEX is transactional or not
        (18:39:49) Heikki: He thought it is transactional
        (18:39:53) vasil: 
        ------------------------------------------------------------------------
        r2128 | marko | 2007-11-29 12:34:55 +0200 (Thu, 29 Nov 2007) | 5 lines
        
        branches/zip: ha_innobase::final_drop_index(): Allocate a separate transaction
        for dropping the index trees, and set the dictionary operation flag, similar
        to what ha_innobase::add_index() does.  This should ensure correct crash
        recovery.
        
        (18:40:10) Heikki: Ok
        (18:40:17) vasil: how do you know CREATE INDEX is not transactional?
        (18:40:33) Heikki: You cannot roll it back
        (18:40:44) vasil: ok
        (18:41:03) Heikki: MySQL devs probably thought this way
        (18:44:15) vasil: hmm
        (18:44:21) vasil: about r1845
        (18:45:00) vasil: the comments says "move the code" but it is actually "move and change the code"
        (18:45:28) Heikki: Well
        (18:45:29) vasil: the code that is deleted from ha_innodb is this:
        (18:45:32) vasil: 
        -       update_thd(ha_thd());
        -
        -       heap = mem_heap_create(1024);
        -
        -       /* In case MySQL calls this in the middle of a SELECT query, release
        -       possible adaptive hash latch to avoid deadlocks of threads. */
        -       trx_search_latch_release_if_reserved(check_trx_exists(user_thd));
        -
        -       trx = trx_allocate_for_mysql();
        -       trx_start_if_not_started(trx);
        -
        -       innobase_register_stmt(ht, user_thd);
        -
        -       trx->mysql_thd = user_thd;
        
        (18:45:38) vasil: (in add_index)
        (18:45:48) inaam left the room.
        (18:45:52) vasil: but the new one in handler0alter is this:
        (18:46:16) vasil: 
        +       update_thd(ha_thd());
        +
        +       heap = mem_heap_create(1024);
        +
        +       /* In case MySQL calls this in the middle of a SELECT query, release
        +       possible adaptive hash latch to avoid deadlocks of threads. */
        +       trx_search_latch_release_if_reserved(check_trx_exists(user_thd));
        +
        +       trx = trx_allocate_for_mysql();
        +       trx_start_if_not_started(trx);
        +
        +       trans_register_ha(user_thd, FALSE, ht);
        +
        +       trx->mysql_thd = user_thd;
        
        (18:46:48) vasil: innobase_register_stmt(ht, user_thd); was substituted with trans_register_ha(user_thd, FALSE, ht); during the move
        (18:46:49) Heikki: I do not see how MySQL could call it in the middle of a SELECT query!
        (18:46:59) Heikki: Yes
        (18:47:21) Heikki: Did Marko change innobase_register_stmt() code too?
        (18:48:05) vasil: not in this commit r1845
        (18:48:54) Heikki: Anyway, please test a version where you remove the register thing
        (18:49:08) vasil: and active_trans=1?
        (18:49:15) Heikki: Remove that, too
        (18:49:35) Heikki: innobase_register_stmt(
        /*===================*/
                handlerton*     hton,   /* in: Innobase hton */
                THD*    thd)    /* in: MySQL thd (connection) object */
        {
                DBUG_ASSERT(hton == innodb_hton_ptr);
                /* Register the statement */
                trans_register_ha(thd, FALSE, hton);
        }
        
        (18:50:23) Heikki: looks pretty much the same as what Marko has in the later version
        (18:50:43) vasil: innobase_register_stmt() was added in add_index() in r1584
        (18:50:47) vasil: 
        r1584 | marko | 2007-06-18 15:46:42 +0300 (Mon, 18 Jun 2007) | 12 lines
        
        branches/zip: ha_innobase::add_index(): Split some assertions.
        Remove the variable parent_trx.
        
        Call innobase_register_stmt() in order to work around a MySQL bug
        in mysql_alter_table(), which, as of
        
        ChangeSet@1.2482.61.2, 2007-06-07 16:37:15+02:00, joerg@trift2. +8 -0
        
        commits the transaction before calling ha_innobase::add_index().
        Without re-registering the statement, the ha_commit_stmt(thd)
        in mysql_alter_table() would not invoke innobase_commit.
        
        (18:51:56) Heikki: But it does not seem to invoke it now, because active_trans is left 1!
        (18:54:15) Heikki: Apparently, there has been confusion also among MySQL devs whether CREATE INDEX is transactional or not
        (18:54:45) vasil: :-(
        (18:54:53) Heikki: The most robust way is to handle all commits and rollbacks internally inside InnoDB and not tell MySQL anything about InnoDB's internal transactions
        (18:55:01) Heikki: That is my idea
        (18:55:20) Heikki: Then MySQL cannot spoil the logic of the code
        (18:55:59) vasil: mysql_alter_table() is ~1100 lines
        (18:56:08) vasil: and you suggest what?
        (18:56:43) Heikki: I suggest removing the ..._register_... thing and active_trans = 1
        (18:57:08) Heikki: Those things tell MySQL that there is a transaction going on
        (18:57:14) Heikki: Better not to tell
        (18:57:29) vasil: and possibly ignoring the above comment in r1584, "Call innobase_register_stmt() in order to work around a MySQL bug
        in mysql_alter_table()........."
        (18:57:32) vasil: ?
        (18:57:36) Heikki: Yes
        (18:57:38) vasil: ok
        (18:57:44) vasil: same for drop_index?
        (18:57:48) Heikki: Yes
        (18:58:51) vasil: testing
        Index: handler/handler0alter.cc
        ===================================================================
        --- handler/handler0alter.cc (revision 2498)
        +++ handler/handler0alter.cc (working copy)
        @@ -632,14 +632,14 @@ ha_innobase::add_index(
         
          /* Create a background transaction for the operations on
          the data dictionary tables. */
          trx = trx_allocate_for_mysql();
          trx_start_if_not_started(trx);
         
        - trans_register_ha(user_thd, FALSE, ht);
        - prebuilt->trx->active_trans = 1;
        + //trans_register_ha(user_thd, FALSE, ht);
        + //prebuilt->trx->active_trans = 1;
         
          trx->mysql_thd = user_thd;
          trx->mysql_query_str = thd_query(user_thd);
         
          innodb_table = indexed_table
           = dict_table_get(prebuilt->table->name, FALSE);
        @@ -1081,14 +1081,14 @@ ha_innobase::final_drop_index(
         
          /* Create a background transaction for the operations on
          the data dictionary tables. */
          trx = trx_allocate_for_mysql();
          trx_start_if_not_started(trx);
         
        - trans_register_ha(user_thd, FALSE, ht);
        - prebuilt->trx->active_trans = 1;
        + //trans_register_ha(user_thd, FALSE, ht);
        + //prebuilt->trx->active_trans = 1;
         
          trx->mysql_thd = user_thd;
          trx->mysql_query_str = thd_query(user_thd);
         
          /* Flag this transaction as a dictionary operation, so that
          the data dictionary will be locked in crash recovery. */
        
        (18:59:07) vasil: not warnings
        (18:59:22) Heikki: Did it create the indexes
        (18:59:36) vasil: I did test only removing active_trans=1 and it removed the warnings too
        (18:59:38) vasil: dunno
        (18:59:54) Heikki: That is the most important thing to check :)
        (19:00:10) vasil: :)
        (19:00:38) vasil: it is some mysqltest that is 610 lines
        (19:00:48) vasil: and it happens at the end
        (19:01:02) vasil: lets see how may add index statements...
        (19:01:11) vasil: 0
        (19:01:26) Heikki: Mostly I am concerned that MySQL might work wrong without that _register_ thing
        (19:03:28) vasil: could be
        (19:03:49) Heikki: You need to check it still calls ::add_index()
        (19:05:15) vasil: well, the change we made is inside ::add_index() how could it result in mysql stopping to call ::add_index()?
        (19:05:39) vasil: if it calls it twice...
        (19:05:43) Heikki: :-D
        (19:08:34) vasil: I guess it must have created the index, because the test is doing SHOW INDEX after CREATE INDEX and after DROP INDEX
        (19:08:43) vasil: the test would have failed if the index was not created
        (19:09:10) Heikki: You need to check that a query using that index will have the right EXPLAIN and result
        (19:15:04) vasil: this is with the patch:
        +CREATE TABLE t (a INT) ENGINE=innodb;
        +CREATE INDEX ti ON t(a);
        +INSERT INTO t VALUES (1), (2), (3);
        +EXPLAIN SELECT * FROM t WHERE a=2;
        +id select_type table type possible_keys key key_len ref rows Extra
        +1 SIMPLE t ref ti ti 5 const 1 Using where; Using index
        +SELECT * FROM t WHERE a=2;
        +a
        +2
        
        (19:15:15) Heikki: :)
        (19:15:46) vasil: this is with the original code:
        +CREATE TABLE t (a INT) ENGINE=innodb;
        +CREATE INDEX ti ON t(a);
        +INSERT INTO t VALUES (1), (2), (3);
        +EXPLAIN SELECT * FROM t WHERE a=2;
        +id select_type table type possible_keys key key_len ref rows Extra
        +1 SIMPLE t ref ti ti 5 const 1 Using where; Using index
        +SELECT * FROM t WHERE a=2;
        +a
        +2
        
        (19:15:59) vasil: it is the same
        (19:16:06) vasil: do you think this is sufficient?
        (19:16:20) Heikki: No, you need to do some manual tests, too
        (19:16:28) vasil: this is manual test
        (19:16:37) Heikki: Several of them
        (19:16:46) vasil: what should they do?
        (19:17:13) Heikki: Test tables of different sizes, different indexes
        (19:17:27) Heikki: CREATE several indexes at a time
        (19:19:14) Heikki: Dinner time -->
        --- cut ---
      
      Approved by:	Heikki
      6ef84d75
    • vasil's avatar
      branches/zip: · 30dc57da
      vasil authored
      Add the patches that are needed to make autotest.sh succeed.
      30dc57da
  15. 18 Jun, 2008 2 commits
    • inaam's avatar
      branches/zip: fixes issue# 36 · ba06ffb3
      inaam authored
      Fixed the issue where an unchanged blob column that is part of the
      primary key as well is lost if it is externally stored.
      
      The fix is provided by Marko.
      
      Reviewed by: Sunny
      ba06ffb3
    • inaam's avatar
      branches/zip: internal mantis issue#49 · 217272fa
      inaam authored
      This is to fix an unintended side effect of file_format_check changes.
      We were reading the trx system page (TRX_SYS_PAGE_NO) before starting
      recovery and that resulted in redo logs not being applied to the page.
      The fix is to force a reread by calling buf_pool_invalidate().
      This, however, made necessary that any writes to page are deferred to
      until after the redo log application.
      
      Reviewed by: Sunny
      
      217272fa
  16. 17 Jun, 2008 3 commits
  17. 09 Jun, 2008 2 commits
    • vasil's avatar
      branches/zip: · 1d0e95b1
      vasil authored
      Update the ChangeLog.
      1d0e95b1
    • vasil's avatar
      branches/zip: Merge 2485:2489 from branches/5.1: · d609478a
      vasil authored
        ------------------------------------------------------------------------
        r2488 | vasil | 2008-06-09 13:53:12 +0300 (Mon, 09 Jun 2008) | 15 lines
        Changed paths:
           M /branches/5.1/mysql-test/innodb.result
        
        branches/5.1:
        
        Merge change from MySQL (this almost fixes the failing innodb test):
        
          ChangeSet@1.2613, 2008-05-07 09:58:21+04:00, sergefp@mysql.com +5 -0
            BUG#35850 "Performance regression in 5.1.23/5.1.24"
            - Disable the "prefer full scan on clustered primary key over full scan
              of any secondary key" rule introduced by BUG#35850.
            - Update test results accordingly
            (bk trigger: file this for BUG#35850)
          
            mysql-test/r/innodb.result@1.214, 2008-05-07 09:57:31+04:00, sergefp@mysql.com +15 -15
              BUG#35850 "Performance regression in 5.1.23/5.1.24"
              - Update test results
        
        ------------------------------------------------------------------------
        r2489 | vasil | 2008-06-09 14:16:51 +0300 (Mon, 09 Jun 2008) | 20 lines
        Changed paths:
           M /branches/5.1/mysql-test/innodb.result
        
        branches/5.1:
        
        Merge change from MySQL (this fixes the failing innodb test):
        
          ChangeSet@1.2628  2008-05-13 17:01:02+05:00  gshchepa@host.loc
            Merge host.loc:/work/bugs/5.0-bugteam-30059
            into  host.loc:/work/bk/5.1-bugteam
        
        NOTE: the diff for mysql-test/r/innodb.result in this ChangeSet looks
        empty but the line:
        
        Data truncated for column 'c' at row 1
        
        is present in annotated mysql-test/r/innodb.result@1.215 [1]
        and not present in annotated mysql-test/r/innodb.result@1.216 [2] so it
        must have been deleted in the above ChangeSet.
        
        [1] http://mysql.bkbits.net:8080/mysql-5.1/mysql-test/r/innodb.result?PAGE=anno&REV=4825e033JNz_463Ha9pwInSjqLvIQg
        [2] http://mysql.bkbits.net:8080/mysql-5.1/mysql-test/r/innodb.result?PAGE=anno&REV=482982fcUoskWjhU2xMH2BZ4Gyukfg
        
        ------------------------------------------------------------------------
      d609478a
  18. 06 Jun, 2008 2 commits
    • vasil's avatar
      branches/zip: · 6859fac1
      vasil authored
      Update the ChangeLog.
      6859fac1
    • vasil's avatar
      branches/zip: Merge 2437:2485 from branches/5.1: · dde455e8
      vasil authored
      (r2478 was skipped for the obvious reason)
      
        ------------------------------------------------------------------------
        r2464 | vasil | 2008-05-19 17:59:42 +0300 (Mon, 19 May 2008) | 9 lines
        
        branches/5.1:
        
        Fix Bug#36600 SHOW STATUS takes a lot of CPU in buf_get_latched_pages_number
        
        by removing the Innodb_buffer_pool_pages_latched variable from
        SHOW STATUS output in non-UNIV_DEBUG compilation.
         
        Approved by:	Heikki
        
        ------------------------------------------------------------------------
        r2466 | calvin | 2008-05-20 01:37:14 +0300 (Tue, 20 May 2008) | 12 lines
        
        branches/5.1:
        
        Fix Bug#11894 innodb_file_per_table crashes w/ Windows .sym symbolic
        link hack
        
        The crash was due to un-handled error 3 (path not found). In the case
        of file per table, change the call to os_file_handle_error_no_exit()
        from os_file_handle_error(). Also, checks for full path pattern during
        table create (Windows only), which is used in symbolic link and temp
        table creation.
        
        Approved by:	Heikki
        ------------------------------------------------------------------------
        r2478 | sunny | 2008-05-23 08:29:08 +0300 (Fri, 23 May 2008) | 3 lines
        
        branches/5.1: Fix for bug# 36793. This is a back port from branches/zip. This
        code has been tested on a big-endian machine too.
        
        ------------------------------------------------------------------------
        r2480 | vasil | 2008-05-27 11:40:07 +0300 (Tue, 27 May 2008) | 11 lines
        
        branches/5.1:
        
        Fix Bug#36819 ut_usectime does not handle errors from gettimeofday
        
        by retrying gettimeofday() several times if it fails in ut_usectime().
        If it fails on all calls then return error to the caller to be handled
        at higher level.
        
        Update the variable innodb_row_lock_time_max in SHOW STATUS output only
        if ut_usectime() was successful.
        
        ------------------------------------------------------------------------
        r2482 | sunny | 2008-05-28 12:18:35 +0300 (Wed, 28 May 2008) | 5 lines
        
        branches/5.1: Fix for Bug#35602, "Failed to read auto-increment value from
        storage engine". The test for REPLACE was an error of ommission since it's
        classified as a simple INSERT. For REPLACE statements we don't acquire
        the special AUTOINC lock for AUTOINC_NEW_STYLE_LOCKING with this fix.
        
        ------------------------------------------------------------------------
        r2485 | vasil | 2008-05-28 16:01:14 +0300 (Wed, 28 May 2008) | 9 lines
        
        branches/5.1:
        
        Fix Bug#36149 Read buffer overflow in srv0start.c found during "make test"
        
        Use strncmp(3) instead of memcmp(3) to avoid reading past end of the string
        if it is empty (*str == '\0'). This bug is _not_ a buffer overflow.
        
        Discussed with:	Sunny (via IM)
        
        ------------------------------------------------------------------------
      dde455e8
  19. 21 May, 2008 1 commit
  20. 19 May, 2008 1 commit
    • vasil's avatar
      branches/zip: · 34af2c97
      vasil authored
      Non-functional change - conform to our coding style and make more readbale
      (hopefully) by not using variables with names "v" and "w".
      34af2c97