From: magnus.blaudd Date: June 9 2011 9:35am Subject: bzr push into mysql-5.1-telco-7.0 branch (magnus.blaudd:4445 to 4448) Bug#37153 List-Archive: http://lists.mysql.com/commits/138924 X-Bug: 37153 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4448 magnus.blaudd@stripped 2011-06-09 Bug#37153 NDB Cluster reports affected rows incorrectly - This is a rewrite of the previous fix which commits the simple autocommit transaction before the update or delete loops asks the handler for number of affected rows. Thus still saving one roundtrip while returning the correct number of affected rows. - Refactor the compound if statements in exec_bulk_update and end_bulk_delete which controls when to no-commit or not. Adding DBUG_PRINT so it's possible to see which path is choosen when optimizing away roundtrips modified: sql/ha_ndbcluster.cc sql/sql_class.cc sql/sql_class.h 4447 magnus.blaudd@stripped 2011-06-08 ndb - remove usage r --init-rpl-role and --rpl-recovery-rank, thay are unused/unimplemented and will be removed in a future version anyway. modified: mysql-test/suite/ndb_rpl/my.cnf mysql-test/suite/ndb_rpl/t/ndb_rpl_break_3_chain.cnf mysql-test/suite/ndb_rpl/t/ndb_rpl_multi_binlog_update.cnf mysql-test/suite/ndb_team/my.cnf mysql-test/suite/rpl_ndb/my.cnf 4446 magnus.blaudd@stripped 2011-06-07 ndb - remove usage of my_malloc and my_free from ndbapi modified: storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp 4445 magnus.blaudd@stripped 2011-06-07 ndb - fix warning about conversion between different datatypes modified: storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp === modified file 'mysql-test/suite/ndb_rpl/my.cnf' --- a/mysql-test/suite/ndb_rpl/my.cnf 2011-05-13 07:40:50 +0000 +++ b/mysql-test/suite/ndb_rpl/my.cnf 2011-06-08 19:25:29 +0000 @@ -60,7 +60,6 @@ relay-log= slave-rela # Cluster only supports row format binlog-format= row -init-rpl-role= slave log-slave-updates master-retry-count= 10 @@ -83,8 +82,6 @@ skip-slave-start # test results will vary, thus a relative path is used. slave-load-tmpdir= ../../../tmp -rpl-recovery-rank= @mysqld.1.slave.server-id - [ENV] NDB_CONNECTSTRING= @mysql_cluster.1.ndb_connectstring MASTER_MYPORT= @mysqld.1.1.port === modified file 'mysql-test/suite/ndb_rpl/t/ndb_rpl_break_3_chain.cnf' --- a/mysql-test/suite/ndb_rpl/t/ndb_rpl_break_3_chain.cnf 2011-05-13 07:40:50 +0000 +++ b/mysql-test/suite/ndb_rpl/t/ndb_rpl_break_3_chain.cnf 2011-06-08 19:25:29 +0000 @@ -66,8 +66,6 @@ default-storage-engine=myisam # test results will vary, thus a relative path is used. slave-load-tmpdir= ../../../tmp -rpl-recovery-rank= @mysqld.1.cluster2.server-id - [mysqld.1.cluster3] log-bin= cluster3-bin relay-log= cluster3-relay-bin @@ -83,8 +81,6 @@ default-storage-engine=myisam # test results will vary, thus a relative path is used. slave-load-tmpdir= ../../../tmp -rpl-recovery-rank= @mysqld.1.cluster3.server-id - [ENV] SERVER_MYPORT_1= @mysqld.1.cluster1.port SERVER_MYPORT_2= @mysqld.1.cluster2.port === modified file 'mysql-test/suite/ndb_rpl/t/ndb_rpl_multi_binlog_update.cnf' --- a/mysql-test/suite/ndb_rpl/t/ndb_rpl_multi_binlog_update.cnf 2011-05-13 07:40:50 +0000 +++ b/mysql-test/suite/ndb_rpl/t/ndb_rpl_multi_binlog_update.cnf 2011-06-08 19:25:29 +0000 @@ -58,7 +58,6 @@ binlog_format=row [mysqld.1.slave] # Note no binlog on this slave server-id= 4 -init-rpl-role= slave skip-slave-start loose-skip-innodb slave-load-tmpdir= ../../../tmp @@ -69,7 +68,6 @@ ndb_connectstring= @mysql_cluster.slave. [mysqld.2.slave] # Note binlog on this slave, but not logging slave updates server-id= 5 -init-rpl-role= slave skip-slave-start loose-skip-innodb slave-load-tmpdir= ../../../tmp @@ -82,7 +80,6 @@ binlog_format=row [mysqld.3.slave] # Note binlog on this slave, with slave updates logged server-id= 6 -init-rpl-role= slave skip-slave-start loose-skip-innodb slave-load-tmpdir= ../../../tmp === modified file 'mysql-test/suite/ndb_team/my.cnf' --- a/mysql-test/suite/ndb_team/my.cnf 2011-04-15 09:31:03 +0000 +++ b/mysql-test/suite/ndb_team/my.cnf 2011-06-08 19:25:29 +0000 @@ -50,7 +50,6 @@ master-connect-retry= 1 log-bin= slave-bin relay-log= slave-relay-bin -init-rpl-role= slave log-slave-updates master-retry-count= 10 @@ -68,9 +67,6 @@ skip-slave-start # test results will vary, thus a relative path is used. slave-load-tmpdir= ../../../tmp -rpl-recovery-rank= @mysqld.1.slave.server-id - - [ENV] NDB_CONNECTSTRING= @mysql_cluster.1.ndb_connectstring MASTER_MYPORT= @mysqld.1.1.port === modified file 'mysql-test/suite/rpl_ndb/my.cnf' --- a/mysql-test/suite/rpl_ndb/my.cnf 2011-04-26 09:28:41 +0000 +++ b/mysql-test/suite/rpl_ndb/my.cnf 2011-06-08 19:25:29 +0000 @@ -60,7 +60,6 @@ relay-log= slave-rela # Cluster only supports row format binlog-format= row -init-rpl-role= slave log-slave-updates master-retry-count= 10 @@ -83,8 +82,6 @@ skip-slave-start # test results will vary, thus a relative path is used. slave-load-tmpdir= ../../../tmp -rpl-recovery-rank= @mysqld.1.slave.server-id - [ENV] NDB_CONNECTSTRING= @mysql_cluster.1.ndb_connectstring MASTER_MYPORT= @mysqld.1.1.port === modified file 'sql/ha_ndbcluster.cc' --- a/sql/ha_ndbcluster.cc 2011-06-07 12:36:05 +0000 +++ b/sql/ha_ndbcluster.cc 2011-06-09 09:20:47 +0000 @@ -4650,26 +4650,79 @@ int ha_ndbcluster::bulk_update_row(const int ha_ndbcluster::exec_bulk_update(uint *dup_key_found) { + NdbTransaction* trans= m_thd_ndb->trans; DBUG_ENTER("ha_ndbcluster::exec_bulk_update"); *dup_key_found= 0; - if (m_thd_ndb->m_unsent_bytes && - !thd_allow_batch(table->in_use) && - (!m_thd_ndb->m_handler || - m_blobs_pending)) + + // m_handler must be NULL or point to _this_ handler instance + assert(m_thd_ndb->m_handler == NULL || m_thd_ndb->m_handler == this); + + if (m_thd_ndb->m_handler && + m_read_before_write_removal_possible) { + /* + This is an autocommit involving only one table and rbwr is on + + Commit the autocommit transaction early(before the usual place + in ndbcluster_commit) in order to: + 1) save one round trip, "no-commit+commit" converted to "commit" + 2) return the correct number of updated and affected rows + to the update loop(which will ask handler in rbwr mode) + */ + DBUG_PRINT("info", ("committing auto-commit+rbwr early")); uint ignore_count= 0; - if (execute_no_commit(m_thd_ndb, m_thd_ndb->trans, - m_ignore_no_key || m_read_before_write_removal_used, - &ignore_count) != 0) + const int ignore_error= 1; + if (execute_commit(m_thd_ndb, trans, + m_thd_ndb->m_force_send, ignore_error, + &ignore_count) != 0) { no_uncommitted_rows_execute_failure(); - DBUG_RETURN(ndb_err(m_thd_ndb->trans)); + DBUG_RETURN(ndb_err(trans)); } + DBUG_PRINT("info", ("ignore_count: %u", ignore_count)); assert(m_rows_changed >= ignore_count); assert(m_rows_updated >= ignore_count); m_rows_changed-= ignore_count; m_rows_updated-= ignore_count; + DBUG_RETURN(0); + } + + if (m_thd_ndb->m_unsent_bytes == 0) + { + DBUG_PRINT("exit", ("skip execute - no unsent bytes")); + DBUG_RETURN(0); + } + + if (thd_allow_batch(table->in_use)) + { + /* + Turned on by @@transaction_allow_batching=ON + or implicitly by slave exec thread + */ + DBUG_PRINT("exit", ("skip execute - transaction_allow_batching is ON")); + DBUG_RETURN(0); + } + + if (m_thd_ndb->m_handler && + !m_blobs_pending) + { + // Execute at commit time(in 'ndbcluster_commit') to save a round trip + DBUG_PRINT("exit", ("skip execute - simple autocommit")); + DBUG_RETURN(0); + } + + uint ignore_count= 0; + if (execute_no_commit(m_thd_ndb, trans, + m_ignore_no_key || m_read_before_write_removal_used, + &ignore_count) != 0) + { + no_uncommitted_rows_execute_failure(); + DBUG_RETURN(ndb_err(trans)); } + assert(m_rows_changed >= ignore_count); + assert(m_rows_updated >= ignore_count); + m_rows_changed-= ignore_count; + m_rows_updated-= ignore_count; DBUG_RETURN(0); } @@ -5005,25 +5058,76 @@ bool ha_ndbcluster::start_bulk_delete() int ha_ndbcluster::end_bulk_delete() { + NdbTransaction* trans= m_thd_ndb->trans; DBUG_ENTER("end_bulk_delete"); assert(m_is_bulk_delete); // Don't allow end() without start() m_is_bulk_delete = false; - if (m_thd_ndb->m_unsent_bytes && - !thd_allow_batch(table->in_use) && - !m_thd_ndb->m_handler) + // m_handler must be NULL or point to _this_ handler instance + assert(m_thd_ndb->m_handler == NULL || m_thd_ndb->m_handler == this); + + if (m_thd_ndb->m_handler && + m_read_before_write_removal_possible) { + /* + This is an autocommit involving only one table and rbwr is on + + Commit the autocommit transaction early(before the usual place + in ndbcluster_commit) in order to: + 1) save one round trip, "no-commit+commit" converted to "commit" + 2) return the correct number of updated and affected rows + to the delete loop(which will ask handler in rbwr mode) + */ + DBUG_PRINT("info", ("committing auto-commit+rbwr early")); uint ignore_count= 0; - if (execute_no_commit(m_thd_ndb, m_thd_ndb->trans, - m_ignore_no_key || m_read_before_write_removal_used, - &ignore_count) != 0) + const int ignore_error= 1; + if (execute_commit(m_thd_ndb, trans, + m_thd_ndb->m_force_send, ignore_error, + &ignore_count) != 0) { no_uncommitted_rows_execute_failure(); - DBUG_RETURN(ndb_err(m_thd_ndb->trans)); + DBUG_RETURN(ndb_err(trans)); } + DBUG_PRINT("info", ("ignore_count: %u", ignore_count)); assert(m_rows_deleted >= ignore_count); m_rows_deleted-= ignore_count; + DBUG_RETURN(0); + } + + if (m_thd_ndb->m_unsent_bytes == 0) + { + DBUG_PRINT("exit", ("skip execute - no unsent bytes")); + DBUG_RETURN(0); + } + + if (thd_allow_batch(table->in_use)) + { + /* + Turned on by @@transaction_allow_batching=ON + or implicitly by slave exec thread + */ + DBUG_PRINT("exit", ("skip execute - transaction_allow_batching is ON")); + DBUG_RETURN(0); + } + + if (m_thd_ndb->m_handler) + { + // Execute at commit time(in 'ndbcluster_commit') to save a round trip + DBUG_PRINT("exit", ("skip execute - simple autocommit")); + DBUG_RETURN(0); + } + + uint ignore_count= 0; + if (execute_no_commit(m_thd_ndb, trans, + m_ignore_no_key || m_read_before_write_removal_used, + &ignore_count) != 0) + { + no_uncommitted_rows_execute_failure(); + DBUG_RETURN(ndb_err(trans)); } + + assert(m_rows_deleted >= ignore_count); + m_rows_deleted-= ignore_count; DBUG_RETURN(0); } @@ -7097,58 +7201,18 @@ int ndbcluster_commit(handlerton *hton, if (thd_ndb->m_handler && thd_ndb->m_handler->m_read_before_write_removal_possible) { -#ifndef NDB_WITHOUT_READ_BEFORE_WRITE_REMOVAL - /* Autocommit with read-before-write removal - * Some operations in this autocommitted statement have not - * yet been executed - * They will be executed here as part of commit, and the results - * (rowcount, message) sent back to the client will then be modified - * according to how the execution went. - * This saves a single roundtrip in the autocommit case - */ - uint ignore_count= 0; - res= execute_commit(thd_ndb, trans, THDVAR(thd, force_send), - TRUE, &ignore_count); - if (!res && ignore_count) - { - DBUG_PRINT("info", ("AutoCommit + RBW removal, ignore_count=%u", - ignore_count)); - /* We have some rows to ignore, modify recorded results, - * regenerate result message as required. - */ - thd->row_count_func-= ignore_count; - - ha_rows affected= 0; - char buff[ STRING_BUFFER_USUAL_SIZE ]; - const char* msg= NULL; - if (thd->lex->sql_command == SQLCOM_DELETE) - { - assert(thd_ndb->m_handler->m_rows_deleted >= ignore_count); - affected= (thd_ndb->m_handler->m_rows_deleted-= ignore_count); - } - else - { - DBUG_PRINT("info", ("Update : message was %s", - thd->main_da.message())); - assert(thd_ndb->m_handler->m_rows_updated >= ignore_count); - affected= (thd_ndb->m_handler->m_rows_updated-= ignore_count); - /* For update in this scenario, we set found and changed to be - * the same as affected - * Regenerate the update message - */ - sprintf(buff, ER(ER_UPDATE_INFO), (ulong)affected, (ulong)affected, - (ulong) thd->cuted_fields); - msg= buff; - DBUG_PRINT("info", ("Update : message changed to %s", - msg)); - } - - /* Modify execution result + optionally message */ - thd->main_da.modify_affected_rows(affected, msg); + /* + This is an autocommit involving only one table and + rbwr is on, thus the transaction has already been + committed in exec_bulk_update() or end_bulk_delete() + */ + DBUG_PRINT("info", ("autocommit+rbwr, transaction already comitted")); + if (trans->commitStatus() != NdbTransaction::Committed) + { + sql_print_error("found uncomitted autocommit+rbwr transaction, " + "commit status: %d", trans->commitStatus()); + abort(); } -#else - abort(); // Should never come here without rbwr support -#endif } else res= execute_commit(thd_ndb, trans, THDVAR(thd, force_send), FALSE); === modified file 'sql/sql_class.cc' --- a/sql/sql_class.cc 2011-04-08 11:06:53 +0000 +++ b/sql/sql_class.cc 2011-06-09 09:20:47 +0000 @@ -583,24 +583,6 @@ Diagnostics_area::set_error_status(THD * m_status= DA_ERROR; } -/** - * modify_affected_rows - * Modify the number of affected rows, and optionally the - * message in the Diagnostics area - */ -void -Diagnostics_area::modify_affected_rows(ha_rows new_affected_rows, - const char* new_message) -{ - DBUG_ASSERT(is_set()); - DBUG_ASSERT(m_status == DA_OK); - DBUG_ASSERT(can_overwrite_status); - - m_affected_rows= new_affected_rows; - if (new_message) - strmake(m_message, new_message, sizeof(m_message) - 1); -} - /** Mark the diagnostics area as 'DISABLED'. === modified file 'sql/sql_class.h' --- a/sql/sql_class.h 2011-04-08 13:59:44 +0000 +++ b/sql/sql_class.h 2011-06-09 09:20:47 +0000 @@ -1163,9 +1163,6 @@ public: void set_eof_status(THD *thd); void set_error_status(THD *thd, uint sql_errno_arg, const char *message_arg); - /* Modify affected rows count and optionally message */ - void modify_affected_rows(ha_rows new_affected_rows, const char *new_message= 0); - void disable_status(); void reset_diagnostics_area(); === modified file 'storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp' --- a/storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp 2011-06-07 12:48:01 +0000 +++ b/storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp 2011-06-07 13:28:40 +0000 @@ -2193,7 +2193,7 @@ NdbIndexStatImpl::MemDefault::mem_alloc( { size += 4 - size % 4; } - Item* item = (Item*)my_malloc(sizeof(Item) + size, MYF(0)); + Item* item = (Item*)malloc(sizeof(Item) + size); if (item != 0) { item->m_magic = MemMagic; @@ -2214,7 +2214,7 @@ NdbIndexStatImpl::MemDefault::mem_free(v assert(item->m_magic == MemMagic); size_t size = item->m_size; item->m_magic = 0; - my_free(item, MYF(0)); + free(item); assert(m_used >= size); m_used -= size; } No bundle (reason: useless for push emails).