From: Jan Wedvik Date: June 21 2012 10:32am Subject: bzr push into mysql-5.1-telco-6.3 branch (jan.wedvik:3475 to 3476) Bug#14208924 List-Archive: http://lists.mysql.com/commits/144291 X-Bug: 14208924 Message-Id: <20120621103203.29162.62639.3476@atum17.no.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 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 #include #include +#include #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).