Changes following review

Signed-off-by: Shamser Ahmed <[email protected]>
hpcc-systems · Nov 24, 2023 · bc3cac4 · bc3cac4
1 parent 9f160f7
commit bc3cac4
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 88 deletions.
diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp
@@ -231,10 +231,10 @@ extern da_decl double calcFileAccessCost(IDistributedFile *f, __int64 numDiskWri
     return calcFileAccessCost(clusterName, numDiskWrites, numDiskReads);
 }
 
-extern da_decl cost_type calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads)
+extern da_decl double calcFileAccessCost(IDistributedFile *f, __int64 numDiskWrites, __int64 numDiskReads)
 {
     StringBuffer clusterName;
-    f->getClusterName(0, clusterName);
+    f->getClusterName(0, clusterName); // Should really specify the cluster number too, but this ok for now
     return money2cost_type(calcFileAccessCost(clusterName, numDiskWrites, numDiskReads));
 }
 

diff --git a/system/jhtree/jhtree.cpp b/system/jhtree/jhtree.cpp
@@ -576,9 +576,9 @@ class jhtree_decl CKeyLevelManager : implements IKeyManager, public CInterface
     virtual void mergeStats(CRuntimeStatisticCollection & targetStats) const
     {
         if (keyCursor)
-            keyCursor->mergeStats(targetStats);
+            keyCursor->mergeStats(targetStats); // merge IO stats
         if (stats.ctx)
-            targetStats.merge(stats.ctx->queryStats());
+            targetStats.merge(stats.ctx->queryStats()); // merge jhtree cache stats
     }
 };
 

diff --git a/thorlcr/activities/indexread/thindexreadslave.cpp b/thorlcr/activities/indexread/thindexreadslave.cpp
@@ -98,7 +98,7 @@ class CIndexReadSlaveBase : public CSlaveActivity
             if (!keyManager)
                 throw MakeActivityException(&activity, 0, "Callback attempting to read blob with no key manager - index being read remotely?");
             needsBlobCleaning = true;
-            return (byte *) keyManager->loadBlob(id, dummy, &activity.contextLogger);
+            return (byte *) keyManager->loadBlob(id, dummy, nullptr);
         }
         void prepareManager(IKeyManager *_keyManager)
         {
@@ -296,7 +296,7 @@ class CIndexReadSlaveBase : public CSlaveActivity
                     part.queryOwner().getClusterLabel(0, planeName);
                     blockedSize = getBlockedFileIOSize(planeName);
                 }
-                lazyIFileIO.setown(queryThor().queryFileCache().lookupIFileIO(*this, logicalFilename, part, nullptr, indexReadActivityStatistics, blockedSize));
+                lazyIFileIO.setown(queryThor().queryFileCache().lookupIFileIO(*this, logicalFilename, part, nullptr, indexReadFileStatistics, blockedSize));
 
                 RemoteFilename rfn;
                 part.getFilename(0, rfn);
@@ -348,44 +348,43 @@ class CIndexReadSlaveBase : public CSlaveActivity
         else
             return nullptr;
     }
-    void mergeFileStats(IPartDescriptor *partDesc, IFileIO *partIO)
-    {
-        if (!currentManager)
-            return;
-        if (fileStats.size()>0)
-        {
-            ISuperFileDescriptor * superFDesc = partDesc->queryOwner().querySuperFileDescriptor();
-            if (superFDesc)
-            {
-                unsigned subfile, lnum;
-                if(superFDesc->mapSubPart(partDesc->queryPartIndex(), subfile, lnum))
-                    currentManager->mergeStats(*fileStats[fileTableStart+subfile]);
-            }
-            else
-            {
-                currentManager->mergeStats(*fileStats[fileTableStart]);
-            }
-        }
-    }
     void updateStats()
     {
+        // Merge jhtree cache stats and file io stats for current part
         // NB: updateStats() should always be called whilst ioStatsCS is held.
+        // NB: caller ensures that currentManager must be valid
         if (lazyIFileIO)
         {
+            // merge stats for activity
             mergeStats(inactiveStats, lazyIFileIO);
+            currentManager->mergeStats(inactiveStats);
+            // merge stats for part
             if (currentPart<partDescs.ordinality())
-                mergeFileStats(&partDescs.item(currentPart), lazyIFileIO);
+            {
+                assertex(fileStats.size());
+                IPartDescriptor & curPartDesc = partDescs.item(currentPart);
+                ISuperFileDescriptor * superFDesc = curPartDesc.queryOwner().querySuperFileDescriptor();
+                if (superFDesc)
+                {
+                    unsigned subfile, lnum;
+                    if(superFDesc->mapSubPart(curPartDesc.queryPartIndex(), subfile, lnum))
+                        currentManager->mergeStats(*fileStats[fileTableStart+subfile]);
+                }
+                else
+                {
+                    currentManager->mergeStats(*fileStats[fileTableStart]);
+                }
+            }
         }
     }
     void configureNextInput()
     {
         if (currentManager)
         {
-            resetManager(currentManager);
-            currentManager = nullptr;
-
             CriticalBlock b(ioStatsCS);
             updateStats();
+            resetManager(currentManager);
+            currentManager = nullptr;
             lazyIFileIO.clear();
         }
         IKeyManager *keyManager = nullptr;
@@ -688,7 +687,7 @@ class CIndexReadSlaveBase : public CSlaveActivity
                 }
             }
             data.read(fileTableStart);
-            setupSpace4FileStats(fileTableStart, reInit, super!=nullptr, super?super->querySubFiles():0, indexReadActivityStatistics);
+            setupSpace4FileStats(fileTableStart, reInit, super!=nullptr, super?super->querySubFiles():0, indexReadFileStatistics);
         }
     }
     // IThorDataLink

diff --git a/thorlcr/activities/keyedjoin/thkeyedjoin.cpp b/thorlcr/activities/keyedjoin/thkeyedjoin.cpp
@@ -304,7 +304,7 @@ class CKeyedJoinMaster : public CMasterActivity
         totalIndexParts = 0;
 
         Owned<IDistributedFile> dataFile;
-        Owned<IDistributedFile> indexFile = lookupReadFile(indexFileName, AccessMode::readRandom, false, false, 0 != (helper->getJoinFlags() & JFindexoptional), true, indexReadActivityStatistics, &indexFileStatsTableEntry);
+        Owned<IDistributedFile> indexFile = lookupReadFile(indexFileName, AccessMode::readRandom, false, false, 0 != (helper->getJoinFlags() & JFindexoptional), true, indexReadFileStatistics, &indexFileStatsTableEntry);
         if (indexFile)
         {
             if (!isFileKey(indexFile))