3476 Jan Wedvik 2012-06-21
This is a fix for bug#14208924 . It is based on an initial patch written by
Frazer Clement.
If TC decides to abort a transaction in the 'prepared' state, it may leak
ApiConnectRecord objects that was allocated via Dbtc::seizeApiConnectCopy().
These are normally released by Dbtc::sendApiCommit() followed by
Dbtc::releaseApiConCopy(). If the transaction aborts, these methods are not
called.
This patch fixes the above issue by checking if the ApiConnectRecord object
has an apiCopyRecord when the transaction aborts. If so, the copy object is put
back in the free list.
The patch also adds a regression test case (using an error insert).
modified:
storage/ndb/src/kernel/blocks/dbtc/DbtcMain.cpp
storage/ndb/test/ndbapi/testBasic.cpp
storage/ndb/test/run-test/daily-basic-tests.txt
3475 Martin Skold 2012-06-21
Bug#12755722 61528: INNODB BACKEND CRASHES ON ALTER TABLE STATEMENT (MYSQL SERVER HAS GONE AWAY: implemented check for new field and added test case
modified:
mysql-test/suite/ndb/r/ndb_alter_table_online.result
mysql-test/suite/ndb/t/ndb_alter_table_online.test
sql/sql_table.cc
=== modified file 'storage/ndb/src/kernel/blocks/dbtc/DbtcMain.cpp'
--- a/storage/ndb/src/kernel/blocks/dbtc/DbtcMain.cpp 2012-04-24 10:55:03 +0000
+++ b/storage/ndb/src/kernel/blocks/dbtc/DbtcMain.cpp 2012-06-21 10:31:29 +0000
@@ -4429,6 +4429,7 @@ void Dbtc::seizeApiConnectCopy(Signal* s
ptrCheckGuard(locApiConnectptr, TapiConnectFilesize, localApiConnectRecord);
cfirstfreeApiConnectCopy = locApiConnectptr.p->nextApiConnect;
locApiConnectptr.p->nextApiConnect = RNIL;
+ ndbassert(regApiPtr->apiCopyRecord == RNIL);
regApiPtr->apiCopyRecord = locApiConnectptr.i;
regApiPtr->triggerPending = false;
regApiPtr->isIndexOp = false;
@@ -4470,7 +4471,9 @@ void Dbtc::execDIVERIFYCONF(Signal* sign
* WE WILL INSERT THE TRANSACTION INTO ITS PROPER QUEUE OF
* TRANSACTIONS FOR ITS GLOBAL CHECKPOINT.
*-------------------------------------------------------------------------*/
- if (TApifailureNr != Tfailure_nr) {
+ if (TApifailureNr != Tfailure_nr ||
+ ERROR_INSERTED(8094)) {
+ jam();
DIVER_node_fail_handling(signal, Tgci);
return;
}//if
@@ -4913,7 +4916,12 @@ err8055:
Ptr<ApiConnectRecord> copyPtr;
UintR TapiConnectFilesize = capiConnectFilesize;
UintR TcommitCount = c_counters.ccommitCount;
+ /**
+ * Unlink copy connect record from main connect record to allow main record
+ * re-use.
+ */
copyPtr.i = regApiPtr.p->apiCopyRecord;
+ regApiPtr.p->apiCopyRecord = RNIL;
UintR TapiFailState = regApiPtr.p->apiFailState;
ApiConnectRecord *localApiConnectRecord = apiConnectRecord;
@@ -5276,6 +5284,7 @@ void Dbtc::handleGcp(Signal* signal, Ptr
void Dbtc::releaseApiConCopy(Signal* signal)
{
ApiConnectRecord * const regApiPtr = apiConnectptr.p;
+ ndbassert(regApiPtr->nextApiConnect == RNIL);
UintR TfirstfreeApiConnectCopyOld = cfirstfreeApiConnectCopy;
cfirstfreeApiConnectCopy = apiConnectptr.i;
regApiPtr->nextApiConnect = TfirstfreeApiConnectCopyOld;
@@ -10805,6 +10814,7 @@ void Dbtc::initApiConnect(Signal* signal
apiConnectptr.p->currSavePointId = 0;
apiConnectptr.p->m_transaction_nodes.clear();
apiConnectptr.p->singleUserMode = 0;
+ apiConnectptr.p->apiCopyRecord = RNIL;
}//for
apiConnectptr.i = tiacTmp - 1;
ptrCheckGuard(apiConnectptr, capiConnectFilesize, apiConnectRecord);
@@ -10834,6 +10844,7 @@ void Dbtc::initApiConnect(Signal* signal
apiConnectptr.p->currSavePointId = 0;
apiConnectptr.p->m_transaction_nodes.clear();
apiConnectptr.p->singleUserMode = 0;
+ apiConnectptr.p->apiCopyRecord = RNIL;
}//for
apiConnectptr.i = (2 * tiacTmp) - 1;
ptrCheckGuard(apiConnectptr, capiConnectFilesize, apiConnectRecord);
@@ -10863,6 +10874,7 @@ void Dbtc::initApiConnect(Signal* signal
apiConnectptr.p->currSavePointId = 0;
apiConnectptr.p->m_transaction_nodes.clear();
apiConnectptr.p->singleUserMode = 0;
+ apiConnectptr.p->apiCopyRecord = RNIL;
}//for
apiConnectptr.i = (3 * tiacTmp) - 1;
ptrCheckGuard(apiConnectptr, capiConnectFilesize, apiConnectRecord);
@@ -11171,6 +11183,19 @@ void Dbtc::releaseAbortResources(Signal*
TcConnectRecordPtr rarTcConnectptr;
c_counters.cabortCount++;
+ if (apiConnectptr.p->apiCopyRecord != RNIL)
+ {
+ // Put apiCopyRecord back in free list.
+ jam();
+ ApiConnectRecordPtr copyPtr;
+ copyPtr.i = apiConnectptr.p->apiCopyRecord;
+ ptrCheckGuard(copyPtr, capiConnectFilesize, apiConnectRecord);
+ ndbassert(copyPtr.p->apiCopyRecord == RNIL);
+ ndbassert(copyPtr.p->nextApiConnect == RNIL);
+ copyPtr.p->nextApiConnect = cfirstfreeApiConnectCopy;
+ cfirstfreeApiConnectCopy = copyPtr.i;
+ apiConnectptr.p->apiCopyRecord = RNIL;
+ }
if (apiConnectptr.p->cachePtr != RNIL) {
cachePtr.i = apiConnectptr.p->cachePtr;
ptrCheckGuard(cachePtr, ccacheFilesize, cacheRecord);
@@ -11270,6 +11295,8 @@ void Dbtc::releaseApiCon(Signal* signal,
TlocalApiConnectptr.i = TapiConnectPtr;
ptrCheckGuard(TlocalApiConnectptr, capiConnectFilesize, apiConnectRecord);
+ ndbassert(TlocalApiConnectptr.p->apiCopyRecord == RNIL);
+ ndbassert(TlocalApiConnectptr.p->nextApiConnect == RNIL);
TlocalApiConnectptr.p->nextApiConnect = cfirstfreeApiConnect;
cfirstfreeApiConnect = TlocalApiConnectptr.i;
setApiConTimer(TlocalApiConnectptr.i, 0, __LINE__);
=== modified file 'storage/ndb/test/ndbapi/testBasic.cpp'
--- a/storage/ndb/test/ndbapi/testBasic.cpp 2011-01-30 20:42:21 +0000
+++ b/storage/ndb/test/ndbapi/testBasic.cpp 2012-06-21 10:31:29 +0000
@@ -23,6 +23,7 @@
#include <Bitmask.hpp>
#include <random.h>
#include <signaldata/DumpStateOrd.hpp>
+#include <NdbConfig.hpp>
#define GETNDB(ps) ((NDBT_NdbApiStep*)ps)->getNdb()
@@ -2094,6 +2095,63 @@ runBug59496_case2(NDBT_Context* ctx, NDB
return NDBT_OK;
}
+// An 'assert' that is always executed, so that 'cond' may have side effects.
+#ifdef NDEBUG
+#define ASSERT_ALWAYS(cond) if(!(cond)){abort();}
+#else
+#define ASSERT_ALWAYS assert
+#endif
+
+// Regression test for bug #14208924
+static int
+runLeakApiConnectObjects(NDBT_Context* ctx, NDBT_Step* step)
+{
+ NdbRestarter restarter;
+ /**
+ * This error insert inc ombination with bug #14208924 will
+ * cause TC to leak ApiConnectRecord objects.
+ */
+ restarter.insertErrorInAllNodes(8094);
+
+ Ndb* const ndb = GETNDB(step);
+ Uint32 maxTrans = 0;
+ NdbConfig conf(GETNDB(step)->getNodeId()+1);
+ ASSERT_ALWAYS(conf.getProperty(conf.getMasterNodeId(),
+ NODE_TYPE_DB,
+ CFG_DB_NO_TRANSACTIONS,
+ &maxTrans));
+ ASSERT_ALWAYS(maxTrans > 0);
+
+ HugoOperations hugoOps(*ctx->getTab());
+ // One ApiConnectRecord object is leaked for each iteration.
+ for (uint i = 0; i < maxTrans+1; i++)
+ {
+ ASSERT_ALWAYS(hugoOps.startTransaction(ndb) == 0);
+ ASSERT_ALWAYS(hugoOps.pkInsertRecord(ndb, i) == 0);
+ NdbTransaction* const trans = hugoOps.getTransaction();
+ /**
+ * The error insert causes trans->execute(Commit) to fail with error code
+ * 286 even if the bug is fixed. Therefore, we ignore this error code.
+ */
+ if (trans->execute(Commit) != 0 &&
+ trans->getNdbError().code != 286)
+ {
+ g_err << "trans->execute() gave unexpected error : "
+ << trans->getNdbError() << endl;
+ restarter.insertErrorInAllNodes(0);
+ return NDBT_FAILED;
+ }
+ ASSERT_ALWAYS(hugoOps.closeTransaction(ndb) == 0);
+ }
+ restarter.insertErrorInAllNodes(0);
+
+ UtilTransactions utilTrans(*ctx->getTab());
+ if (utilTrans.clearTable(ndb) != 0){
+ return NDBT_FAILED;
+ }
+ return NDBT_OK;
+}
+
NDBT_TESTSUITE(testBasic);
TESTCASE("PkInsert",
"Verify that we can insert and delete from this table using PK"
@@ -2419,6 +2477,10 @@ TESTCASE("Bug59496_case2", "")
STEP(runBug59496_case2);
STEPS(runBug59496_scan, 10);
}
+TESTCASE("LeakApiConnectObjects", "")
+{
+ INITIALIZER(runLeakApiConnectObjects);
+}
NDBT_TESTSUITE_END(testBasic);
#if 0
=== modified file 'storage/ndb/test/run-test/daily-basic-tests.txt'
--- a/storage/ndb/test/run-test/daily-basic-tests.txt 2012-04-24 10:55:03 +0000
+++ b/storage/ndb/test/run-test/daily-basic-tests.txt 2012-06-21 10:31:29 +0000
@@ -1537,3 +1537,7 @@ max-time:300
cmd: testScan
args: -nScanFragRecExhaust T1
+max-time: 300
+cmd: testBasic
+args: -n LeakApiConnectObjects T1
+
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-5.1-telco-6.3 branch (jan.wedvik:3475 to 3476)Bug#14208924 | Jan Wedvik | 25 Jun |
