HPCC-12282 Move roxie global stats into new mechanism

Restrict size of activity stats tracking objects.
Don't report empty stats to workunit

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

roxie/ccd/ccdcontext.cpp

@@ -1016,6 +1016,8 @@ public:
 class CSlaveContext : public CInterface, implements IRoxieSlaveContext, implements ICodeContext, implements roxiemem::ITimeLimiter, implements IRowAllocatorMetaActIdCacheCallback
 {
 protected:
+    Owned<IConstWUGraphProgress> progress;  // These need to be destroyed very late (particularly, after the childgraphs)
+    Owned<IWUGraphStats> graphStats;
     mutable Owned<IRowAllocatorMetaActIdCache> allocatorMetaCache;
     Owned<IRowManager> rowManager; // NOTE: the order of destruction here is significant. For leak check to work destroy this BEFORE allAllocators, but after most other things
     Owned <IDebuggableContext> debugContext;
@@ -1363,9 +1365,6 @@ public:
         }
     }
 
-    Owned<IConstWUGraphProgress> progress;
-    Owned<IWUGraphStats> graphStats;
-
     void beginGraph(const char *graphName)
     {
         if (debugContext)

roxie/ccd/ccdserver.cpp

@@ -376,6 +376,29 @@ protected:
 
 //=================================================================================
 
+// General activity statistics
+
+static const StatisticsMapping actStatistics(StWhenFirstRow, StTimeElapsed, StTimeLocalExecute, StTimeTotalExecute, StSizeMaxRowSize,
+                                              StNumRowsProcessed, StNumSlaves, StNumStarted, StNumStopped, StKindNone);
+static const StatisticsMapping joinStatistics(actStatistics, StNumAtmostTriggered, StKindNone);
+static const StatisticsMapping keyedJoinStatistics(joinStatistics, StNumServerCacheHits, StNumIndexSeeks, StNumIndexScans, StNumIndexWildSeeks,
+                                                    StNumIndexSkips, StNumIndexNullSkips, StNumIndexMerges, StNumIndexMergeCompares,
+                                                    StNumPreFiltered, StNumPostFiltered, StNumIndexAccepted, StNumIndexRejected,
+                                                    StNumIndexRowsRead, StNumDiskRowsRead, StNumDiskSeeks, StNumDiskAccepted,
+                                                    StNumBlobCacheHits, StNumLeafCacheHits, StNumNodeCacheHits,
+                                                    StNumBlobCacheAdds, StNumLeafCacheAdds, StNumNodeCacheAdds,
+                                                    StNumDiskRejected, StKindNone);
+static const StatisticsMapping indexStatistics(actStatistics, StNumServerCacheHits, StNumIndexSeeks, StNumIndexScans, StNumIndexWildSeeks,
+                                                StNumIndexSkips, StNumIndexNullSkips, StNumIndexMerges, StNumIndexMergeCompares,
+                                                StNumPreFiltered, StNumPostFiltered, StNumIndexAccepted, StNumIndexRejected,
+                                                StNumBlobCacheHits, StNumLeafCacheHits, StNumNodeCacheHits,
+                                                StNumBlobCacheAdds, StNumLeafCacheAdds, StNumNodeCacheAdds,
+                                                StNumIndexRowsRead, StKindNone);
+static const StatisticsMapping diskStatistics(actStatistics, StNumServerCacheHits, StNumDiskRowsRead, StNumDiskSeeks, StNumDiskAccepted,
+                                               StNumDiskRejected, StKindNone);
+
+//=================================================================================
+
 class CRoxieServerActivityFactoryBase : public CActivityFactory, implements IRoxieServerActivityFactory
 {
 protected:
@@ -594,6 +617,10 @@ public:
     {
         // Most activities have nothing to say...
     }
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return actStatistics; // Overridden by anyone that needs more
+    }
 };
 
 class CRoxieServerMultiInputInfo
@@ -852,10 +879,6 @@ private:
     IRoxieServerActivityCopyArray & activities;
 };
 
-
-static const StatisticsMapping actStatistics(StWhenFirstRow, StTimeElapsed, StTimeLocalExecute, StTimeTotalExecute, StSizeMaxRowSize,
-                                             StNumRowsProcessed, StNumSlaves, StNumStarted, StNumStopped);
-
 class CRoxieServerActivity : public CInterface, implements IRoxieServerActivity, implements IRoxieInput, implements IRoxieContextLogger
 {
 protected:
@@ -885,12 +908,11 @@ protected:
 public:
     IMPLEMENT_IINTERFACE;
 
-    CRoxieServerActivity(const IRoxieServerActivityFactory *_factory, IProbeManager *_probeManager,
-                         const StatisticsMapping& _statsMapping = actStatistics)
+    CRoxieServerActivity(const IRoxieServerActivityFactory *_factory, IProbeManager *_probeManager)
         : factory(_factory), 
           basehelper(_factory->getHelper()),
           activityId(_factory->queryId()),
-          stats(_statsMapping)
+          stats(_factory ? factory->queryStatsMapping() : actStatistics)
     {
         input = NULL;
         ctx = NULL;
@@ -2467,7 +2489,7 @@ public:
                     logctx.CTXLOG("CRoxieServerSideCache::findCachedResult cache hit");
                 logctx.noteStatistic(StNumServerCacheHits, 1);
                 return NULL;
-                // Because IMessageResult cannot be replayed, this echeme is flawed. I'm leaving the code here just as a stats gatherer to see how useful it would have been....
+                // Because IMessageResult cannot be replayed, this scheme is flawed. I'm leaving the code here just as a stats gatherer to see how useful it would have been....
                 //IRoxieServerQueryPacket *ret = new CRoxieServerQueryPacket(p);
                 //ret->setResult(found->getResult());
                 //return ret;
@@ -12325,6 +12347,10 @@ public:
     }
 
     virtual unsigned numInputs() const { return 2; }
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return joinStatistics;
+    }
 };
 
 IRoxieServerActivityFactory *createRoxieServerJoinActivityFactory(unsigned _id, unsigned _subgraphId, IQueryFactory &_queryFactory, HelperFactory *_helperFactory, ThorActivityKind _kind)
@@ -16392,6 +16418,10 @@ public:
         //I don't think the action version of this is implemented - but this would be the code
         return isRoot && !meta.queryOriginal();
     }
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return allStatistics;  // Child queries...
+    }
 };
 
 IRoxieServerActivityFactory *createRoxieServerRemoteActivityFactory(unsigned _id, unsigned _subgraphId, IQueryFactory &_queryFactory, HelperFactory *_helperFactory, ThorActivityKind _kind, const RemoteActivityId &_remoteId, bool _isRoot)
@@ -17259,6 +17289,11 @@ public:
     {
         return new CRoxieServerSelfJoinActivity(this, _probeManager);
     }
+
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return joinStatistics;
+    }
 };
 
 IRoxieServerActivityFactory *createRoxieServerSelfJoinActivityFactory(unsigned _id, unsigned _subgraphId, IQueryFactory &_queryFactory, HelperFactory *_helperFactory, ThorActivityKind _kind)
@@ -18051,6 +18086,11 @@ public:
     {
         return new CRoxieServerLookupJoinActivity(this, _probeManager, useFewTable);
     }
+
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return joinStatistics;
+    }
 protected:
     bool useFewTable;
 };
@@ -18448,6 +18488,10 @@ public:
     {
         return new CRoxieServerAllJoinActivity(this, _probeManager);
     }
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return joinStatistics;
+    }
 };
 
 IRoxieServerActivityFactory *createRoxieServerAllJoinActivityFactory(unsigned _id, unsigned _subgraphId, IQueryFactory &_queryFactory, HelperFactory *_helperFactory, ThorActivityKind _kind)
@@ -21384,6 +21428,10 @@ public:
             }
         }
     }
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return diskStatistics;
+    }
 };
 
 IRoxieServerActivityFactory *createRoxieServerDiskReadActivityFactory(unsigned _id, unsigned _subgraphId, IQueryFactory &_queryFactory, HelperFactory *_helperFactory, ThorActivityKind _kind, const RemoteActivityId &_remoteId, IPropertyTree &_graphNode)
@@ -22513,6 +22561,10 @@ public:
         return activityMeta;
     }
 
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return indexStatistics;
+    }
 };
 
 class CRoxieServerIndexReadActivityFactory : public CRoxieServerBaseIndexActivityFactory
@@ -23487,6 +23539,10 @@ public:
             }
         }
     }
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return diskStatistics;
+    }
 };
 
 IRoxieServerActivityFactory *createRoxieServerFetchActivityFactory(unsigned _id, unsigned _subgraphId, IQueryFactory &_queryFactory, HelperFactory *_helperFactory, ThorActivityKind _kind, const RemoteActivityId &_remoteId, IPropertyTree &_graphNode)
@@ -25133,6 +25189,11 @@ public:
                 tailId, map, joinFlags, isLocal);
     }
 
+    virtual const StatisticsMapping &queryStatsMapping() const
+    {
+        return keyedJoinStatistics;
+    }
+
     virtual void getXrefInfo(IPropertyTree &reply, const IRoxieContextLogger &logctx) const
     {
         if (datafile)

roxie/ccd/ccdserver.hpp

@@ -212,6 +212,7 @@ interface IRoxieServerActivityFactory : extends IActivityFactory
     virtual IRoxieServerSideCache *queryServerSideCache() const = 0;
     virtual IDefRecordMeta *queryActivityMeta() const = 0;
     virtual unsigned numInputs() const = 0;
+    virtual const StatisticsMapping &queryStatsMapping() const = 0;
 };
 interface IGraphResult : public IInterface
 {

system/jlib/jstats.cpp

@@ -688,7 +688,24 @@ StatisticsMapping::StatisticsMapping(StatisticKind kind, ...)
         unsigned next  = va_arg(args, unsigned);
         if (!next)
             break;
-        indexToKind.append(next);
+        indexToKind.appendUniq(next);
+    }
+    va_end(args);
+    createMappings();
+}
+
+StatisticsMapping::StatisticsMapping(const StatisticsMapping &from, ...)
+{
+    ForEachItemIn(idx, from.indexToKind)
+        indexToKind.append(from.indexToKind.item(idx));
+    va_list args;
+    va_start(args, from);
+    for (;;)
+    {
+        unsigned next  = va_arg(args, unsigned);
+        if (!next)
+            break;
+        indexToKind.appendUniq(next);
     }
     va_end(args);
     createMappings();
@@ -1317,8 +1334,12 @@ void CRuntimeStatisticCollection::merge(const CRuntimeStatisticCollection & othe
     ForEachItemIn(i, other)
     {
         StatisticKind kind = other.getKind(i);
-        StatsMergeAction mergeAction = queryMergeMode(kind);
-        mergeStatistic(kind, other.getStatisticValue(kind), mergeAction);
+        unsigned __int64 value = other.getStatisticValue(kind);
+        if (value)
+        {
+            StatsMergeAction mergeAction = queryMergeMode(kind);
+            mergeStatistic(kind, other.getStatisticValue(kind), mergeAction);
+        }
     }
 }
 
@@ -1326,10 +1347,13 @@ void CRuntimeStatisticCollection::rollupStatistics(unsigned numTargets, IContext
 {
     ForEachItem(iStat)
     {
-        StatisticKind kind = getKind(iStat);
         unsigned __int64 value = values[iStat].getClear();
-        for (unsigned iTarget = 0; iTarget < numTargets; iTarget++)
-            targets[iTarget]->noteStatistic(kind, value);
+        if (value)
+        {
+            StatisticKind kind = getKind(iStat);
+            for (unsigned iTarget = 0; iTarget < numTargets; iTarget++)
+                targets[iTarget]->noteStatistic(kind, value);
+        }
     }
     reportIgnoredStats();
 }
@@ -1338,9 +1362,13 @@ void CRuntimeStatisticCollection::recordStatistics(IStatisticGatherer & target)
 {
     ForEachItem(i)
     {
-        StatisticKind kind = getKind(i);
-        StatsMergeAction mergeAction = queryMergeMode(kind);
-        target.updateStatistic(kind, values[i].get(), mergeAction);
+        unsigned __int64 value = values[i].get();
+        if (value)
+        {
+            StatisticKind kind = getKind(i);
+            StatsMergeAction mergeAction = queryMergeMode(kind);
+            target.updateStatistic(kind, values[i].get(), mergeAction);
+        }
     }
     reportIgnoredStats();
 }
@@ -1348,7 +1376,7 @@ void CRuntimeStatisticCollection::recordStatistics(IStatisticGatherer & target)
 void CRuntimeStatisticCollection::reportIgnoredStats() const
 {
     if (values[mapping.numStatistics()].getClear())
-        DBGLOG("Some statistics were addded but thrown away");
+        DBGLOG("Some statistics were added but thrown away");
 }
 
 StringBuffer & CRuntimeStatisticCollection::toXML(StringBuffer &str) const

system/jlib/jstats.h

@@ -406,6 +406,8 @@ class jlib_decl StatisticsMapping
 public:
     //Takes a list of StatisticKind terminated by StKindNone
     StatisticsMapping(StatisticKind kind, ...);
+    //Takes an existing Mapping, and extends it with a list of StatisticKind terminated by StKindNone
+    StatisticsMapping(const StatisticsMapping &, ...);
     //Accepts all StatisticKind values
     StatisticsMapping();
 
@@ -475,11 +477,13 @@ public:
     inline CRuntimeStatistic & queryStatistic(StatisticKind kind)
     {
         unsigned index = queryMapping().getIndex(kind);
+        dbgassertex(index < mapping.numStatistics());
         return values[index];
     }
     inline const CRuntimeStatistic & queryStatistic(StatisticKind kind) const
     {
         unsigned index = queryMapping().getIndex(kind);
+        dbgassertex(index < mapping.numStatistics());
         return values[index];
     }