Bug#35783683 Ndb table reorganize modifies tuple header after checksum calculated

Any changes to a tuple header must be done before the checksum is
recalculated.

Fix regression from 8.0 Query Threads feature resulting in checksum
calculation being moved before and not considering Reorg bit setting.

Problem visibly fixed when running forthcoming testcase exercising
multiple operations on rows undergoing reorg.

Change-Id: I743765bf022c471ba4cb3f72796de30481aef0e0

Author: frazerclement

storage/ndb/src/kernel/blocks/dbtup/DbtupExecQuery.cpp

@@ -1895,30 +1895,31 @@ int Dbtup::handleReadReq(Signal* signal,
 }
 
 static
-void
-handle_reorg(Dbtup::KeyReqStruct * req_struct,
-             Dbtup::Fragrecord::FragState state)
+Uint32
+get_reorg_flag(Dbtup::KeyReqStruct * req_struct,
+               Dbtup::Fragrecord::FragState state)
 {
   Uint32 reorg = req_struct->m_reorg;
   switch(state){
   case Dbtup::Fragrecord::FS_FREE:
   case Dbtup::Fragrecord::FS_REORG_NEW:
   case Dbtup::Fragrecord::FS_REORG_COMMIT_NEW:
   case Dbtup::Fragrecord::FS_REORG_COMPLETE_NEW:
-    return;
+    return 0;
   case Dbtup::Fragrecord::FS_REORG_COMMIT:
   case Dbtup::Fragrecord::FS_REORG_COMPLETE:
     if (reorg != ScanFragReq::REORG_NOT_MOVED)
-      return;
+      return 0;
     break;
   case Dbtup::Fragrecord::FS_ONLINE:
     if (reorg != ScanFragReq::REORG_MOVED)
-      return;
+      return 0;
     break;
   default:
-    return;
+    return 0;
   }
-  req_struct->m_tuple_ptr->m_header_bits |= Dbtup::Tuple_header::REORG_MOVE;
+
+  return Dbtup::Tuple_header::REORG_MOVE;
 }
 
 /* ---------------------------------------------------------------- */
@@ -2093,7 +2094,8 @@ int Dbtup::handleUpdateReq(Signal* signal,
 
   if (req_struct->m_reorg != ScanFragReq::REORG_ALL)
   {
-    handle_reorg(req_struct, regFragPtr->fragStatus);
+    req_struct->m_tuple_ptr->m_header_bits |=
+      get_reorg_flag(req_struct, regFragPtr->fragStatus);
   }
 
   req_struct->m_tuple_ptr->set_tuple_version(tup_version);
@@ -2909,6 +2911,13 @@ int Dbtup::handleInsertReq(Signal* signal,
 
       disk_ptr->set_base_record_ref(ref);
     }
+
+    if (req_struct->m_reorg != ScanFragReq::REORG_ALL)
+    {
+      req_struct->m_tuple_ptr->m_header_bits |=
+        get_reorg_flag(req_struct, regFragPtr->fragStatus);
+    }
+
     setChecksum(req_struct->m_tuple_ptr, regTabPtr);
     /**
      * At this point we hold the fragment mutex to ensure that TUP scans
@@ -2924,6 +2933,7 @@ int Dbtup::handleInsertReq(Signal* signal,
   }
   else 
   {
+    /* ! mem_insert */
     if (ERROR_INSERTED(4020))
     {
       c_lqh->upgrade_to_exclusive_frag_access();
@@ -2947,11 +2957,23 @@ int Dbtup::handleInsertReq(Signal* signal,
      * finalizing the writes on the row.
      */
     m_base_header_bits &= ~(Uint32)Tuple_header::FREE;
-  }
 
-  if (req_struct->m_reorg != ScanFragReq::REORG_ALL)
-  {
-    handle_reorg(req_struct, regFragPtr->fragStatus);
+    if (req_struct->m_reorg != ScanFragReq::REORG_ALL)
+    {
+      m_base_header_bits |=
+        get_reorg_flag(req_struct, regFragPtr->fragStatus);
+    }
+
+    /**
+     * Fragment-locking :
+     *   No need to protect this checksum write, we only perform it here for
+     *   non-first inserts since first insert operations are handled above
+     *   while holding the mutex. For non-first operations the row is not
+     *   visible to other threads at this time, copy rows are not even visible to
+     *   TUP scans, thus no need to protect it here. The row becomes visible
+     *   when inserted into the active list after returning from this call.
+     */
+    setChecksum(req_struct->m_tuple_ptr, regTabPtr);
   }
 
   /* Have been successful with disk + mem, update ACC to point to
@@ -2974,19 +2996,7 @@ int Dbtup::handleInsertReq(Signal* signal,
   {
     * accminupdateptr = 0; // No accminupdate should be performed
   }
-  if (!mem_insert)
-  {
-    /**
-     * No need to protect this checksum write, we only perform it here for
-     * non-first inserts since first insert operations are handled above
-     * while holding the mutex. For non-first operations the row is not
-     * visible to others at this time, copy rows are not even visible to
-     * TUP scans, thus no need to protect it here. The row becomes visible
-     * when inserted into the active list after returning from this call.
-     */
-    jamDebug();
-    setChecksum(req_struct->m_tuple_ptr, regTabPtr);
-  }
+
   set_tuple_state(regOperPtr.p, TUPLE_PREPARED);
   return 0;