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).
| Thread |
|---|
| • bzr push into mysql-5.1-telco-7.0 branch (magnus.blaudd:4445 to 4448)Bug#37153 | magnus.blaudd | 9 Jun |
