bzr commit into mysql-5.1-telco-7.0 branch (ole.john.aske:4468)

[Ole John Aske]

#At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1-telco-7.0/ based on revid:jonas@stripped

 4468 Ole John Aske	2011-06-20
      SPJ API change: Relax lifetime dependencies between 'NdbQueryDef' and its instantiated NdbQuery objects.
      
      The application is now allowed to destruct the NdbQueryDef after all its instantiated 
      NdbQuery objects has been '::close()'ed.

    modified:
      storage/ndb/src/ndbapi/NdbQueryBuilder.hpp
      storage/ndb/src/ndbapi/NdbQueryOperation.cpp
      storage/ndb/src/ndbapi/NdbQueryOperation.hpp
      storage/ndb/src/ndbapi/NdbQueryOperationImpl.hpp
=== modified file 'storage/ndb/src/ndbapi/NdbQueryBuilder.hpp'
--- a/storage/ndb/src/ndbapi/NdbQueryBuilder.hpp	2011-06-16 09:32:43 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryBuilder.hpp	2011-06-20 13:25:48 +0000
@@ -479,13 +479,8 @@ private:
  * times. It is valid until it is explicitely released().
  *
  * The NdbQueryDef *must* be keept alive until the last thread
- * which executing a query based on this NdbQueryDef has completed execution 
- * *and* result handling. Used from multiple threads this implies either:
- *
- *  - Keep the NdbQueryDef until all threads terminates.
- *  - Implement reference counting on the NdbQueryDef.
- *  - Use the supplied copy constructor to give each thread its own copy
- *    of the NdbQueryDef.
+ * which executing a query based on this NdbQueryDef has called
+ * NdbQuery::close().
  *
  * A NdbQueryDef is scheduled for execution by appending it to an open 
  * transaction - optionally together with a set of parameters specifying 

=== modified file 'storage/ndb/src/ndbapi/NdbQueryOperation.cpp'
--- a/storage/ndb/src/ndbapi/NdbQueryOperation.cpp	2011-06-16 09:32:43 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryOperation.cpp	2011-06-20 13:25:48 +0000
@@ -1389,7 +1389,7 @@ NdbQueryImpl::NdbQueryImpl(NdbTransactio
   m_state(Initial),
   m_tcState(Inactive),
   m_next(NULL),
-  m_queryDef(queryDef),
+  m_queryDef(&queryDef),
   m_error(),
   m_transaction(trans),
   m_scanTransaction(NULL),
@@ -1452,9 +1452,13 @@ NdbQueryImpl::NdbQueryImpl(NdbTransactio
 
 NdbQueryImpl::~NdbQueryImpl()
 {
- 
-  // Do this to check that m_queryDef still exists.
-  assert(getNoOfOperations() == m_queryDef.getNoOfOperations());
+  /** BEWARE:
+   *  Don't refer NdbQueryDef or NdbQueryOperationDefs after 
+   *  NdbQuery::close() as at this stage the appliaction is 
+   *  allowed to destruct the Def's.
+   */
+  assert(m_state==Closed);
+  assert(m_rootFrags==NULL);
 
   // NOTE: m_operations[] was allocated as a single memory chunk with
   // placement new construction of each operation.
@@ -1465,8 +1469,6 @@ NdbQueryImpl::~NdbQueryImpl()
     }
     m_operations = NULL;
   }
-  delete[] m_rootFrags;
-  m_rootFrags = NULL;
   m_state = Destructed;
 }
 
@@ -1478,6 +1480,9 @@ NdbQueryImpl::postFetchRelease()
     { m_operations[i].postFetchRelease();
     }
   }
+  delete[] m_rootFrags;
+  m_rootFrags = NULL;
+
   m_rowBufferAlloc.reset();
   m_tupleSetAlloc.reset();
   m_resultStreamAlloc.reset();
@@ -1957,7 +1962,7 @@ NdbQueryImpl::awaitMoreResults(bool forc
   assert(m_applFrags.getCurrent() == NULL);
 
   /* Check if there are any more completed fragments available.*/
-  if (m_queryDef.isScanQuery())
+  if (getQueryDef().isScanQuery())
   {
     assert (m_scanTransaction);
     assert (m_state==Executing);
@@ -2052,7 +2057,7 @@ NdbQueryImpl::awaitMoreResults(bool forc
     assert(m_pendingFrags == 0);
     assert(m_finalBatchFrags == getRootFragCount());
     return FetchResult_noMoreData;
-  } // if(m_queryDef.isScanQuery())
+  } // if(getQueryDef().isScanQuery())
 
 } //NdbQueryImpl::awaitMoreResults
 
@@ -2131,32 +2136,41 @@ NdbQueryImpl::close(bool forceSend)
   int res = 0;
 
   assert (m_state >= Initial && m_state < Destructed);
-  Ndb* const ndb = m_transaction.getNdb();
-
-  if (m_tcState != Inactive)
+  if (m_state != Closed)
   {
-    /* We have started a scan, but we have not yet received the last batch
-     * for all root fragments. We must therefore close the scan to release 
-     * the scan context at TC.*/
-    res = closeTcCursor(forceSend);
-  }
+    if (m_tcState != Inactive)
+    {
+      /* We have started a scan, but we have not yet received the last batch
+       * for all root fragments. We must therefore close the scan to release 
+       * the scan context at TC.*/
+      res = closeTcCursor(forceSend);
+    }
 
-  // Throw any pending results
-  m_fullFrags.clear();
-  m_applFrags.clear();
+    // Throw any pending results
+    m_fullFrags.clear();
+    m_applFrags.clear();
 
-  if (m_scanTransaction != NULL)
-  {
-    assert (m_state != Closed);
-    assert (m_scanTransaction->m_scanningQuery == this);
-    m_scanTransaction->m_scanningQuery = NULL;
-    ndb->closeTransaction(m_scanTransaction);
-    ndb->theRemainingStartTransactions--;  // Compensate; m_scanTransaction was not a real Txn
-    m_scanTransaction = NULL;
+    Ndb* const ndb = m_transaction.getNdb();
+    if (m_scanTransaction != NULL)
+    {
+      assert (m_state != Closed);
+      assert (m_scanTransaction->m_scanningQuery == this);
+      m_scanTransaction->m_scanningQuery = NULL;
+      ndb->closeTransaction(m_scanTransaction);
+      ndb->theRemainingStartTransactions--;  // Compensate; m_scanTransaction was not a real Txn
+      m_scanTransaction = NULL;
+    }
+
+    postFetchRelease();
+    m_state = Closed;  // Even if it was previously 'Failed' it is closed now!
   }
 
-  postFetchRelease();
-  m_state = Closed;  // Even if it was previously 'Failed' it is closed now!
+  /** BEWARE:
+   *  Don't refer NdbQueryDef or its NdbQueryOperationDefs after ::close()
+   *  as the application is allowed to destruct the Def's after this point.
+   */
+  m_queryDef= NULL;
+
   return res;
 } //NdbQueryImpl::close
 
@@ -2819,7 +2833,7 @@ NdbQueryImpl::sendFetchMore(NdbRootFragm
 {
   assert(getRoot().m_resultStreams!=NULL);
   assert(!emptyFrag.finalBatchReceived());
-  assert(m_queryDef.isScanQuery());
+  assert(getQueryDef().isScanQuery());
 
   const Uint32 fragNo = emptyFrag.getFragNo();
   emptyFrag.reset();
@@ -2892,7 +2906,7 @@ NdbQueryImpl::sendFetchMore(NdbRootFragm
 int
 NdbQueryImpl::closeTcCursor(bool forceSend)
 {
-  assert (m_queryDef.isScanQuery());
+  assert (getQueryDef().isScanQuery());
 
   NdbImpl* const ndb = m_transaction.getNdb()->theImpl;
   const Uint32 timeout  = ndb->get_waitfor_timeout();

=== modified file 'storage/ndb/src/ndbapi/NdbQueryOperation.hpp'
--- a/storage/ndb/src/ndbapi/NdbQueryOperation.hpp	2011-04-06 14:16:13 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryOperation.hpp	2011-06-20 13:25:48 +0000
@@ -180,7 +180,15 @@ public:
   NdbTransaction* getNdbTransaction() const;
 
   /**
-   * Close query
+   * Close query.
+   *
+   * Will release most of the internally allocated objects owned 
+   * by this NdbQuery and detach itself from the NdbQueryDef
+   * used to instantiate it.
+   *
+   * The application may destruct the NdbQueryDef after 
+   * ::close() has been called on *all* NdbQuery objects
+   * instantiated from it.
    */
   void close(bool forceSend = false);
 

=== modified file 'storage/ndb/src/ndbapi/NdbQueryOperationImpl.hpp'
--- a/storage/ndb/src/ndbapi/NdbQueryOperationImpl.hpp	2011-04-06 14:16:13 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryOperationImpl.hpp	2011-06-20 13:25:48 +0000
@@ -142,7 +142,8 @@ public:
   /** Close query: 
    *  - Release datanode resources,
    *  - Discard pending result sets,
-   *  - optionaly dealloc NdbQuery structures
+   *  - Delete internal buffer and structures for receiving results.
+   *  - Disconnect with NdbQueryDef - it might now be destructed .
    */
   int close(bool forceSend);
 
@@ -191,7 +192,10 @@ public:
 
   /** Get the (transaction independent) definition of this query. */
   const NdbQueryDefImpl& getQueryDef() const
-  { return m_queryDef; }
+  {
+    assert(m_queryDef);
+    return *m_queryDef;
+  }
 
   /** Process TCKEYCONF message. Return true if query is complete. */
   bool execTCKEYCONF();
@@ -213,7 +217,7 @@ public:
    */
   void setStartIndicator()
   { 
-    assert(!m_queryDef.isScanQuery());
+    assert(!getQueryDef().isScanQuery());
     m_startIndicator = true; 
   }
 
@@ -224,7 +228,7 @@ public:
    */
   void setCommitIndicator()
   {  
-    assert(!m_queryDef.isScanQuery());
+    assert(!getQueryDef().isScanQuery());
     m_commitIndicator = true; 
   }
 
@@ -415,7 +419,7 @@ private:
   /** Next query in same transaction.*/
   NdbQueryImpl* m_next;
   /** Definition of this query.*/
-  const NdbQueryDefImpl& m_queryDef;
+  const NdbQueryDefImpl* m_queryDef;
 
   /** Possible error status of this query.*/
   NdbError m_error;


Attachment: [text/bzr-bundle] bzr/[e-mail]