From: magnus.blaudd Date: June 9 2011 9:25am Subject: bzr commit into mysql-5.1-telco-7.0 branch (magnus.blaudd:4448) Bug#37153 List-Archive: http://lists.mysql.com/commits/138923 X-Bug: 37153 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5692676575183644303==" --===============5692676575183644303== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///data0/magnus/mysql/7.0-bug37153/ based on revid:magnus.blaudd@stripped 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 === 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(); --===============5692676575183644303== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/magnus.blaudd@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: magnus.blaudd@stripped\ # q22hqu71hglycbwv # target_branch: file:///data0/magnus/mysql/7.0-bug37153/ # testament_sha1: 1a5ff48d4a9a42ee5b0bceb680716b4e63db5584 # timestamp: 2011-06-09 11:25:10 +0200 # base_revision_id: magnus.blaudd@stripped\ # aqhnq8s8tcpfke1e # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWbJU9nEABgD/gFYQDAB7//// fqXeAL////5gDJ2+Z5h1djQkoJNRNAa0FsBVGzCtAKpjgGEYTTEMAgGQAwjTJkwjAQ0ElCBiYhE2 kSfoyjNNJpqPJlMgwEBiMgGppDEQADQAAAAAAAAABEkomU/VPSaDTGjU09MjUDTEDQAGgaNMjQ4B hGE0xDAIBkAMI0yZMIwENBIkTIBGgExJkekDU00xRPTRtU9IaD1NqB7VMAgHIdzpM5FyWzaL+tJA kkaqq+xy4OYzbuX4Fx5D/RfE6A3lDDg0XAYWnjhX5TccpGJvCHvIHpKqvs2b+R5hkw4YUdjz45nf LGwZADTNoECPR/9Xr6Ih669bgxYk2rGllzUi1WffSZc7KQOJeMrOmaSEKgIkUEkWERhCQQxJq5SG R5nGWU+odaIYTWG42X6vbZJILIPlG6H4A7DE8nJQj+VjoB4hBtkLw4iW0pQQIBphcHIMkZwSVMoU wYyzlpSFKSdRyaOYMMqWXPFEMCUaQIOPnRDtsQQgE0MI7eBKhiR3j5tTJjP2JXokZFFKQUn0NdEy CsQf81foIDaql8SJHmKFdP9r5wfqVtEZdjcLMgZeTwfrJEmTRrJBrmtua4n2MkzIoG1ajiXTybGX sxNQ85m1Qrpa7f7Ey7FacCRhdiVJ3k7Q/ummU7kULSLbMCw+PHjSxSE4nlgRo94560HSZxMhl+wr kUR946Kc9haGY8SwzGYZ/vKvXiQBvOQ1JeFBvA4OOmktRHgunvX9XY4HnIF5qkhov8HbERZ8fbql FosiwKKGgMtnjY+00i4sSO6OWiEMWmpuyDWBoWBIc0mmNTDgQWg+pTg3sg07FdKB9MQox8T5H9S8 9uysREJVkGIoYmMmneA/uR5H9A7UFrLjQZTYYoUyXgEqbgN5uD6gHWnqJoICXQtHkR3dRhlYT7/A Q46jdRPD/DXe7c3CA3tM0QEL8EdA6B0Z3I26rV714nqoJ5Skt/GEBDRJi8QP4xQOxTlG4LQgAc0/ nCvpMKDOJURbYyYlVlItLKW1LLSxRVkCHzEd9Qoi9YF+BgazGnMqBi0GWuM5RPeQY+mAelrQiAeh m45vurAJ4z5yElc+FrPRB089JkY0rt4kaiJq0hfvprJRiwsT8mLXIkcfjfgmd4P1miI4gqVNxsWo g5uQqaxHLhu+2NjN7IxlGOdN3U2lGAcBF+M6MMjZ3Ff3ESxOM6ta7OJNd/Sm1arr0txfuOIYkZF6 vI1tg9tcW3PcVvY4JEsCG8tuJ1PwIu/z++Kq095vNpw/iU9lVcdy9ZgWpj1D3hCGNBhfyNhSolyK 2iNk/K6fG/C0t8A7iRl3RmSOZOwx6rzL9del0YWTha1QFsd87Zari5Ox1OSka9MfIXO+ZdcYETDG xXG075Fc7cXFLTtSNBKjxyUap3EFymzhZANqbNddiTDKYzjnq9uRmYclr03Wa9maRi4sCxXDhrMG jziEWPOetD7TrB+glMHYkLpl43S9DuQuOUodBpfHCcHPFQbZYGNZIYy6cIG3k2LYq8+mV+NgSCEj gxHs76B4ZldlfBhWqOZZhzJm04ma3ltnQ27y/DJRww753bI7uDuaTrAThDo5hcKlZ4zFHFVNRDCR wkLQT2B8zjIXjvli3udDcTKc1gntXBKDxF7T2/gpLAQMogP9iP1tNE026AeXav8BXHbzDegMIYec G8f0BoH3hQc4WBvc4MXXIcgD0A4AbYzTB1AbfnUK77RvgOkHxB2zoAcZth0jQfKHEahgOIdBsnCU UAfoWGgVmEhl4LGakpFpAsBB81AVwVAkEvrIKDJlooEG4uIMS0wLy0KrPE+JkiQxdXafk0tsqLRn 4jOvf290y9OJHs8M0bY6CXIgNSFuySN6PlOwXcgKH1PyD6+4Vx1WuEy6Bb0BCQvouetBrCyi0E0a mPxFxAwtnCogKJHTaSGpen143iD8oxIIPzX8yRzIolvF2JkkqG2DEmM/qVCWwPqgKruTLII7cz28 6Bkfp1Qe0RqCwZ/JAt5MW4ZaHsNx/icipLTIRtF7yszYaztjvR4lkbSQbZFrmKEBqStxQFtd63+M iSg/7ILwLAg+pT3WnO4n/MYp3AGtQcQNaE2xJlFMIyBCLEn8jRHK9zd3+2ZDX28Zcm+dRPAlt3Wp XID3mgUIEGsmHvAmIPmmkfA8jShtQHA+0Z9dpfabDYHOZYBZQA0MHxsOM7K9LjmFmfEpJjOJA0SI USCQ4lUhrNSkidGTPMsMzwIEj3lccz5KLfl8n8TDyVi9FWtUM1AhmauFBIYbSA9YxbGZHMMjPZ+o UWaHAljijF7xoN4ybIzXre89e1+zlI5L+kksednk584OAtW0JdxouR4QWMawV0C6CWAX4EoUPqwt 9B4oIReRrDbeQFekSEdxAYcBXJc/wMy4hFCoMYvEnDtcGbMbe5tJiLV2TyGkhoOsETXehzuwFnQ5 htcYJQfDn6iKJHm33EHHMCCgj2eJoJGxUQE1wLWvoxj7D7AauhtS2IasItSA2eZ/A8BECoH4hQPt 5KfmImWPvSBo6Xnp0R3Ij3HTbtSd30Nwj8n9wf3CEchoLTsehZsXIw9A0aK0+A1kMPxH8Bcc0a7j wzOwb7JFx6xnwO7V7u2dFkBsG0weVprE17x0SyBphYL3CD5iZNiViOxAjuvXhLd4FStB0Cqqu8Cw XvNRaSqdR+tdgmNqCw3iQymIeFAwFqYXBSXkChyK4RWFeoWLlIUrdqxFM6xU/fFKa5rgtA+gj4Fg B9s6GmT9G8WM8kkgfsiURhfM4/g7wRo03gG08uC1c62HU8UgLG2KXpUhCNy1FoXXBVFMViVPyJlt FFqaIGyTCpBIX48YQWFq9Jar1OhIJMaLSYmdhBvBr9i4nVJVLxYjuBFR0VctauWZEOwmJFVBqzXc QV6QphEmUGDZCosKgy1DcGcuQL7fI7hsXUGzB3C15KbGrtNFhz0JY0shppvFvA4wHeIiyDYZjGJt IYvg1JGC426Ddi8h6M1WI/VAQKZMa0EhpXhv1lFfCpEK4KsqAZ7AzFNCtPMsQXDKb0EHqM+zTQ+Y FA8LfQs19CpRDO/ZFprLL2pzwyxOojQWh6FsESOpmvmdBHx2J6vESoQKSFEC6ipC1doLUWoaIEVO nRIPoNsP0N5+jIKJchiFkcQ7zIIDNBaGOvtcjlJmN1rhZIyPORCizWKNiiPIxwRNIkB0KBeDQ2Ay BrjqOl1DqsNUKbznWSXnOb4X3EjQKjFaSQHeLYfBSsChQW/CCaFbtRfaEabSgcqoe5qakvEqshTW 4+4TzR9KHqHYvi228U2NnjaLRo1rQ0JsuJAPQUD8iuwoQMINFsTFIYg6gwZAQNDE8P5HhqmldQgX gLNfgYh9h1F0WgoY9hfEDumKYSmzkaI0IJTgi4IYewMheGHtrqOogogGNfmp8MsLDkdszWJaFpQD 7G0gYiy4XBagsBv+AxHmLRpuYMsKMva1DUGaxXuOweuYRMgWID375DC3EUuRuAgggAcDGO7ocD4j p95BagHaHSWgPNYlMFI711ILR14ViqiiY6C4kqlxPP87hVlYZo6jWKC7WFqFIRzmI/sMFMYjKw5Q POFlMsJBM7vVIR2GfiVCpbsyDEc9QHQEF/4u5IpwoSFkqezi --===============5692676575183644303==--