List:Commits« Previous MessageNext Message »
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
View as plain text  
 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#14208924Jan Wedvik25 Jun