From: Date: January 17 2007 10:53am Subject: bk commit into 5.1 tree (tomas:1.2389) BUG#25387 List-Archive: http://lists.mysql.com/commits/18225 X-Bug: 25387 Message-Id: <20070117095355.0FAB01006A@poseidon.mysql.com> Below is the list of changes that have just been committed into a local 5.1 repository of tomas. When tomas does a push these changes will be propagated to the main repository and, within 24 hours after the push, to the public repository. For information on how to access the public repository see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html ChangeSet 1.2389 07/01/17 10:53:42 tomas@stripped +2 -0 Bug#25387 - added comments in code suggestion during review storage/ndb/src/ndbapi/NdbEventOperationImpl.hpp 1.33 07/01/17 10:53:35 tomas@stripped +60 -2 Bug#25387 - added comments in code suggestion during review storage/ndb/src/ndbapi/NdbEventOperationImpl.cpp 1.75 07/01/17 10:53:34 tomas@stripped +28 -5 Bug#25387 - added comments in code suggestion during review # This is a BitKeeper patch. What follows are the unified diffs for the # set of deltas contained in the patch. The rest of the patch, the part # that BitKeeper cares about, is below these diffs. # User: tomas # Host: poseidon.mysql.com # Root: /home/tomas/mysql-5.1-new-ndb --- 1.74/storage/ndb/src/ndbapi/NdbEventOperationImpl.cpp 2007-01-16 19:22:02 +01:00 +++ 1.75/storage/ndb/src/ndbapi/NdbEventOperationImpl.cpp 2007-01-17 10:53:34 +01:00 @@ -559,6 +559,8 @@ m_state= EO_EXECUTING; mi_type= m_eventImpl->mi_type; m_ndb->theEventBuffer->add_op(); + // add kernel reference + // removed on TE_STOP, TE_CLUSTER_FAILURE, or error below m_ref_count++; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", m_ref_count, this)); int r= NdbDictionaryImpl::getImpl(*myDict).executeSubscribeEvent(*this); @@ -571,8 +573,8 @@ if (r != 0) { break; } - // blob event op now holds reference - // cleared by TE_STOP or TE_CLUSTER_FAILURE + // add blob reference to main op + // removed by TE_STOP or TE_CLUSTER_FAILURE m_ref_count++; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", m_ref_count, this)); blob_op = blob_op->m_next; @@ -583,7 +585,9 @@ DBUG_RETURN(0); } } - //Error + // Error + // remove kernel reference + // added above m_ref_count--; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", m_ref_count, this)); m_state= EO_ERROR; @@ -1227,6 +1231,8 @@ EventBufData_list::Gci_ops *gci_ops = m_available_data.first_gci_ops(); while (gci_ops && op->getGCI() > gci_ops->m_gci) { + // moved to next gci, check if any references have been + // released when completing the last gci deleteUsedEventOperations(); gci_ops = m_available_data.next_gci_ops(); } @@ -1254,6 +1260,8 @@ #endif // free all "per gci unique" collected operations + // completed gci, check if any references have been + // released when completing the gci EventBufData_list::Gci_ops *gci_ops = m_available_data.first_gci_ops(); while (gci_ops) { @@ -1290,6 +1298,8 @@ { NdbEventOperationImpl *op = &op_f->m_impl; DBUG_ASSERT(op->m_ref_count > 0); + // remove gci reference + // added in inserDataL op->m_ref_count--; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", op->m_ref_count, op)); if (op->m_ref_count == 0) @@ -1807,14 +1817,17 @@ { op->m_node_bit_mask.clear(); DBUG_ASSERT(op->m_ref_count > 0); + // remove kernel reference + // added in execute_nolock op->m_ref_count--; DBUG_PRINT("info", ("_TE_CLUSTER_FAILURE: m_ref_count: %u for op: %p", op->m_ref_count, op)); if (op->theMainOp) { - // blob event op, need to clear ref count in main op DBUG_ASSERT(op->m_ref_count == 0); DBUG_ASSERT(op->theMainOp->m_ref_count > 0); + // remove blob reference in main op + // added in execute_no_lock op->theMainOp->m_ref_count--; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", op->theMainOp->m_ref_count, op->theMainOp)); @@ -1826,14 +1839,17 @@ if (op->m_node_bit_mask.isclear()) { DBUG_ASSERT(op->m_ref_count > 0); + // remove kernel reference + // added in execute_no_lock op->m_ref_count--; DBUG_PRINT("info", ("_TE_STOP: m_ref_count: %u for op: %p", op->m_ref_count, op)); if (op->theMainOp) { - // blob event op, need to clear ref count in main op DBUG_ASSERT(op->m_ref_count == 0); DBUG_ASSERT(op->theMainOp->m_ref_count > 0); + // remove blob reference in main op + // added in execute_no_lock op->theMainOp->m_ref_count--; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", op->theMainOp->m_ref_count, op->theMainOp)); @@ -2586,6 +2602,8 @@ #ifndef DBUG_OFF i = m_gci_op_count; #endif + // add gci reference + // removed in deleteUsedOperations g.op->m_ref_count++; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", g.op->m_ref_count, g.op)); m_gci_op_list[m_gci_op_count++] = g; @@ -2654,6 +2672,8 @@ delete tOp; DBUG_RETURN(NULL); } + // add user reference + // removed in dropEventOperation getEventOperationImpl(tOp)->m_ref_count = 1; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", getEventOperationImpl(tOp)->m_ref_count, getEventOperationImpl(tOp))); @@ -2706,6 +2726,9 @@ } DBUG_ASSERT(op->m_ref_count > 0); + // remove user reference + // added in createEventOperation + // user error to use reference after this op->m_ref_count--; DBUG_PRINT("info", ("m_ref_count: %u for op: %p", op->m_ref_count, op)); if (op->m_ref_count == 0) --- 1.32/storage/ndb/src/ndbapi/NdbEventOperationImpl.hpp 2007-01-16 19:22:02 +01:00 +++ 1.33/storage/ndb/src/ndbapi/NdbEventOperationImpl.hpp 2007-01-17 10:53:35 +01:00 @@ -400,7 +400,59 @@ Uint32 m_eventId; Uint32 m_oid; + /* + m_node_bit_mask keeps track of which ndb nodes have reference to + an event op + + - add - TE_ACTIVE + - remove - TE_STOP, TE_NODE_FAILURE, TE_CLUSTER_FAILURE + + TE_NODE_FAILURE and TE_CLUSTER_FAILURE are created as events + and added to all event ops listed as active or pending delete + in m_dropped_ev_op using insertDataL, includeing the blob + event ops referenced by a regular event op. + - NdbEventBuffer::report_node_failure + - NdbEventBuffer::completeClusterFailed + + TE_ACTIVE is sent from the kernel on initial execute/start of the + event op, but is also internally generetad on node connect like + TE_NODE_FAILURE and TE_CLUSTER_FAILURE + - NdbEventBuffer::report_node_connected + + when m_node_bit_mask becomes clear, the kernel reference is + removed from m_ref_count + */ + Bitmask<(unsigned int)_NDB_NODE_BITMASK_SIZE> m_node_bit_mask; + + /* + m_ref_count keeps track of outstanding references to an event + operation impl object. To make sure that the object is not + deleted too early. + + If on dropEventOperation there are still references to an + object it is queued for delete in NdbEventBuffer::m_dropped_ev_op + + the following references exists for a _non_ blob event op: + * user reference + - add - NdbEventBuffer::createEventOperation + - remove - NdbEventBuffer::dropEventOperation + * kernel reference + - add - execute_nolock + - remove - TE_STOP, TE_CLUSTER_FAILURE + * blob reference + - add - execute_nolock on blob event + - remove - TE_STOP, TE_CLUSTER_FAILURE on blob event + * gci reference + - add - insertDataL/add_gci_op + - remove - NdbEventBuffer::deleteUsedEventOperations + + the following references exists for a blob event op: + * kernel reference + - add - execute_nolock + - remove - TE_STOP, TE_CLUSTER_FAILURE + */ + int m_ref_count; bool m_mergeEvents; @@ -557,8 +609,14 @@ Vector m_allocated_data; unsigned m_sz; - // dropped event operations that have not yet - // been deleted + /* + dropped event operations (dropEventOperation) that have not yet + been deleted because of outstanding m_ref_count + + check for delete is done on occations when the ref_count may have + changed by calling deleteUsedEventOperations: + - nextEvent - each time the user has completed processing a gci + */ NdbEventOperationImpl *m_dropped_ev_op; Uint32 m_active_op_count;