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 Wedvik | 26 Jun |
