Merge pull request #3979 from jakesmith/hpcc-8735

HPCC-8735 - Avoid seperate call to fetch total # workunits

Reviewed-By: Gavin Halliday <gavin.halliday@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

common/workunit/workunit.cpp

@@ -2183,15 +2183,19 @@ public:
                                                 const char *queryowner, 
                                                 __int64 *cachehint,
                                                 ISecManager *secmgr, 
-                                                ISecUser *secuser)
+                                                ISecUser *secuser,
+                                                unsigned *total)
     {
-        class cScopeChecker: implements ISortedElementsTreeFilter
+        class CScopeChecker : public CSimpleInterface, implements ISortedElementsTreeFilter
         {
             UniqueScopes done;
             ISecManager *secmgr;
             ISecUser *secuser;
+            CriticalSection crit;
         public:
-            cScopeChecker(ISecManager *_secmgr,ISecUser *_secuser)
+            IMPLEMENT_IINTERFACE_USING(CSimpleInterface);
+
+            CScopeChecker(ISecManager *_secmgr,ISecUser *_secuser)
             {
                 secmgr = _secmgr;
                 secuser = _secuser;
@@ -2201,14 +2205,23 @@ public:
                 const char *scopename = tree.queryProp("@scope");
                 if (!scopename||!*scopename)
                     return true;
-                const bool *b = done.getValue(scopename);
-                if (b)
-                    return *b;
+
+                {
+                    CriticalBlock block(crit);
+                    const bool *b = done.getValue(scopename);
+                    if (b)
+                        return *b;
+                }
                 bool ret = checkWuScopeSecAccess(scopename,*secmgr,secuser,SecAccess_Read,"iterating",false,false);
-                done.setValue(scopename,ret);
+                {
+                    // conceivably could have already been checked and added, but ok.
+                    CriticalBlock block(crit);
+                    done.setValue(scopename,ret);
+                }
                 return ret;
             }
-        } sc(secmgr,secuser);
+        };
+        Owned<ISortedElementsTreeFilter> sc = new CScopeChecker(secmgr,secuser);
         StringBuffer query("*");
         StringBuffer so;
         StringAttr namefilterlo;
@@ -2249,7 +2262,7 @@ public:
         }
         IArrayOf<IPropertyTree> results;
         Owned<IRemoteConnection> conn=getElementsPaged( "WorkUnits", query.str(), so.length()?so.str():NULL,startoffset,maxnum,
-            secmgr?&sc:NULL,queryowner,cachehint,namefilterlo.get(),namefilterhi.get(),results);
+            secmgr?sc:NULL,queryowner,cachehint,namefilterlo.get(),namefilterhi.get(),results,total);
         return new CConstWUArrayIterator(conn, results, secmgr, secuser);
     }
 
@@ -2260,9 +2273,10 @@ public:
                                                 unsigned startoffset,
                                                 unsigned maxnum,
                                                 const char *queryowner, 
-                                                __int64 *cachehint)
+                                                __int64 *cachehint,
+                                                unsigned *total)
     {
-        return getWorkUnitsSorted(sortorder,filters,filterbuf,startoffset,maxnum,queryowner,cachehint, NULL, NULL);
+        return getWorkUnitsSorted(sortorder,filters,filterbuf,startoffset,maxnum,queryowner,cachehint, NULL, NULL, total);
     }
 
     virtual unsigned numWorkUnits()
@@ -2279,12 +2293,9 @@ public:
                                         ISecManager *secmgr, 
                                         ISecUser *secuser)
     {
-        Owned<IConstWorkUnitIterator> iter =  getWorkUnitsSorted( NULL,filters,filterbuf,0,0x7fffffff,NULL,NULL,secmgr,secuser);
-        // this is rather slow but necessarily so (for security check)
-        unsigned ret = 0;
-        ForEach(*iter)
-            ret++;
-        return ret;
+        unsigned total;
+        Owned<IConstWorkUnitIterator> iter =  getWorkUnitsSorted( NULL,filters,filterbuf,0,0x7fffffff,NULL,NULL,secmgr,secuser,&total);
+        return total;
     }
 
     virtual unsigned numWorkUnitsFiltered(WUSortField *filters,const void *filterbuf)
@@ -2494,9 +2505,10 @@ public:
                                                         unsigned startoffset,
                                                         unsigned maxnum,
                                                         const char *queryowner, 
-                                                        __int64 *cachehint)
+                                                        __int64 *cachehint,
+                                                        unsigned *total)
     {
-        return factory->getWorkUnitsSorted(sortorder,filters,filterbuf,startoffset,maxnum,queryowner,cachehint, secMgr.get(), secUser.get());
+        return factory->getWorkUnitsSorted(sortorder,filters,filterbuf,startoffset,maxnum,queryowner,cachehint, secMgr.get(), secUser.get(), total);
     }
 
     virtual unsigned numWorkUnits()

common/workunit/workunit.hpp

@@ -1100,7 +1100,7 @@ interface IWorkUnitFactory : extends IInterface
     virtual IConstWorkUnitIterator * getWorkUnitsByECL(const char * ecl) = 0;
     virtual IConstWorkUnitIterator * getWorkUnitsByCluster(const char * cluster) = 0;
     virtual IConstWorkUnitIterator * getWorkUnitsByXPath(const char * xpath) = 0;
-    virtual IConstWorkUnitIterator * getWorkUnitsSorted(WUSortField * sortorder, WUSortField * filters, const void * filterbuf, unsigned startoffset, unsigned maxnum, const char * queryowner, __int64 * cachehint) = 0;
+    virtual IConstWorkUnitIterator * getWorkUnitsSorted(WUSortField * sortorder, WUSortField * filters, const void * filterbuf, unsigned startoffset, unsigned maxnum, const char * queryowner, __int64 * cachehint, unsigned *total) = 0;
     virtual unsigned numWorkUnits() = 0;
     virtual unsigned numWorkUnitsFiltered(WUSortField * filters, const void * filterbuf) = 0;
     virtual void descheduleAllWorkUnits() = 0;

dali/base/dautils.cpp

@@ -25,6 +25,7 @@
 #include "jsort.hpp"
 #include "jprop.hpp"
 #include "jregexp.hpp"
+#include "jset.hpp"
 #include "rmtfile.hpp"
 
 #include "mpbase.hpp"
@@ -1807,15 +1808,19 @@ class CPECacheElem: public CTimedCacheItem
 {
 public:
     IMPLEMENT_IINTERFACE;
-    CPECacheElem(const char *owner)
-        : CTimedCacheItem(owner)
+    CPECacheElem(const char *owner, ISortedElementsTreeFilter *_postFilter)
+        : CTimedCacheItem(owner), postFilter(_postFilter), postFiltered(0)
     {
+        passesFilter.setown(createBitSet());
     }
     ~CPECacheElem()
     {
     }
     Owned<IRemoteConnection> conn;
     IArrayOf<IPropertyTree> totalres;
+    Linked<ISortedElementsTreeFilter> postFilter;
+    unsigned postFiltered;
+    Owned<IBitSet> passesFilter;
 };
 
 IRemoteConnection *getElementsPaged( const char *basexpath,
@@ -1828,7 +1833,8 @@ IRemoteConnection *getElementsPaged( const char *basexpath,
                                      __int64 *hint,
                                      const char *namefilterlo,
                                      const char *namefilterhi,
-                                     IArrayOf<IPropertyTree> &results)
+                                     IArrayOf<IPropertyTree> &results,
+                                     unsigned *total)
 {
     if (pagesize==0)
         return NULL;
@@ -1841,28 +1847,53 @@ IRemoteConnection *getElementsPaged( const char *basexpath,
     }
     Owned<CPECacheElem> elem;
     if (hint&&*hint)
-        elem.setown(QUERYINTERFACE(pagedElementsCache->get(owner,*hint),CPECacheElem));
+    {
+        elem.setown(QUERYINTERFACE(pagedElementsCache->get(owner,*hint),CPECacheElem)); // NB: removes from cache in process, added back at end
+        postfilter = elem->postFilter; // reuse cached postfilter
+    }
     if (!elem)
-        elem.setown(new CPECacheElem(owner));
+        elem.setown(new CPECacheElem(owner, postfilter));
     if (!elem->conn)
         elem->conn.setown(getSortedElements(basexpath,xpath,sortorder,namefilterlo,namefilterhi,elem->totalres));
     if (!elem->conn)
         return NULL;
     unsigned n;
+    if (total)
+        *total = elem->totalres.ordinality();
     if (postfilter) {
+        unsigned numFiltered = 0;
         n = 0;
         ForEachItemIn(i,elem->totalres) {
             IPropertyTree &item = elem->totalres.item(i);
-            if (postfilter->isOK(item)) {
+            bool passesFilter = false;
+            if (elem->postFiltered>i) // postFiltered is high water mark of items checked
+                passesFilter = elem->passesFilter->test(i);
+            else
+            {
+                passesFilter = postfilter->isOK(item);
+                elem->passesFilter->set(i, passesFilter);
+                elem->postFiltered = i+1;
+            }
+            if (passesFilter)
+            {
                 if (n>=startoffset) {
                     item.Link();
                     results.append(item);
                     if (results.ordinality()>=pagesize)
-                        break;
+                    {
+                        // if total needed, need to iterate through all items
+                        if (NULL == total)
+                            break;
+                        startoffset = (unsigned)-1; // no more results needed
+                    }
                 }
                 n++;
             }
+            else
+                ++numFiltered;
         }
+        if (total)
+            *total -= numFiltered;
     }
     else {
         n = (elem->totalres.ordinality()>startoffset)?(elem->totalres.ordinality()-startoffset):0;

dali/base/dautils.hpp

@@ -259,7 +259,7 @@ IRemoteConnection *getSortedElements( const char *basexpath,
                                      const char *namefilterlo, // if non null filter less than this value
                                      const char *namefilterhi, // if non null filter greater than this value
                                      IArrayOf<IPropertyTree> &results);
-interface ISortedElementsTreeFilter
+interface ISortedElementsTreeFilter : extends IInterface
 {
     virtual bool isOK(IPropertyTree &tree) = 0;
 };
@@ -274,7 +274,8 @@ extern da_decl IRemoteConnection *getElementsPaged( const char *basexpath,
                                      __int64 *hint,                         // if non null points to in/out cache hint
                                      const char *namefilterlo, // if non null filter less than this value
                                      const char *namefilterhi, // if non null filter greater than this value
-                                     IArrayOf<IPropertyTree> &results);
+                                     IArrayOf<IPropertyTree> &results,
+                                     unsigned *total); // total possible filtered matches, i.e. irrespective of startoffset and pagesize
 
 extern da_decl void clearPagedElementsCache();
 

dali/datest/dfuwutest.cpp

@@ -641,7 +641,7 @@ void testPagedIterate()
     unsigned n=0;
     for (unsigned page=0;page<3;page++) {
         DFUsortfield sortorder[] = {DFUsf_user,DFUsf_state,DFUsf_term};
-        Owned<IConstDFUWorkUnitIterator> iter = factory->getWorkUnitsSorted(sortorder, NULL, NULL, page*10, 10, "nigel", &cachehint);
+        Owned<IConstDFUWorkUnitIterator> iter = factory->getWorkUnitsSorted(sortorder, NULL, NULL, page*10, 10, "nigel", &cachehint, NULL);
         StringBuffer s;
         ForEach(*iter) {
 

dali/dfu/dfuwu.cpp

@@ -2967,7 +2967,8 @@ public:
                                                     unsigned startoffset,
                                                     unsigned maxnum,
                                                     const char *queryowner, 
-                                                    __int64 *cachehint)
+                                                    __int64 *cachehint,
+                                                    unsigned *total)
     {
         StringBuffer query("*");
         StringBuffer so;
@@ -3005,7 +3006,7 @@ public:
         }
         IArrayOf<IPropertyTree> results;
         Owned<IRemoteConnection> conn=getElementsPaged( "DFU/WorkUnits", query.str(), so.length()?so.str():NULL,startoffset,maxnum,
-            NULL,queryowner,cachehint,namefilterlo.get(),namefilterhi.get(),results);
+            NULL,queryowner,cachehint,namefilterlo.get(),namefilterhi.get(),results,total);
         return new CConstDFUWUArrayIterator(this,conn,results);
     }
 
@@ -3022,12 +3023,9 @@ public:
     {
         if (!filters)
             return numWorkUnits();
-        // bit slow
-        Owned<IConstDFUWorkUnitIterator> iter =  getWorkUnitsSorted( NULL,filters,filterbuf,0,0x7fffffff,NULL,NULL);
-        unsigned ret = 0;
-        ForEach(*iter)
-            ret++;
-        return ret;
+        unsigned total;
+        Owned<IConstDFUWorkUnitIterator> iter = getWorkUnitsSorted( NULL,filters,filterbuf,0,0x7fffffff,NULL,NULL,&total);
+        return total;
     }
 
 

dali/dfu/dfuwu.hpp

@@ -440,7 +440,8 @@ interface IDFUWorkUnitFactory : extends IInterface
                                                         unsigned startoffset,
                                                         unsigned maxnum,
                                                         const char *queryowner, 
-                                                        __int64 *cachehint) = 0;    // set to NULL if caching not required
+                                                        __int64 *cachehint,         // set to NULL if caching not required
+                                                        unsigned *total) = 0;       // set to NULL if caching not required
     virtual unsigned numWorkUnits()=0;
     virtual unsigned numWorkUnitsFiltered(DFUsortfield *filters,const void *filterbuf)=0;
     virtual __int64  subscribe(const char *xpath,void *iface) =0;       // internal use

ecl/wutest/wutest.cpp

@@ -146,7 +146,7 @@ void testPagedWuList(IWorkUnitFactory *factory)
     unsigned n=0;
     for (unsigned page=0;page<3;page++) {
         WUSortField sortorder[] = {WUSFuser,WUSFstate,WUSFterm};
-        Owned<IConstWorkUnitIterator> it = factory->getWorkUnitsSorted(sortorder, NULL, NULL, page*10, 10, "nigel", &cachehint);
+        Owned<IConstWorkUnitIterator> it = factory->getWorkUnitsSorted(sortorder, NULL, NULL, page*10, 10, "nigel", &cachehint, NULL);
         ForEach(*it) {
             n++;
             IConstWorkUnit& wu = it->query();

esp/services/ws_fs/ws_fsService.cpp

@@ -952,8 +952,8 @@ bool CFileSprayEx::onGetDFUWorkunits(IEspContext &context, IEspGetDFUWorkunits &
 
         IArrayOf<IEspDFUWorkunit> result;
         Owned<IDFUWorkUnitFactory> factory = getDFUWorkUnitFactory();
-        unsigned numWUs = factory->numWorkUnitsFiltered(filters, filterbuf.bufferBase());
-        Owned<IConstDFUWorkUnitIterator> itr = factory->getWorkUnitsSorted(sortorder, filters, filterbuf.bufferBase(), (int) displayFrom, (int) pagesize+1, req.getOwner(), &cachehint);
+        unsigned numWUs;
+        Owned<IConstDFUWorkUnitIterator> itr = factory->getWorkUnitsSorted(sortorder, filters, filterbuf.bufferBase(), (int) displayFrom, (int) pagesize+1, req.getOwner(), &cachehint, &numWUs);
 
         //unsigned actualCount = 0;
         itr->first();

esp/services/ws_workunits/ws_workunitsService.cpp

@@ -2023,8 +2023,8 @@ void doWUQueryWithSort(IEspContext &context, IEspWUQueryRequest & req, IEspWUQue
     filters[filterCount] = WUSFterm;
 
     Owned<IWorkUnitFactory> factory = getWorkUnitFactory(context.querySecManager(), context.queryUser());
-    unsigned numWUs = factory->numWorkUnitsFiltered(filters, filterbuf.bufferBase());
-    Owned<IConstWorkUnitIterator> it = factory->getWorkUnitsSorted(sortorder, filters, filterbuf.bufferBase(), begin, pagesize+1, "", NULL);
+    unsigned numWUs;
+    Owned<IConstWorkUnitIterator> it = factory->getWorkUnitsSorted(sortorder, filters, filterbuf.bufferBase(), begin, pagesize+1, "", NULL, &numWUs);
 
     unsigned actualCount = 0;
     ForEach(*it)