3867 Ole John Aske 2012-06-19
This patch is intended to fix possible memory leak when a failure
occurs in Dbspj::execLQHKEYREQ or Dbspj::execSCAN_FRAGREQ,
during, or after, Dbspj::build() has completed.
In order to do a proper release of any allocated SPJ resources
in these cases, we need to call ::cleanup().
That will take care of releasing any resources allocated by
the (partial?) build TreeNode structures.
Instead of only calling ::cleanup() in case of 'late failures',
I have enhanced the consistency of the TreeNode and Request
structures such that ::cleanup() can be called in any state
on a partial build Request struct - which seems simpler and cleaner.
This also involved moving the assignment of 'm_send.m_keyInfoPtrI'
from within :build(), to after ::build() had succeeded. That avoided
a possible memory leak due to ownership of that memory section being
in an undefined limbo state.
Also introduce ERROR_INSERT(17013) which simulate such a
'late failure' after ::build()
modified:
storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp
storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp
storage/ndb/test/ndbapi/testSpj.cpp
3866 Ole John Aske 2012-06-07
Change classification of SPJ 'out of memory/resources' error from
'permanent' to 'temporary' resource errors.
Also change error text for SOJ-error 20006 to contain
'LongMessageBuffer' as this is the config variable which has
to be adjusted to avoid this error.
modified:
storage/ndb/src/ndbapi/ndberror.c
=== modified file 'storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp'
--- a/storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp 2012-05-24 11:43:11 +0000
+++ b/storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp 2012-06-19 11:04:55 +0000
@@ -298,7 +298,6 @@ public:
Uint32 m_senderRef; // TC (used for routing)
Uint32 m_scan_cnt;
Signal* m_start_signal; // Argument to first node in tree
- SegmentedSectionPtr m_keyPtr;
TreeNodeBitMask m_scans; // TreeNodes doing scans
=== modified file 'storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp'
--- a/storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp 2012-05-24 11:43:11 +0000
+++ b/storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp 2012-06-19 11:04:55 +0000
@@ -426,9 +426,10 @@ void Dbspj::execLQHKEYREQ(Signal* signal
* (unless StoredProcId is set, when only paramters are sent,
* but this is not yet implemented)
*/
+ SegmentedSectionPtr attrPtr;
SectionHandle handle = SectionHandle(this, signal);
- SegmentedSectionPtr ssPtr;
- handle.getSection(ssPtr, LqhKeyReq::AttrInfoSectionNum);
+ handle.getSection(attrPtr, LqhKeyReq::AttrInfoSectionNum);
+ const Uint32 keyPtrI = handle.m_ptr[LqhKeyReq::KeyInfoSectionNum].i;
Uint32 err;
Ptr<Request> requestPtr = { 0, RNIL };
@@ -457,7 +458,7 @@ void Dbspj::execLQHKEYREQ(Signal* signal
Uint32 len_cnt;
{
- SectionReader r0(ssPtr, getSectionSegmentPool());
+ SectionReader r0(attrPtr, getSectionSegmentPool());
err = DbspjErr::ZeroLengthQueryTree;
if (unlikely(!r0.getWord(&len_cnt)))
@@ -468,8 +469,8 @@ void Dbspj::execLQHKEYREQ(Signal* signal
Uint32 cnt = QueryTree::getNodeCnt(len_cnt);
{
- SectionReader treeReader(ssPtr, getSectionSegmentPool());
- SectionReader paramReader(ssPtr, getSectionSegmentPool());
+ SectionReader treeReader(attrPtr, getSectionSegmentPool());
+ SectionReader paramReader(attrPtr, getSectionSegmentPool());
paramReader.step(len); // skip over tree to parameters
Build_context ctx;
@@ -477,31 +478,39 @@ void Dbspj::execLQHKEYREQ(Signal* signal
ctx.m_savepointId = req->savePointId;
ctx.m_scanPrio = 1;
ctx.m_start_signal = signal;
- ctx.m_keyPtr.i = handle.m_ptr[LqhKeyReq::KeyInfoSectionNum].i;
ctx.m_senderRef = signal->getSendersBlockRef();
err = build(ctx, requestPtr, treeReader, paramReader);
if (unlikely(err != 0))
break;
- }
- /**
- * a query being shipped as a LQHKEYREQ may only return finite rows
- * i.e be a (multi-)lookup
- */
- ndbassert(requestPtr.p->isLookup());
- ndbassert(requestPtr.p->m_node_cnt == cnt);
- err = DbspjErr::InvalidRequest;
- if (unlikely(!requestPtr.p->isLookup() || requestPtr.p->m_node_cnt != cnt))
- break;
+ /**
+ * Root TreeNode in Request takes ownership of keyPtr
+ * section when build has completed.
+ * We are done with attrPtr which is now released.
+ */
+ Ptr<TreeNode> rootNodePtr = ctx.m_node_list[0];
+ rootNodePtr.p->m_send.m_keyInfoPtrI = keyPtrI;
+ release(attrPtr);
+ handle.clear();
+ }
/**
* Store request in list(s)/hash(es)
*/
store_lookup(requestPtr);
- release(ssPtr);
- handle.clear();
+ /**
+ * A query being shipped as a LQHKEYREQ may return at most a row
+ * per operation i.e be a (multi-)lookup
+ */
+ if (ERROR_INSERTED_CLEAR(17013) ||
+ unlikely(!requestPtr.p->isLookup() || requestPtr.p->m_node_cnt != cnt))
+ {
+ jam();
+ err = DbspjErr::InvalidRequest;
+ break;
+ }
start(signal, requestPtr);
return;
@@ -514,9 +523,9 @@ void Dbspj::execLQHKEYREQ(Signal* signal
if (!requestPtr.isNull())
{
jam();
- m_request_pool.release(requestPtr);
+ cleanup(requestPtr);
}
- releaseSections(handle);
+ releaseSections(handle); // a NOOP, if we reached 'handle.clear()' above
handle_early_lqhkey_ref(signal, req, err);
}
@@ -729,8 +738,8 @@ Dbspj::execSCAN_FRAGREQ(Signal* signal)
* if first op is lookup - contains keyinfo for lookup
*/
SectionHandle handle = SectionHandle(this, signal);
- SegmentedSectionPtr ssPtr;
- handle.getSection(ssPtr, ScanFragReq::AttrInfoSectionNum);
+ SegmentedSectionPtr attrPtr;
+ handle.getSection(attrPtr, ScanFragReq::AttrInfoSectionNum);
Uint32 err;
Ptr<Request> requestPtr = { 0, RNIL };
@@ -758,7 +767,7 @@ Dbspj::execSCAN_FRAGREQ(Signal* signal)
Uint32 len_cnt;
{
- SectionReader r0(ssPtr, getSectionSegmentPool());
+ SectionReader r0(attrPtr, getSectionSegmentPool());
err = DbspjErr::ZeroLengthQueryTree;
if (unlikely(!r0.getWord(&len_cnt)))
break;
@@ -768,8 +777,8 @@ Dbspj::execSCAN_FRAGREQ(Signal* signal)
Uint32 cnt = QueryTree::getNodeCnt(len_cnt);
{
- SectionReader treeReader(ssPtr, getSectionSegmentPool());
- SectionReader paramReader(ssPtr, getSectionSegmentPool());
+ SectionReader treeReader(attrPtr, getSectionSegmentPool());
+ SectionReader paramReader(attrPtr, getSectionSegmentPool());
paramReader.step(len); // skip over tree to parameters
Build_context ctx;
@@ -780,35 +789,38 @@ Dbspj::execSCAN_FRAGREQ(Signal* signal)
ctx.m_start_signal = signal;
ctx.m_senderRef = signal->getSendersBlockRef();
+ err = build(ctx, requestPtr, treeReader, paramReader);
+ if (unlikely(err != 0))
+ break;
+
+ /**
+ * Root TreeNode in Request takes ownership of keyPtr
+ * section when build has completed.
+ * We are done with attrPtr which is now released.
+ */
+ Ptr<TreeNode> rootNodePtr = ctx.m_node_list[0];
if (handle.m_cnt > 1)
{
jam();
- ctx.m_keyPtr.i = handle.m_ptr[ScanFragReq::KeyInfoSectionNum].i;
- }
- else
- {
- jam();
- ctx.m_keyPtr.i = RNIL;
+ const Uint32 keyPtrI = handle.m_ptr[ScanFragReq::KeyInfoSectionNum].i;
+ rootNodePtr.p->m_send.m_keyInfoPtrI = keyPtrI;
}
-
- err = build(ctx, requestPtr, treeReader, paramReader);
- if (unlikely(err != 0))
- break;
+ release(attrPtr);
+ handle.clear();
}
- ndbassert(requestPtr.p->isScan());
- ndbassert(requestPtr.p->m_node_cnt == cnt);
- err = DbspjErr::InvalidRequest;
- if (unlikely(!requestPtr.p->isScan() || requestPtr.p->m_node_cnt != cnt))
- break;
-
/**
* Store request in list(s)/hash(es)
*/
store_scan(requestPtr);
- release(ssPtr);
- handle.clear();
+ if (ERROR_INSERTED_CLEAR(17013) ||
+ unlikely(!requestPtr.p->isScan() || requestPtr.p->m_node_cnt != cnt))
+ {
+ jam();
+ err = DbspjErr::InvalidRequest;
+ break;
+ }
start(signal, requestPtr);
return;
@@ -817,9 +829,9 @@ Dbspj::execSCAN_FRAGREQ(Signal* signal)
if (!requestPtr.isNull())
{
jam();
- m_request_pool.release(requestPtr);
+ cleanup(requestPtr);
}
- releaseSections(handle);
+ releaseSections(handle); // a NOOP, if we reached 'handle.clear()' above
handle_early_scanfrag_ref(signal, req, err);
}
@@ -1919,33 +1931,12 @@ Dbspj::cleanup(Ptr<Request> requestPtr)
requestPtr.p->m_state = Request::RS_ABORTED;
return;
}
-
-#ifdef VM_TRACE
- {
- Request key;
- key.m_transId[0] = requestPtr.p->m_transId[0];
- key.m_transId[1] = requestPtr.p->m_transId[1];
- key.m_senderData = requestPtr.p->m_senderData;
- Ptr<Request> tmp;
- ndbrequire(m_scan_request_hash.find(tmp, key));
- }
-#endif
- m_scan_request_hash.remove(requestPtr);
+ m_scan_request_hash.remove(requestPtr, *requestPtr.p);
}
else
{
jam();
-#ifdef VM_TRACE
- {
- Request key;
- key.m_transId[0] = requestPtr.p->m_transId[0];
- key.m_transId[1] = requestPtr.p->m_transId[1];
- key.m_senderData = requestPtr.p->m_senderData;
- Ptr<Request> tmp;
- ndbrequire(m_lookup_request_hash.find(tmp, key));
- }
-#endif
- m_lookup_request_hash.remove(requestPtr);
+ m_lookup_request_hash.remove(requestPtr, *requestPtr.p);
}
releaseRequestBuffers(requestPtr, false);
ArenaHead ah = requestPtr.p->m_arena;
@@ -3037,7 +3028,6 @@ Dbspj::lookup_build(Build_context& ctx,
dst->fragmentData = fragId;
dst->attrLen = attrLen; // fragdist is in here
- treeNodePtr.p->m_send.m_keyInfoPtrI = ctx.m_keyPtr.i;
treeNodePtr.p->m_bits |= TreeNode::T_ONE_SHOT;
}
return 0;
@@ -4092,6 +4082,7 @@ Dbspj::scanFrag_build(Build_context& ctx
break;
}
+ treeNodePtr.p->m_info = &g_ScanFragOpInfo;
treeNodePtr.p->m_scanfrag_data.m_scanFragHandlePtrI = RNIL;
Ptr<ScanFragHandle> scanFragHandlePtr;
if (ERROR_INSERTED_CLEAR(17004))
@@ -4115,7 +4106,6 @@ Dbspj::scanFrag_build(Build_context& ctx
treeNodePtr.p->m_scanfrag_data.m_scanFragHandlePtrI = scanFragHandlePtr.i;
requestPtr.p->m_bits |= Request::RT_SCAN;
- treeNodePtr.p->m_info = &g_ScanFragOpInfo;
treeNodePtr.p->m_bits |= TreeNode::T_ATTR_INTERPRETED;
treeNodePtr.p->m_batch_size = ctx.m_batch_size_rows;
@@ -4235,7 +4225,6 @@ Dbspj::scanFrag_build(Build_context& ctx
ndbassert(dst->transId2 == transId2);
#endif
- treeNodePtr.p->m_send.m_keyInfoPtrI = ctx.m_keyPtr.i;
treeNodePtr.p->m_bits |= TreeNode::T_ONE_SHOT;
if (rangeScanFlag)
=== modified file 'storage/ndb/test/ndbapi/testSpj.cpp'
--- a/storage/ndb/test/ndbapi/testSpj.cpp 2012-05-24 11:43:11 +0000
+++ b/storage/ndb/test/ndbapi/testSpj.cpp 2012-06-19 11:04:55 +0000
@@ -118,6 +118,7 @@ runLookupJoinError(NDBT_Context* ctx, ND
int lookupFaults[] = {
17001, 17005, 17006, 17008,
17012, // testing abort in :execDIH_SCAN_TAB_CONF
+ 17013, // Simulate DbspjErr::InvalidRequest
17020, 17021, 17022, // lookup_send() encounter dead node -> NodeFailure
17030, 17031, 17032, // LQHKEYREQ reply is LQHKEYREF('Invalid..')
17040, 17041, 17042, // lookup_parent_row -> OutOfQueryMemory
@@ -208,6 +209,7 @@ runScanJoinError(NDBT_Context* ctx, NDBT
int scanFaults[] = {
17002, 17004, 17005, 17006, 17008,
17012, // testing abort in :execDIH_SCAN_TAB_CONF
+ 17013, // Simulate DbspjErr::InvalidRequest
17020, 17021, 17022, // lookup_send() encounter dead node -> NodeFailure
17030, 17031, 17032, // LQHKEYREQ reply is LQHKEYREF('Invalid..')
17040, 17041, 17042, // lookup_parent_row -> OutOfQueryMemory
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-5.5-cluster-7.2-spj branch (ole.john.aske:3866 to 3867) | Ole John Aske | 19 Jun |
