From: Date: October 29 2008 12:39pm Subject: bzr commit into mysql-5.1 branch (mattias.jonsson:2679) Bug#39084 List-Archive: http://lists.mysql.com/commits/57296 X-Bug: 39084 Message-Id: <20081029113920.F0474165C101@witty.localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT #At file:///Users/mattiasj/clones/bzrroot/b39084-51-bugteam/ 2679 Mattias Jonsson 2008-10-29 Bug#39084: Getting intermittent errors with statement-based binary logging Problem was that partitioning cached the table flags. These flags could change due to TRANSACTION LEVEL changes. Solution was to remove the cache and always return the table flags from the first partition (if the handler was initialized). added: mysql-test/r/partition_innodb_stmt.result mysql-test/t/partition_innodb_stmt.test modified: sql/ha_partition.cc sql/ha_partition.h per-file messages: mysql-test/r/partition_innodb_stmt.result Bug#39084: Getting intermittent errors with statement-based binary logging New test result file mysql-test/t/partition_innodb_stmt.test Bug#39084: Getting intermittent errors with statement-based binary logging New test file sql/ha_partition.cc Bug#39084: Getting intermittent errors with statement-based binary logging Removed m_table_flags, and added m_handler_status. Moved some variable initializations. Updated some comments. Added checks that all partitions have the same table flags. sql/ha_partition.h Bug#39084: Getting intermittent errors with statement-based binary logging Removed m_table_flags, and added m_handler_status. Updated some comments. Always return the first partitions table flags, instead of using cached table flags. === added file 'mysql-test/r/partition_innodb_stmt.result' --- a/mysql-test/r/partition_innodb_stmt.result 1970-01-01 00:00:00 +0000 +++ b/mysql-test/r/partition_innodb_stmt.result 2008-10-29 11:39:08 +0000 @@ -0,0 +1,30 @@ +#connection default +SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ; +CREATE TABLE t1 +( +id SMALLINT NOT NULL, +PRIMARY KEY (id) +) ENGINE=innodb +PARTITION BY RANGE (id) +( +PARTITION p1 VALUES LESS THAN (2), +PARTITION p2 VALUES LESS THAN (4), +PARTITION p3 VALUES LESS THAN (10) +); +FLUSH TABLES; +INSERT INTO t1 VALUES (1),(2),(3); +FLUSH TABLES; +SET TRANSACTION ISOLATION LEVEL READ COMMITTED; +BEGIN; +SELECT * FROM t1; +id +1 +2 +3 +#connection con1 +SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; +BEGIN; +INSERT INTO t1 VALUES(7); +COMMIT; +COMMIT; +DROP TABLE t1; === added file 'mysql-test/t/partition_innodb_stmt.test' --- a/mysql-test/t/partition_innodb_stmt.test 1970-01-01 00:00:00 +0000 +++ b/mysql-test/t/partition_innodb_stmt.test 2008-10-29 11:39:08 +0000 @@ -0,0 +1,39 @@ +--source include/have_binlog_format_statement.inc +--source include/have_innodb.inc + +--echo #connection default +SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ; + +CREATE TABLE t1 +( + id SMALLINT NOT NULL, + PRIMARY KEY (id) +) ENGINE=innodb +PARTITION BY RANGE (id) +( + PARTITION p1 VALUES LESS THAN (2), + PARTITION p2 VALUES LESS THAN (4), + PARTITION p3 VALUES LESS THAN (10) +); + +FLUSH TABLES; +INSERT INTO t1 VALUES (1),(2),(3); + +FLUSH TABLES; +SET TRANSACTION ISOLATION LEVEL READ COMMITTED; +BEGIN; +SELECT * FROM t1; + +connect (con1, localhost, root,,); +connection con1; + +--echo #connection con1 +SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; +BEGIN; +INSERT INTO t1 VALUES(7); +COMMIT; + +disconnect con1; +connection default; +COMMIT; +DROP TABLE t1; === modified file 'sql/ha_partition.cc' --- a/sql/ha_partition.cc 2008-10-06 13:14:20 +0000 +++ b/sql/ha_partition.cc 2008-10-29 11:39:08 +0000 @@ -160,8 +160,7 @@ const uint ha_partition::NO_CURRENT_PART ha_partition::ha_partition(handlerton *hton, TABLE_SHARE *share) :handler(hton, share), m_part_info(NULL), m_create_handler(FALSE), - m_is_sub_partitioned(0), is_clone(FALSE), auto_increment_lock(FALSE), - auto_increment_safe_stmt_log_lock(FALSE) + m_is_sub_partitioned(0) { DBUG_ENTER("ha_partition::ha_partition(table)"); init_handler_variables(); @@ -181,10 +180,8 @@ ha_partition::ha_partition(handlerton *h */ ha_partition::ha_partition(handlerton *hton, partition_info *part_info) - :handler(hton, NULL), m_part_info(part_info), - m_create_handler(TRUE), - m_is_sub_partitioned(m_part_info->is_sub_partitioned()), is_clone(FALSE), - auto_increment_lock(FALSE), auto_increment_safe_stmt_log_lock(FALSE) + :handler(hton, NULL), m_part_info(part_info), m_create_handler(TRUE), + m_is_sub_partitioned(m_part_info->is_sub_partitioned()) { DBUG_ENTER("ha_partition::ha_partition(part_info)"); init_handler_variables(); @@ -231,7 +228,7 @@ void ha_partition::init_handler_variable m_innodb= FALSE; m_extra_cache= FALSE; m_extra_cache_size= 0; - m_table_flags= HA_FILE_BASED | HA_REC_NOT_IN_SEQ; + m_handler_status= handler_not_initialized; m_low_byte_first= 1; m_part_field_array= NULL; m_ordered_rec_buffer= NULL; @@ -240,6 +237,9 @@ void ha_partition::init_handler_variable m_last_part= 0; m_rec0= 0; m_curr_key_info= 0; + is_clone= FALSE, + auto_increment_lock= FALSE; + auto_increment_safe_stmt_log_lock= FALSE; /* this allows blackhole to work properly */ @@ -333,6 +333,7 @@ ha_partition::~ha_partition() bool ha_partition::initialise_partition(MEM_ROOT *mem_root) { handler **file_array, *file; + ulonglong check_table_flags; DBUG_ENTER("ha_partition::initialise_partition"); if (m_create_handler) @@ -345,11 +346,9 @@ bool ha_partition::initialise_partition( else if (!table_share || !table_share->normalized_path.str) { /* - Called with dummy table share (delete, rename and alter table) - Don't need to set-up table flags other than - HA_FILE_BASED here + Called with dummy table share (delete, rename and alter table). + Don't need to set-up anything. */ - m_table_flags|= HA_FILE_BASED | HA_REC_NOT_IN_SEQ; DBUG_RETURN(0); } else if (get_from_handler_file(table_share->normalized_path.str, mem_root)) @@ -361,15 +360,12 @@ bool ha_partition::initialise_partition( We create all underlying table handlers here. We do it in this special method to be able to report allocation errors. - Set up table_flags, low_byte_first, primary_key_is_clustered and + Set up low_byte_first, primary_key_is_clustered and has_transactions since they are called often in all kinds of places, other parameters are calculated on demand. - HA_FILE_BASED is always set for partition handler since we use a - special file for handling names of partitions, engine types. - HA_CAN_GEOMETRY, HA_CAN_FULLTEXT, HA_CAN_SQL_HANDLER, HA_DUPLICATE_POS, - HA_CAN_INSERT_DELAYED is disabled until further investigated. + Verify that all partitions have the same table_flags. */ - m_table_flags= (ulong)m_file[0]->ha_table_flags(); + check_table_flags= m_file[0]->ha_table_flags(); m_low_byte_first= m_file[0]->low_byte_first(); m_pkey_is_clustered= TRUE; file_array= m_file; @@ -384,12 +380,13 @@ bool ha_partition::initialise_partition( } if (!file->primary_key_is_clustered()) m_pkey_is_clustered= FALSE; - m_table_flags&= file->ha_table_flags(); + if (check_table_flags != file->ha_table_flags()) + { + my_error(ER_MIX_HANDLER_ERROR, MYF(0)); + DBUG_RETURN(1); + } } while (*(++file_array)); - m_table_flags&= ~(HA_CAN_GEOMETRY | HA_CAN_FULLTEXT | HA_DUPLICATE_POS | - HA_CAN_SQL_HANDLER | HA_CAN_INSERT_DELAYED | - HA_PRIMARY_KEY_REQUIRED_FOR_POSITION); - m_table_flags|= HA_FILE_BASED | HA_REC_NOT_IN_SEQ; + m_handler_status= handler_initialized; DBUG_RETURN(0); } @@ -2410,6 +2407,7 @@ int ha_partition::open(const char *name, handler **file; char name_buff[FN_REFLEN]; bool is_not_tmp_table= (table_share->tmp_table == NO_TMP_TABLE); + ulonglong check_table_flags; DBUG_ENTER("ha_partition::open"); DBUG_ASSERT(table->s == table_share); @@ -2458,7 +2456,7 @@ int ha_partition::open(const char *name, } /* Recalculate table flags as they may change after open */ - m_table_flags= m_file[0]->ha_table_flags(); + check_table_flags= m_file[0]->ha_table_flags(); file= m_file; do { @@ -2470,11 +2468,9 @@ int ha_partition::open(const char *name, m_no_locks+= (*file)->lock_count(); name_buffer_ptr+= strlen(name_buffer_ptr) + 1; set_if_bigger(ref_length, ((*file)->ref_length)); - m_table_flags&= (*file)->ha_table_flags(); + if (check_table_flags != (*file)->ha_table_flags()) + goto err_handler; } while (*(++file)); - m_table_flags&= ~(HA_CAN_GEOMETRY | HA_CAN_FULLTEXT | HA_DUPLICATE_POS | - HA_CAN_SQL_HANDLER | HA_CAN_INSERT_DELAYED); - m_table_flags|= HA_FILE_BASED | HA_REC_NOT_IN_SEQ; key_used_on_scan= m_file[0]->key_used_on_scan; implicit_emptied= m_file[0]->implicit_emptied; /* @@ -2526,6 +2522,7 @@ int ha_partition::open(const char *name, calling open on all individual handlers. */ info(HA_STATUS_VARIABLE | HA_STATUS_CONST); + m_handler_status= handler_opened; DBUG_RETURN(0); err_handler: @@ -2595,6 +2592,7 @@ repeat: goto repeat; } + m_handler_status= handler_closed; DBUG_RETURN(0); } @@ -4763,7 +4761,7 @@ int ha_partition::info(uint flag) } } while (*(++file_array)); if (stats.records < 2 && - !(m_table_flags & HA_STATS_RECORDS_IS_EXACT)) + !(m_file[0]->ha_table_flags() & HA_STATS_RECORDS_IS_EXACT)) stats.records= 2; if (stats.records > 0) stats.mean_rec_length= (ulong) (stats.data_file_length / stats.records); === modified file 'sql/ha_partition.h' --- a/sql/ha_partition.h 2008-10-01 10:14:55 +0000 +++ b/sql/ha_partition.h 2008-10-29 11:39:08 +0000 @@ -85,8 +85,15 @@ private: for this since the MySQL Server sometimes allocating the handler object without freeing them. */ - longlong m_table_flags; ulong m_low_byte_first; + enum enum_handler_status + { + handler_not_initialized= 0, + handler_initialized, + handler_opened, + handler_closed + }; + enum_handler_status m_handler_status; uint m_reorged_parts; // Number of reorganised parts uint m_tot_parts; // Total number of partitions; @@ -585,6 +592,8 @@ public: The partition handler will support whatever the underlying handlers support except when specifically mentioned below about exceptions to this rule. + NOTE: This cannot be cached since it can depend on TRANSACTION ISOLATION + LEVEL which is dynamic, see bug#39084. HA_READ_RND_SAME: Not currently used. (Means that the handler supports the rnd_same() call) @@ -709,9 +718,34 @@ public: transfer those calls into index_read and other calls in the index scan module. (NDB) + + HA_PRIMARY_KEY_REQUIRED_FOR_POSITION: + Does the storage engine need a PK for position? + Used with hidden primary key in InnoDB. + Hidden primary keys annot be supported by partitioning, since the + partitioning expressions columns must be a part of the primary key. + (InnoDB) + + HA_FILE_BASED is always set for partition handler since we use a + special file for handling names of partitions, engine types. + HA_REC_NOT_IN_SEQ is always set for partition handler since we cannot + guarantee that the records will be returned in sequence. + HA_CAN_GEOMETRY, HA_CAN_FULLTEXT, HA_CAN_SQL_HANDLER, HA_DUPLICATE_POS, + HA_CAN_INSERT_DELAYED, HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is disabled + until further investigated. */ - virtual ulonglong table_flags() const - { return m_table_flags; } + virtual Table_flags table_flags() const + { + if (m_handler_status < handler_initialized || + m_handler_status >= handler_closed) + return (HA_FILE_BASED | HA_REC_NOT_IN_SEQ); + else + return (m_file[0]->ha_table_flags() & + ~(HA_CAN_GEOMETRY | HA_CAN_FULLTEXT | HA_DUPLICATE_POS | + HA_CAN_SQL_HANDLER | HA_CAN_INSERT_DELAYED | + HA_PRIMARY_KEY_REQUIRED_FOR_POSITION)) | + (HA_FILE_BASED | HA_REC_NOT_IN_SEQ); + } /* This is a bitmap of flags that says how the storage engine