Bug#34348706 AUTOTEST: testLimits -n DropSignalFragments T1 fails'

As the TAB_SCANREF-error reply is received asynchronously wrt other client
processing, a NdbScanOperation error, as well as a NdbTransaction error can
arrive, and be set, at any time after a NdbTransaction::execute() has
been sent.

The testLimits test case in subject execute a scan request where a failure
was expected to be returned, but the test pass required that the error did
not become visible until after a nextResult-wait. However, there were
no such guarantees in the error return protocol. In particular after
the patch for bug 33752914 'Allow receiveThread to become more active', it
became more likely that a REF-error could race back to the client before
it reached a 'nextResult' where it had to wait. (Note that it is still
not a regression caused by that receiveThread-patch)

This patch makes the behaviour more deterministic by not setting the
NdbTransaction error code when a TAB_SCANREF-error is received.
The REF-error code is only set and kept in the NdbOperation::theError
object. Subsequent calls to ::nextResult() will check 'theError.code' after
the PollGuard has been aquired, when it could be propogated to the
NdbTransaction object as well. Note that holding the PollGuard also
provide a protection against new REF-errors appearing while theError
is being checked & propogated.

Patch also fixed ::ordered_send_scan_wait_for_all() where we failed
to setErrorCode() when having the PollGuard and theError.code was set.
(Was propably not required prior to this patch as a TAB_SCANREF
used to setErrorCode() immediately)

Test case is enhanced such that the reported failure in subject
will be hit without this patch - also fixed as similar testcase
having the same issue.

Note that the documentation of our getErrorCode() methods are very
fuzzy in this area as well - that need to improve.

Change-Id: If990c42b061644485a7003664a5e31113d498dac
(cherry picked from commit c61ca293e08241c09d912be1cb4470c1c1005277)

Author: Ole John Aske

storage/ndb/src/ndbapi/NdbScanOperation.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2022, 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,
@@ -67,6 +67,16 @@ NdbScanOperation::~NdbScanOperation()
   assert(m_scan_buffer==NULL);
 }
 
+/*****************************************************************************
+ * setErrorCode(int aErrorCode)
+ *
+ * Use setErrorCode() with errors so that they are mapped to an operation,
+ * and propogated to the 'transConnection' as well, unless they are
+ * encountered in the asynchronous signal handling part of the scan
+ * processing code, in which case the error should be set on
+ * 'theError' member variable only.
+ *
+ *****************************************************************************/
 void
 NdbScanOperation::setErrorCode(int aErrorCode) const
 {
@@ -1891,12 +1901,10 @@ NdbScanOperation::nextResultNdbRecord(const char * & out_row,
      * after getting return value 1 (meaning end of scan) or
      * -1 (for error).
      *
-     * Or there seems to be a bug in ndbapi that put operation
-     * in error between calls.
-     *
-     * Or an error have been received.
+     * Or an SCAN_TABREF-error have been received into the operation
+     * (asynchronously) between calls.
      *
-     * In any case, keep and propagate error and fail.
+     * In any case, keep and propagate as NdbTransaction error and fail.
      */
     if (theError.code != Err_scanAlreadyComplete)
       setErrorCode(theError.code);
@@ -3880,7 +3888,11 @@ NdbIndexScanOperation::ordered_send_scan_wait_for_all(bool forceSend)
 
   PollGuard poll_guard(* impl);
   if(theError.code)
+  {
+    if (theError.code != Err_scanAlreadyComplete)
+      setErrorCode(theError.code);
     return -1;
+  }
 
   Uint32 seq= theNdbCon->theNodeSequence;
   Uint32 nodeId= theNdbCon->theDBnode;

storage/ndb/src/ndbapi/NdbTransactionScan.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2022, 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,
@@ -36,7 +36,16 @@
  * int  receiveSCAN_TABREF(NdbApiSignal* aSignal)
  *
  *  This means the scan could not be started, set status(s) to indicate 
- *  the failure
+ *  the failure. Note that scan requests are asynchronous, i.e. we do not
+ *  wait for the CONF or REF to be returned. Which also imples that a REF-error
+ *  could be received into a scan operation while the client is in the midts
+ *  of doing other work, e.g. handling results from other operations in the
+ *  same transaction.
+ *
+ *  To avoid transaction errors appearing 'out of the blue', such asynch
+ *  errors are set only on the operation when they are received. Only when
+ *  processing the scan results with ::nextResult(), operational errors are
+ *  propogated to the transaction level.
  *
  ****************************************************************************/
 int			
@@ -46,7 +55,8 @@ NdbTransaction::receiveSCAN_TABREF(const NdbApiSignal* aSignal){
   if (checkState_TransId(&ref->transId1)) {
     if (theScanningOp) {
       theScanningOp->execCLOSE_SCAN_REP();
-      theScanningOp->setErrorCode(ref->errorCode);
+      // Do not ::setErrorCode() yet! - See comment in header
+      theScanningOp->theError.code = ref->errorCode;
       if(!ref->closeNeeded){
         return 0;
       }

storage/ndb/test/ndbapi/testLimits.cpp

@@ -1,4 +1,4 @@
-/* Copyright (c) 2008, 2021, Oracle and/or its affiliates.
+/* Copyright (c) 2008, 2022, 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,
@@ -931,14 +931,19 @@ int testSegmentedSectionScan(NDBT_Context* ctx, NDBT_Step* step){
 
   CHECKEQUAL(0, scan->setInterpretedCode(&prog));
 
-  /* Api doesn't seem to wait for result of scan request */
   CHECKEQUAL(0, trans->execute(NdbTransaction::NoCommit));
 
+  // Scan errors arrive asynchronously into the ScanOperation.
+  // However, errors should not become visible on the Transaction object
+  // until after the nextResult-wait.
+  CHECKEQUAL(0, trans->getNdbError().code);
+  NdbSleep_MilliSleep(10);    // Not even after a long sleep.
   CHECKEQUAL(0, trans->getNdbError().code);
 
   CHECKEQUAL(-1, scan->nextResult());
 
   CHECKEQUAL(217, scan->getNdbError().code);
+  CHECKEQUAL(217, trans->getNdbError().code);
 
   trans->close();
 
@@ -1037,15 +1042,20 @@ int testDropSignalFragments(NDBT_Context* ctx, NDBT_Step* step){
 
     CHECKEQUAL(0, scan->setInterpretedCode(&prog));
 
-    /* Api doesn't seem to wait for result of scan request */
     CHECKEQUAL(0, trans->execute(NdbTransaction::NoCommit));
-    
+
+    // Scan errors arrive asynchronously into the ScanOperation.
+    // However, they should not become visible on the Transaction object
+    // until after the nextResult-wait.
+    CHECKEQUAL(0, trans->getNdbError().code);
+    NdbSleep_MilliSleep(10);    // Not even after a long sleep.
     CHECKEQUAL(0, trans->getNdbError().code);
 
     CHECKEQUAL(-1, scan->nextResult());
 
     int expectedResult= subcase.expectedRc;
     CHECKEQUAL(expectedResult, scan->getNdbError().code);
+    CHECKEQUAL(expectedResult, trans->getNdbError().code);
 
     scan->close();