Bug#36495761 PB2: ndb_restore_schema_tolerance fail sporadically creating table

2/2

Patch for 7.6 Only

Context:
MTR test ndb_restore_schema_tolerance fail sporadically
due to an error in DBUTIL. Util has a pool of prepared
operations that stores the Prepared Operations until they
are executed.
The pool has a fixed size of 6 where 4 are 'reserved' for
some operations on special tables, so, in fact, there
are only 2 available spaces in preparedOperationPoll.
When UTIL handles many preperareOperation simultaneous
the pool can get exhausted because it fails seize a free
object in the pool to store the new prepared op.

In a similar way, runningPrepares pool used by UTIL
to store the running prepare operations, can also be
exhausted. The pool size is 1 but there is nothing
preventing TRIX/DICT to send many UTIL_PREPARE_REQ
in parallel to UTIL. In that scenario UTIL will fail to
seize a free object for the new running prepare operation.

Problem:
Test fail to create table because the prepared operation triggered
by trix to build the index stats of the new table finds one the
util pool exhausted.

Solution:
UTIL changed to distinguish between internal Prepared Operations
(from TRIX) and Prepared Operation for events (from DICT).
In both pools  a special space for internal prepare
operations is reserved, remaining space is for prepOps from DICT
as before. This way we ensure that, in a scenario where UTIL
is force to handle many concurrent operation the internal ones
are priority and there will be always a free space
in both pools for them.

Change-Id: I3625e08cc29f341be6d77b5a60885af6e5f601cc

Author: vinc13e

storage/ndb/include/kernel/signaldata/UtilPrepare.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2024, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -55,6 +55,7 @@ class UtilPrepareReq {
    */
   friend class DbUtil;
   friend class Trix;
+  friend class Dbdict;
 
   /**
    * For printing
@@ -71,7 +72,7 @@ class UtilPrepareReq {
     Insert             = 2,
     Delete             = 3,
     Write	       = 4,
-    Probe              = 5  // check existance...
+    Probe              = 5  // check existence...
   };
 
   enum KeyValue {
@@ -85,8 +86,10 @@ class UtilPrepareReq {
     ReorgInd           = 8
   };
 
+  enum Flags { InternalOperation = 1 };
+
   // Signal constants
-  STATIC_CONST( SignalLength = 3 );
+  STATIC_CONST( SignalLength = 4 );
   STATIC_CONST( PROPERTIES_SECTION = 0 );
   STATIC_CONST( NoOfSections = 1 );
 
@@ -96,6 +99,7 @@ class UtilPrepareReq {
   Uint32 senderData; // MUST be no 1!
   Uint32 senderRef;
   Uint32 schemaTransId;
+  Uint32 flags;
 };
 
 /**

storage/ndb/src/kernel/blocks/dbdict/Dbdict.cpp

@@ -17894,6 +17894,7 @@ Dbdict::prepareUtilTransaction(Callback *pcallback,
 
   utilPrepareReq->setSenderRef(reference());
   utilPrepareReq->setSenderData(senderData);
+  utilPrepareReq->flags = 0;
 
   const Uint32 pageSizeInWords = 128;
   Uint32 propPage[pageSizeInWords];

storage/ndb/src/kernel/blocks/dbutil/DbUtil.cpp

@@ -143,6 +143,10 @@ void
 DbUtil::releasePrepare(PreparePtr prepPtr) {
   prepPtr.p->preparePages.release();
   c_runningPrepares.release(prepPtr);  // Automatic release in pool
+  if (prepPtr.p->internalFlag) {
+    c_freeInternalRunningPrepares++;
+    prepPtr.p->internalFlag = false;
+  }
 }
 
 void 
@@ -152,6 +156,10 @@ DbUtil::releasePreparedOperation(PreparedOperationPtr prepOpPtr) {
   prepOpPtr.p->rsInfo.release();
   prepOpPtr.p->pkBitmask.clear();
   c_preparedOperationPool.release(prepOpPtr);  // No list holding these structs
+  if (prepOpPtr.p->internalFlag) {
+    c_freeInternalPreparedOps++;
+    prepOpPtr.p->internalFlag = false;
+  }
 }
 
 void
@@ -192,6 +200,27 @@ DbUtil::execREAD_CONFIG_REQ(Signal* signal)
   ndbrequire(p != 0);
 
   {
+    /**
+     * UTIL_PREPARE_REQ can be send by client blocks
+     * to create a PreparedOperation template in DBUTIL
+     * which can later be used by one or more
+     * UTIL_EXECUTE_REQ signals to define and execute
+     * one or more operations in a transaction, one
+     * or more times before releasing the prepared
+     * operations with UTIL_RELEASE_REQ
+     *
+     * UTIL_PREPARE_REQ allocates a Prepare(Request) record
+     * for the duration of the PREPARE_REQ.
+     * UTIL_PREPARE_REQ allocates a PreparedOperation record
+     * for the duration of the PreparedOperation existing
+     * (until UTIL_RELEASE_REQ)
+     *
+     * UTIL_PREPARE_REQ requests can be marked as
+     * internal, in which case they can use a reserved
+     * pool of Prepare and PreparedOperation records to
+     * avoid concurrent non-internal requests starving
+     * them.
+     */
     /* ** Dimensioning inputs : */
     Uint32 maxUIBuildBatchSize = 64;
     ndb_mgm_get_int_parameter(p, CFG_DB_UI_BUILD_MAX_BATCHSIZE,
@@ -207,8 +236,20 @@ DbUtil::execREAD_CONFIG_REQ(Signal* signal)
 
     /* Based on existing setting, probably excessive */
     const Uint32 MaxNonSchemaBuildOps = 48;
-    const Uint32 MaxPreparedOps = 6;  //  three hardcoded, one for setval, two for test
-    const Uint32 NumConcurrentPrepares = 1;  /* One parallel prepare */
+
+    //  4 hardcoded and defined at startup, 1 internal 1 for api events
+    const Uint32 HardcodedOps = 4;           /* SEQUENCE Service */
+    const Uint32 MaxInternalPreparedOps = 1; /* TRIX ops - serialised */
+    const Uint32 MaxExternalPreparedOps = 1; /* DICT Event ops */
+    const Uint32 MaxPreparedOps =
+        HardcodedOps + MaxInternalPreparedOps + MaxExternalPreparedOps;
+
+    const Uint32 MaxInternalConcurrentPrepares =
+        MaxInternalPreparedOps; /* TRIX ops */
+    const Uint32 MaxExternalConcurrentPrepares =
+        MaxExternalPreparedOps; /* DICT Event ops */
+    const Uint32 NumConcurrentPrepares =
+        MaxInternalConcurrentPrepares + MaxExternalConcurrentPrepares;
 
     const Uint32 SparePages = 5;             /* Arbitrary */    
     const Uint32 PagesPerPreparingOp = 5;    /* Arbitrary */
@@ -281,8 +322,11 @@ DbUtil::execREAD_CONFIG_REQ(Signal* signal)
       ndbout_c("  DataBuffWordsPerOp : %u", DataBuffWordsPerOp);
       ndbout_c("  numDataBuffers : %u", numDataBuffers);
       ndbout_c("  numPages : %u", numPages);
+      ndbout_c("  MaxInternalPreparedOps : %u", MaxInternalPreparedOps);
+      ndbout_c("  MaxInternalConcurrentPrepares : %u",
+               MaxInternalConcurrentPrepares);
     }
-      
+
 
     /* ** Settings */
 
@@ -293,6 +337,9 @@ DbUtil::execREAD_CONFIG_REQ(Signal* signal)
     c_transactionPool.setSize(MaxConcurrentTrans);
     c_attrMappingPool.setSize(MaxAttributeMappings);
     c_dataBufPool.setSize(numDataBuffers);
+
+    c_freeInternalPreparedOps = MaxInternalPreparedOps;
+    c_freeInternalRunningPrepares = MaxInternalConcurrentPrepares;
   }
 
   {
@@ -1106,6 +1153,7 @@ DbUtil::execUTIL_PREPARE_REQ(Signal* signal)
   const Uint32 senderRef    = req->senderRef;
   const Uint32 senderData   = req->senderData;
   const Uint32 schemaTransId= req->schemaTransId;
+  const Uint32 flags = req->flags;
 
   if(signal->getNoOfSections() == 0) {
     // Missing prepare data
@@ -1131,8 +1179,16 @@ DbUtil::execUTIL_PREPARE_REQ(Signal* signal)
 		       senderRef, senderData);
     return;
   }
-  if (!c_runningPrepares.seizeFirst(prepPtr))
-  {
+
+  const bool internalPrepOp = flags & UtilPrepareReq::InternalOperation;
+  ndbassert(c_runningPrepares.getPool().getNoOfFree() >=
+            c_freeInternalRunningPrepares);
+  const Uint32 freePrepares =
+      (internalPrepOp ? c_freeInternalRunningPrepares
+                      : c_runningPrepares.getPool().getNoOfFree() -
+                            c_freeInternalRunningPrepares);
+
+  if (freePrepares == 0) {
     jam();
     releaseSections(handle);
     sendUtilPrepareRef(signal, UtilPrepareRef::PREPARE_SEIZE_ERROR,
@@ -1142,8 +1198,17 @@ DbUtil::execUTIL_PREPARE_REQ(Signal* signal)
       ndbrequire(false);
     }
     return;
-  };
-  handle.getSection(ptr, UtilPrepareReq::PROPERTIES_SECTION);
+  }
+
+  jam();
+  ndbrequire(c_runningPrepares.seizeFirst(prepPtr));
+  if (internalPrepOp) {
+    jam();
+    ndbrequire(c_freeInternalRunningPrepares > 0);
+    c_freeInternalRunningPrepares--;
+    prepPtr.p->internalFlag = true;
+  }
+  ndbrequire(handle.getSection(ptr, UtilPrepareReq::PROPERTIES_SECTION));
   const Uint32 noPages  = (ptr.sz + sizeof(Page32)) / sizeof(Page32);
   ndbassert(noPages > 0);
   if (!prepPtr.p->preparePages.seize(noPages)) {
@@ -1354,7 +1419,13 @@ DbUtil::prepareOperation(Signal* signal,
    * Seize and store PreparedOperation struct
    *******************************************/
   PreparedOperationPtr prepOpPtr;  
-  if(!c_preparedOperationPool.seize(prepOpPtr)) {
+  const bool internalPrepOp = prepPtr.p->internalFlag;
+  ndbassert(c_preparedOperationPool.getNoOfFree() >= c_freeInternalPreparedOps);
+  const Uint32 freePrepares =
+      (internalPrepOp
+           ? c_freeInternalPreparedOps
+           : c_preparedOperationPool.getNoOfFree() - c_freeInternalPreparedOps);
+  if (freePrepares == 0) {
     jam();
     sendUtilPrepareRef(signal, UtilPrepareRef::PREPARED_OPERATION_SEIZE_ERROR,
 		       prepPtr.p->clientRef, prepPtr.p->clientData);
@@ -1365,7 +1436,15 @@ DbUtil::prepareOperation(Signal* signal,
     }
     return;
   }
+  ndbrequire(c_preparedOperationPool.seize(prepOpPtr));
+  if (internalPrepOp) {
+    jam();
+    ndbrequire(c_freeInternalPreparedOps > 0);
+    c_freeInternalPreparedOps--;
+    prepOpPtr.p->internalFlag = true;
+  }
   prepPtr.p->prepOpPtr = prepOpPtr;
+  ndbrequire(prepPtr.p->internalFlag == prepOpPtr.p->internalFlag);
 
   /********************
    * Read request info

storage/ndb/src/kernel/blocks/dbutil/DbUtil.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2024, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -178,13 +178,15 @@ class DbUtil : public SimulatedBlock
    * information.
    */
   struct Prepare {
-    Prepare(Page32_pool & ap) : preparePages(ap) {}
+    Prepare(Page32_pool &ap) : internalFlag(false), preparePages(ap) {}
 
     /*** Client info ***/
     Uint32 clientRef;
     Uint32 clientData;
     Uint32 schemaTransId;
 
+    bool internalFlag;  // flag if operation is internal
+
     /** 
      * SimpleProp sent in UTIL_PREPARE_REQ 
      *
@@ -227,9 +229,12 @@ class DbUtil : public SimulatedBlock
   struct PreparedOperation {
     PreparedOperation(AttrMappingBuffer::DataBufferPool & am,
 		      AttrInfoBuffer::DataBufferPool & ai,
-		      ResultSetInfoBuffer::DataBufferPool & rs) :
-      releaseFlag(false), attrMapping(am), attrInfo(ai), rsInfo(rs)
-    {
+                      ResultSetInfoBuffer::DataBufferPool &rs)
+        : releaseFlag(false),
+          internalFlag(false),
+          attrMapping(am),
+          attrInfo(ai),
+          rsInfo(rs) {
       pkBitmask.clear();
     }
 
@@ -240,6 +245,7 @@ class DbUtil : public SimulatedBlock
     bool      releaseFlag;     // flag if operation release after completion
     UtilPrepareReq::OperationTypeValue operationType;
 
+    bool internalFlag;  // flag if operation is internal
     /**
      * Attribute Mapping
      *
@@ -419,8 +425,13 @@ class DbUtil : public SimulatedBlock
   PreparedOperation_pool    c_preparedOperationPool;
   Transaction_pool          c_transactionPool;
 
+  Uint32 c_freeInternalPreparedOps;
+
+  Uint32 c_freeInternalRunningPrepares;
+
   DataBuffer<1,ArrayPool<DataBufferSegment<1> > >::DataBufferPool   c_attrMappingPool;
   DataBuffer<11,ArrayPool<DataBufferSegment<11> > >::DataBufferPool  c_dataBufPool;
+
   Prepare_dllist                  c_runningPrepares;
   Transaction_dllist              c_seizingTransactions; // Being seized at TC
   Transaction_dllist              c_runningTransactions; // Seized and now exec.

storage/ndb/src/kernel/blocks/trix/Trix.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2022, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2024, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -1144,6 +1144,7 @@ void Trix::prepareInsertTransactions(Signal* signal,
   utilPrepareReq->senderRef = reference();
   utilPrepareReq->senderData = subRecPtr.i;
   utilPrepareReq->schemaTransId = subRec->schemaTransId;
+  utilPrepareReq->flags = UtilPrepareReq::InternalOperation;
 
   const Uint32 pageSizeInWords = 128;
   Uint32 propPage[pageSizeInWords];
@@ -1648,6 +1649,7 @@ Trix::execCOPY_DATA_IMPL_REQ(Signal* signal)
     utilPrepareReq->senderRef = reference();
     utilPrepareReq->senderData = subRecPtr.i;
     utilPrepareReq->schemaTransId = subRec->schemaTransId;
+    utilPrepareReq->flags = UtilPrepareReq::InternalOperation;
 
     const Uint32 pageSizeInWords = 128;
     Uint32 propPage[pageSizeInWords];
@@ -1770,6 +1772,7 @@ Trix::execBUILD_FK_IMPL_REQ(Signal* signal)
     utilPrepareReq->senderRef = reference();
     utilPrepareReq->senderData = subRecPtr.i;
     utilPrepareReq->schemaTransId = subRec->schemaTransId;
+    utilPrepareReq->flags = UtilPrepareReq::InternalOperation;
 
     const Uint32 pageSizeInWords = 128;
     Uint32 propPage[pageSizeInWords];
@@ -3085,6 +3088,7 @@ Trix::statSendPrepare(Signal* signal, StatOp& stat)
   utilReq->senderData = stat.m_ownPtrI;
   utilReq->senderRef = reference();
   utilReq->schemaTransId = req->transId;
+  utilReq->flags = UtilPrepareReq::InternalOperation;
 
   Uint32 wbuf[256];
   LinearWriter w(&wbuf[0], sizeof(wbuf) >> 2);