mariadb

Commit Graph

Author	SHA1	Message	Date
marko	5983048f06	branches/zip: Minor cleanup. btr_search_drop_page_hash_index(): Add const qualifiers to the local variables page, rec, and index, to ensure that they are not modified by this function. page_get_infimum_offset(), page_get_supremum_offset(): New functions. page_get_infimum_rec(), page_get_supremum_rec(): Replaced by const-preserving macros that invoke the accessor functions.	17 years ago
marko	855c2aa891	branches/zip: btr0btr.c: Add more UNIV_BTR_DEBUG checks. This should help in tracking down issue #63 (memory corruption). UNIV_BTR_DEBUG is currently enabled in univ.i. btr_root_fseg_validate(): New function, for validating a file segment header on a B-tree root page. btr_root_block_get(), btr_free_but_not_root(), btr_root_raise_and_insert(), btr_discard_only_page_on_level(): Check PAGE_BTR_SEG_LEAF and PAGE_BTR_SEG_TOP on the root page with btr_root_fseg_validate(). btr_root_raise_and_insert(): Move the assertion dict_index_get_page(index) == page_get_page_no(root) inside UNIV_BTR_DEBUG. It was previously enabled by UNIV_DEBUG. btr_free_root(): Check PAGE_BTR_SEG_TOP on the root page with btr_root_fseg_validate().	17 years ago
vasil	46b312cd4e	branches/zip: Add a test case to check that mysqld does not crash when running ANALYZE TABLE with different values for innodb_stats_sample_pages. Suggested by: Marko Approved by: Marko	17 years ago
vasil	733e615e6d	branches/zip: Limit the number of the pages that are sampled so it is never greater than the total number of pages in the index. The parameter that specifies the number of pages to test is global for all tables. By limiting it this way we allow the user to set it "high" to suit "large" tables and to avoid unnecessary work for "small" tables (e.g. doing 100 dives in a table that has 5 pages, obviously testing some pages more than once). Suggested by: Ken Approved by: Marko	17 years ago
inaam	489abd2245	branches/zip Add missing semicolon. Introduced in r2602 which was obviously not compiled with UNIV_DEBUG.	17 years ago
vasil	4a4f1ef33f	branches/zip: Update the ChangeLog which has not been updated for a long time, phew!	17 years ago
marko	5d47801078	branches/zip: btr_cur_insert_if_possible(): Correct the function comment.	17 years ago
vasil	10436b05b0	branches/zip: Merge 2605:2617 from branches/5.1: ------------------------------------------------------------------------ r2609 \| sunny \| 2008-08-24 01:19:05 +0300 (Sun, 24 Aug 2008) \| 12 lines Changed paths: M /branches/5.1/handler/ha_innodb.cc M /branches/5.1/mysql-test/innodb-autoinc.result M /branches/5.1/mysql-test/innodb-autoinc.test branches/5.1: Fix for MySQL Bug#38839. Reset the statement level last value field in prebuilt. This field tracks the last value in an autoincrement interval. We use this value to check whether we need to update a table's AUTOINC counter, if the value written to a table is less than this value then we avoid updating the table's AUTOINC value in order to reduce mutex contention. If it's not reset (e.g., after a DELETE statement) then there is the possibility of missing updates to the table's AUTOINC counter resulting in a subsequent duplicate row error message under certain conditions (see the test case for details). Bug #38839 - auto increment does not work properly with InnoDB after update ------------------------------------------------------------------------ r2617 \| vasil \| 2008-09-09 15:46:17 +0300 (Tue, 09 Sep 2008) \| 47 lines Changed paths: M /branches/5.1/mysql-test/innodb.result branches/5.1: Merge a change from MySQL (fix the failing innodb test): ------------------------------------------------------------ revno: 2646.12.1 committer: Mattias Jonsson <mattiasj@mysql.com> branch nick: wl4176_2-51-bugteam timestamp: Mon 2008-08-11 20:02:03 +0200 message: Bug#20129: ALTER TABLE ... REPAIR PARTITION ... complains that partition is corrupt The main problem was that ALTER TABLE t ANALYZE/CHECK/OPTIMIZE/REPAIR PARTITION took another code path (over mysql_alter_table instead of mysql_admin_table) which differs in two ways: 1) alter table opens the tables in a different way than admin tables do resulting in returning with error before it tried the command 2) alter table does not start to send any diagnostic rows to the client which the lower admin functions continue to use -> resulting in assertion crash The fix: Remapped ALTER TABLE t ANALYZE/CHECK/OPTIMIZE/REPAIR PARTITION to use the same code path as ANALYZE/CHECK/OPTIMIZE/REPAIR TABLE t. Adding check in mysql_admin_table to setup the partition list for which partitions that should be used. Partitioned tables will still not work with REPAIR TABLE/PARTITION USE_FRM, since that requires moving partitions to tables, REPAIR TABLE t USE_FRM, and check that the data still fulfills the partitioning function and then move the table back to being a partition. NOTE: I have removed the following functions from the handler interface: analyze_partitions, check_partitions, optimize_partitions, repair_partitions Since they are not longer needed. THIS ALTERS THE STORAGE ENGINE API I have verified that OPTIMIZE TABLE actually rebuilds the table and calls ANALYZE. Approved by: Heikki	17 years ago
marko	31ef5d076d	branches/zip: ha_innobase::prepare_drop_index(): When there is a foreign key constraint, find a truly equivalent index for it. If none is available, refuse to drop the index. MySQL can drop an index when creating a "stronger" index. This was reported as Mantis issue #70 and MySQL Bug #38786. innodb-index.test: Add a test case. dict_foreign_find_equiv_index(): New function, to replace the incorrectly written function dict_table_find_equivalent_index(). dict_table_replace_index_in_foreign_list(): Simplify the implementation.	17 years ago
marko	7fbdbf3167	branches/zip: Write clarifying comments about the insert buffer and the "free" bits in the insert buffer bitmap in particular.	17 years ago
marko	2c7e2c3059	branches/zip: Clean up: do not assign index->id as a special case in fast index creation. In r1399, we wrote undo log records about creating indexes. The special undo log records were deemed unnecessary later, but this special handling was not removed then. row_merge_create_index(): Do not assign index->id. dict_build_index_def_step(): Unconditionally assign index->id.	17 years ago
marko	a3feeae4b0	branches/zip: page_zip_reorganize(): Compressed pages are always in compact format. Replace dict_table_is_comp(index->table) with TRUE.	17 years ago
marko	aab31a41c8	branches/zip: buf_LRU_get_free_block(): Add missing spaces to and correct the grammar of the warning message.	17 years ago
marko	1cda7bd8fe	branches/zip: thd_is_select(): Add the missing UNIV_INTERN to the definition. This function was introduced in r2606, when merging r2603 from branches/5.1.	17 years ago
vasil	e7f9daa5ac	branches/zip: Merge 2537:2605 from branches/5.1: ------------------------------------------------------------------------ r2545 \| vasil \| 2008-07-25 17:24:23 +0300 (Fri, 25 Jul 2008) \| 37 lines Changed paths: M /branches/5.1/handler/ha_innodb.cc branches/5.1: Fix Bug#38185 ha_innobase::info can hold locks even when called with HA_STATUS_NO_LOCK The fix is to call fsp_get_available_space_in_free_extents() from ha_innobase::info() only if HA_STATUS_NO_LOCK is not present in the flag AND change get_schema_tables_record() in MySQL's sql/sql_show.cc to call ::info() without HA_STATUS_NO_LOCK whenever a user issues SELECT FROM information_schema.tables; Without the change to sql/sql_show.cc this patch would lead to Bug#32440 resurfacing. I.e. delete_length would never be updated in ::info() and will remain 0 forever, resulting in the free space not being shown anywhere. This is the change to sql/sql_show.cc for reference, it needs to be committed to the MySQL repo before or at the same time with this change to ha_innodb.cc: --- patch begins here --- --- sql/sql_show.cc.orig 2008-07-23 09:32:14.000000000 +0300 +++ sql/sql_show.cc 2008-07-23 09:32:19.000000000 +0300 @@ -3549,8 +3549,7 @@ static int get_schema_tables_record(THD if(file) { - file->info(HA_STATUS_VARIABLE \| HA_STATUS_TIME \| HA_STATUS_AUTO \| - HA_STATUS_NO_LOCK); + file->info(HA_STATUS_VARIABLE \| HA_STATUS_TIME \| HA_STATUS_AUTO); enum row_type row_type = file->get_row_type(); switch (row_type) { case ROW_TYPE_NOT_USED: --- patch ends here --- Approved by: Heikki ------------------------------------------------------------------------ r2603 \| marko \| 2008-08-21 16:25:05 +0300 (Thu, 21 Aug 2008) \| 10 lines Changed paths: M /branches/5.1/handler/ha_innodb.cc M /branches/5.1/include/ha_prototypes.h M /branches/5.1/row/row0sel.c branches/5.1: Identify SELECT statements by thd_sql_command() == SQLCOM_SELECT instead of parsing the query string. This fixes MySQL Bug #37885 without us having to implement lexical analysis of SQL comments in yet another place. thd_is_select(): A new predicate. row_search_for_mysql(): Use thd_is_select(). Approved by Heikki. ------------------------------------------------------------------------	17 years ago
marko	157b8eebb5	branches/zip: Minor cleanup. dict_table_get_referenced_constraint(), dict_table_get_foreign_constraint(): Simplify the iteration loop. dict_table_find_equivalent_index(): Correct the function comment.	17 years ago
marko	a8382891f4	branches/zip: Cleanup: make dict_scan_to() a static function.	17 years ago
marko	3d3bd9877e	branches/zip: Introduce UNIV_LRU_DEBUG for debugging the LRU buffer pool cache, especially buf_pool->LRU_old and bpage->old. buf_LRU_old_adjust_len(), buf_LRU_remove_block(): Check that blocks in buf_pool->LRU_old have the "old" flag set and the blocks preceding buf_pool->LRU_old have the "old" flag clear. buf_LRU_add_block_low(), buf_relocate(): Check that buf_pool->LRU_old is the first block in the LRU list whose "old" flag is set. buf_LRU_free_block(): When replacing a control block in the LRU list with a control block for a compressed page, assert that the "old" flags in the neighboring LRU list entries grow monotonically. buf_page_set_old(): Assert that the "old" flags in the neighboring LRU list entries grow monotonically.	17 years ago
marko	81ec8249ac	branches/zip: buf_page_set_old(): Add the assertion ut_ad(bpage->in_LRU_list). Adjust the callers where necessary: buf_LRU_add_block_to_end_low() and buf_LRU_add_block_low().	17 years ago
marko	012b1cc795	branches/zip: flst_write_addr(), flst_read_addr(): Assert that no file list link is written or read below FIL_PAGE_DATA. This has helped diagnose issue#63.	17 years ago
sunny	3b81f2d266	branches/zip: Fix typo in comment.	17 years ago
marko	c976021ef4	branches/zip: buf_LRU_free_block(): When buf_page_is_old(b), we incremented buf_pool->LRU_old_len. However, we forgot to check if buf_pool->LRU_old happens to point to b's successor in the LRU list. If it does, we must assign buf_pool->LRU_old = b. The following invariants hold: In the LRU list, the "old" flag should grow monotonically, i.e., it is 0 for the first few items and 1 from thereafter. If buf_pool->LRU_old != NULL, it must point to the first item with old=1 in the LRU list, and there must be buf_pool->LRU_old_len old items in the list. This should fix Mantis issue#50 and issue#68.	17 years ago
vasil	4821672479	branches/zip: 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	17 years ago
vasil	a97b0cc383	branches/zip: Fix Mantis issue#66. Do not try to dereference the MySQL thread object if it is NULL. Approved by: Sunny (via IM)	17 years ago
marko	0ce4e51d8b	branches/zip: Correct some typos in comments.	17 years ago
marko	046c1d91b8	branches/zip: Fix terminology that was introduced in r2569: replace rbmode and "rollback mode" with rb_ctx and "rollback context". Suggested by Sunny.	17 years ago
marko	8a73d35603	branches/zip: In the rollback of incomplete transactions after crash 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.	17 years ago
marko	94a7cee3c2	branches/zip: Improve the diagnostics for tracking down issue#63 and issue#65. 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.	17 years ago
marko	07315cdba3	branches/zip: Always display a newline after ut_print_buf(stderr, ...).	17 years ago
marko	a9b6c6bcd3	branches/zip: Disable two debug assertions that attempt to call 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().	17 years ago
marko	d3c0caa90b	branches/zip: Simplify the interface to mem0pool.c. mem_heap_create_block(): Compute len and call mem_area_alloc() only once. mem_heap_block_free(): Call mem_area_free() only once. Approved by Heikki.	17 years ago
marko	76dea7ad1d	branches/zip: Dump the compressed page when fil_page_get_type() returns an incorrect value. This is to track down Mantis issue#63 and issue#65. buf_LRU_block_remove_hashed_page(), buf_flush_init_for_writing(): dump the compressed page before ut_error.	17 years ago
marko	9df7d7fa11	branches/zip: page_copy_rec_list_start(): Remove the bogus assertion that was added in r2554. When this function is called on an empty new_block, "ret" will always point to the page infimum.	17 years ago
marko	a9c1562fd0	branches/zip: page_copy_rec_list_start(): Add the assertion ut_a(ret_pos > 0) that was added to page_copy_rec_list_end() in r2552, as suggested by Sunny in Mantis issue#52.	17 years ago
vasil	a95ad23a11	branches/zip: Fix typo in comment.	17 years ago
marko	5e3bf6166b	branches/zip: page_copy_rec_list_end(): Add the assertion ut_a(ret_pos > 0) that was suggested by Sunny in Mantis issue#52.	17 years ago
vasil	59790220f8	branches/zip: Fix typo in comment.	17 years ago
inaam	aa87f6bd1f	branches/zip: issue#57 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	17 years ago
inaam	3468767c22	branches/zip issue# 33 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	17 years ago
sunny	8c22d953b3	branches/zip: The test for issue#36 assumed that AUTOCOMMIT=OFF. Add an explicit BEGIN.	17 years ago
inaam	47e159a3e2	branches/zip: Fixed a mistake in r2539 where I changed UNIV_INLINE to UNIV_INTERN for the wrong function in btr0sea.h	18 years ago
inaam	f2ad32be80	branches/zip: Merge 2524:2537 from branches/5.1: ------------------------------------------------------------------------ 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 ------------------------------------------------------------------------	18 years ago
vasil	1e298958c9	branches/zip: Fix the failing innodb-index test by adjusting the result to a new MySQL behavior. The author of the behavioral change Davi Arnaut <davi@mysql.com>, BZR-r2667: http://bazaar.launchpad.net/~mysql/mysql-server/mysql-5.1/revision/davi%40mysql.com-20080617141221-8yre8ys9j4uw3xx5?start_revid=joerg%40mysql.com-20080630105418-7qoe5ehomgrcdb89 has been asked if this change is intended: http://lists.mysql.com/internals/35805 Regardless of the answer we need to adjust to the new behavior to get the test to succeed. If they switch back, then we will revert this commit.	18 years ago
calvin	6e41b961a2	branches/zip: 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;..."	18 years ago
calvin	8b44913303	branches/zip: 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	18 years ago
vasil	f66c5e5d4a	branches/zip: 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$	18 years ago
vasil	ab4a89c67f	branches/zip: Merge 2489:2524 from branches/5.1: ------------------------------------------------------------------------ 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 ------------------------------------------------------------------------	18 years ago
sunny	a557763f87	branches/zip: Function doesn't exist, remove declaration.	18 years ago
vasil	232d8337bf	branches/zip: 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	18 years ago
vasil	6ef84d75fd	branches/zip: 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	18 years ago

1 2 3 4 5 ...

1208 Commits (5983048f06e1377dc01d1562794834220d575660) All Branches Search

1208 Commits (5983048f06e1377dc01d1562794834220d575660)

All Branches