List:Commits« Previous MessageNext Message »
From:Jan Wedvik Date:June 26 2012 9:00am
Subject:bzr push into mysql-5.1-telco-6.3 branch (jan.wedvik:3476 to 3477)
View as plain text  
 3477 Jan Wedvik	2012-06-26
      Patch updated according to suggestions from Magnus Blåudd and Fraqzer Clement.
      
      This is a fix for bug#11748194 "TRANSACTION OBJECT CREATED AND UNRELEASED BY 
      EXTRA CALL TO NEXTRESULT()".
      
      This bug had the following effect: If an API application tried calling 
      NdbScanOperation::nextResult() once more when the previous call had retuned
      end-of-file (return code 1), the api would leak a transaction object.
      
      This commit fixes that problem. Ndb::closeTransaction() will now put the
      extra transaction object associated with the scan back in the idle list
      for the right TC node.
      
      Also, calling NdbScanOperation::nextResult() once too much will now set
      a proper error code (4210), instead of the undefined code -1.
      
      Finally, this commit adds a regression test that will trigger the bug if the
      bugfix is not applied.

    modified:
      storage/ndb/src/ndbapi/NdbScanOperation.cpp
      storage/ndb/src/ndbapi/ndberror.c
      storage/ndb/test/ndbapi/testScan.cpp
      storage/ndb/test/run-test/daily-basic-tests.txt
 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
=== modified file 'storage/ndb/src/ndbapi/NdbScanOperation.cpp'
--- a/storage/ndb/src/ndbapi/NdbScanOperation.cpp	2011-01-30 20:42:21 +0000
+++ b/storage/ndb/src/ndbapi/NdbScanOperation.cpp	2012-06-26 09:00:13 +0000
@@ -40,6 +40,8 @@
 
 #define DEBUG_NEXT_RESULT 0
 
+static const int Err_scanAlreadyComplete = 4120;
+
 NdbScanOperation::NdbScanOperation(Ndb* aNdb, NdbOperation::Type aType) :
   NdbOperation(aNdb, aType),
   m_transConnection(NULL)
@@ -1807,6 +1809,15 @@ NdbScanOperation::nextResultNdbRecord(co
 
   if(theError.code)
   {
+    if (theError.code == Err_scanAlreadyComplete)
+    {
+      /**
+       * The scan is already complete. There must be a bug in the api 
+       * application such that is calls nextResult()/nextResultNdbRecord() 
+       * again after getting return value 1 (meaning end of scan).
+       */
+      return -1;
+    }
     goto err4;
   }
 
@@ -1850,8 +1861,9 @@ NdbScanOperation::nextResultNdbRecord(co
       {
         /**
          * No completed & no sent -> EndOfData
+         * Make sure user gets error if he tries again.
          */
-        theError.code= -1; // make sure user gets error if he tries again
+        theError.code= Err_scanAlreadyComplete;
         return 1;
       }
 
@@ -3601,7 +3613,7 @@ NdbIndexScanOperation::next_result_order
   }
   else
   {
-    theError.code= -1;
+    theError.code= Err_scanAlreadyComplete;
     return 1;                                   // End-of-file
   }
 }

=== modified file 'storage/ndb/src/ndbapi/ndberror.c'
--- a/storage/ndb/src/ndbapi/ndberror.c	2011-02-01 21:05:11 +0000
+++ b/storage/ndb/src/ndbapi/ndberror.c	2012-06-26 09:00:13 +0000
@@ -535,6 +535,7 @@ ErrorBundle ErrorCodes[] = {
   { 4116, DMEC, AE, "Operation was not defined correctly, probably missing a key" },
   { 4117, DMEC, AE, "Could not start transporter, configuration error"}, 
   { 4118, DMEC, AE, "Parameter error in API call" },
+  { 4120, DMEC, AE, "Scan already complete" },
   { 4300, DMEC, AE, "Tuple Key Type not correct" },
   { 4301, DMEC, AE, "Fragment Type not correct" },
   { 4302, DMEC, AE, "Minimum Load Factor not correct" },

=== modified file 'storage/ndb/test/ndbapi/testScan.cpp'
--- a/storage/ndb/test/ndbapi/testScan.cpp	2012-04-24 10:55:03 +0000
+++ b/storage/ndb/test/ndbapi/testScan.cpp	2012-06-26 09:00:13 +0000
@@ -24,6 +24,7 @@
 #include "ScanFunctions.hpp"
 #include <random.h>
 #include <signaldata/DumpStateOrd.hpp>
+#include <NdbConfig.hpp>
 
 const NdbDictionary::Table *
 getTable(Ndb* pNdb, int i){
@@ -1531,6 +1532,111 @@ runCloseRefresh(NDBT_Context* ctx, NDBT_
   return NDBT_OK;
 }
 
+// An 'assert' that is always executed, so that 'cond' may have side effects.
+#define require(cond) \
+  do \
+  { \
+    if (!(cond)) \
+    { \
+      g_err << "require(" << #cond << ") failed at " << __FILE__ << ":" \
+        << __LINE__ << endl; \
+      abort(); \
+    } \
+  }while(false)
+
+/**
+ * This is a regression test for bug #11748194 "TRANSACTION OBJECT CREATED 
+ * AND UNRELEASED BY EXTRA CALL TO NEXTRESULT()".
+ * If a transaction made an extra call to nextResult() after getting
+ * end-of-scan from nextResult(), the API would leak transaction objects. 
+ */
+static int
+runExtraNextResult(NDBT_Context* ctx, NDBT_Step* step)
+{
+  const NdbDictionary::Table * pTab = ctx->getTab();
+  // Fill table with 10 rows.
+  HugoTransactions hugoTrans(*pTab);
+  Ndb* const ndb = GETNDB(step);
+  hugoTrans.loadTable(ndb, 10);
+  // Read MaxNoOfConcurrentTransactions configuration value.
+  Uint32 maxTrans = 0;
+  NdbConfig conf(ndb->getNodeId());
+  require(conf.getProperty(conf.getMasterNodeId(),
+                           NODE_TYPE_DB,
+                           CFG_DB_NO_TRANSACTIONS,
+                           &maxTrans));
+  require(maxTrans > 0);
+  
+  /**
+   * The bug causes each scan to leak one object.
+   */
+  int result = NDBT_OK;
+  Uint32 i = 0;
+  while (i < maxTrans+1)
+  {
+    NdbTransaction* const trans = ndb->startTransaction();
+    if (trans == NULL)
+    {
+      g_err << "ndb->startTransaction() gave unexpected error : " 
+            << ndb->getNdbError() << " in the " << i << "th iteration." <<endl;
+      return NDBT_FAILED;
+    }
+    
+    // Do a random numer of scans in this transaction.
+    for (Uint32 j=0; j < random()%4; j++)
+    {
+      NdbScanOperation* const scan = trans->getNdbScanOperation(pTab);
+      if (scan == NULL)
+      {
+        g_err << "trans->getNdbScanOperation() gave unexpected error : " 
+              << trans->getNdbError() << " in the " << i
+              << "th iteration." <<endl;
+        return NDBT_FAILED;
+      }
+      
+      require(scan->readTuples(NdbOperation::LM_CommittedRead) == 0);
+      require(scan->getValue(0u) != 0);
+      require(trans->execute(NoCommit) == 0);
+      
+      // Scan table until end.
+      int scanResult;
+      do
+      {
+        // Fetch new batch.
+        scanResult = scan->nextResult(true);
+        while (scanResult == 0)
+        {
+          // Iterate over batch.
+          scanResult = scan->nextResult(false);
+        }
+      } while (scanResult == 0 || scanResult == 2);
+
+      /** 
+       * Do extra nextResult. This is the application error that triggers the 
+       * bug.
+       */
+      scanResult = scan->nextResult(true);
+      require(scanResult < 0);
+      // Here we got the undefined error code -1. So check for that too.
+      // if (trans->getNdbError().code != 4120
+      if (scan->getNdbError().code != 4120
+          && result == NDBT_OK)
+      {
+        g_err << "scan->nextResult() gave unexpected error : " 
+              << scan->getNdbError() << " in the " << i << "th iteration." 
+              << endl;
+        result = NDBT_FAILED;
+      }
+      i++;
+    }
+    ndb->closeTransaction(trans);
+  } // while (i < maxTrans+1)
+
+  // Delete table rows.
+  require(UtilTransactions(*ctx->getTab()).clearTable(ndb) == 0);
+  return result;
+}
+
 NDBT_TESTSUITE(testScan);
 TESTCASE("ScanRead", 
 	 "Verify scan requirement: It should be possible "\
@@ -2072,6 +2178,11 @@ TESTCASE("ScanFragRecExhaust",
   INITIALIZER(runScanReadExhaust);
   FINALIZER(runClearTable);
 }
+TESTCASE("extraNextResultBug11748194",
+         "Regression test for bug #11748194")
+{
+  INITIALIZER(runExtraNextResult);
+}
 NDBT_TESTSUITE_END(testScan);
 
 int main(int argc, const char** argv){

=== modified file 'storage/ndb/test/run-test/daily-basic-tests.txt'
--- a/storage/ndb/test/run-test/daily-basic-tests.txt	2012-06-21 10:31:29 +0000
+++ b/storage/ndb/test/run-test/daily-basic-tests.txt	2012-06-26 09:00:13 +0000
@@ -1541,3 +1541,7 @@ max-time: 300
 cmd: testBasic
 args: -n LeakApiConnectObjects T1
 
+max-time: 300
+cmd: testScan
+args:  -n extraNextResultBug11748194 T1
+

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.1-telco-6.3 branch (jan.wedvik:3476 to 3477) Jan Wedvik26 Jun