Browse Source

HPCC-15476 Error viewing Roxie graphs if multiple packages

This fixes the problem in two places - though either would probably be enough:

1. Fixes the code in mergeStats to handle duplicates better - so that if we
DID want to include stats from all instances of a query in multiple packages,
we probably could.

2. Modifies the meaning of control:queryStats to refer to the active copy of
the named query - which is almost certainly what is wanted!

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 9 năm trước cách đây
mục cha
commit
a50bde1106
1 tập tin đã thay đổi với 59 bổ sung30 xóa
  1. 59 30
      roxie/ccd/ccdstate.cpp

+ 59 - 30
roxie/ccd/ccdstate.cpp

@@ -1353,25 +1353,30 @@ public:
         return true;
     }
 
-    void getStats(const char *queryId, const char *action, const char *graphName, StringBuffer &reply, const IRoxieContextLogger &logctx) const
+    bool getStats(const char *queryId, const char *action, const char *graphName, StringBuffer &reply, const IRoxieContextLogger &logctx) const
     {
         CriticalBlock b2(updateCrit);
-        Owned<IQueryFactory> query = serverManager->getQuery(queryId, NULL, logctx);
-        if (query)
+        if (serverManager->isActive())
         {
-            StringBuffer freply;
-            serverManager->getStats(queryId, graphName, freply, logctx);
-            Owned<IPropertyTree> stats = createPTreeFromXMLString(freply.str());
-            for (unsigned channel = 0; channel < numChannels; channel++)
-                if (slaveManagers->item(channel))
-                {
-                    StringBuffer sreply;
-                    slaveManagers->item(channel)->getStats(queryId, graphName, sreply, logctx);
-                    Owned<IPropertyTree> cstats = createPTreeFromXMLString(sreply.str());
-                    mergeStats(stats, cstats, 1);
-                }
-            toXML(stats, reply);
+            Owned<IQueryFactory> query = serverManager->getQuery(queryId, NULL, logctx);
+            if (query)
+            {
+                StringBuffer freply;
+                serverManager->getStats(queryId, graphName, freply, logctx);
+                Owned<IPropertyTree> stats = createPTreeFromXMLString(freply.str());
+                for (unsigned channel = 0; channel < numChannels; channel++)
+                    if (slaveManagers->item(channel))
+                    {
+                        StringBuffer sreply;
+                        slaveManagers->item(channel)->getStats(queryId, graphName, sreply, logctx);
+                        Owned<IPropertyTree> cstats = createPTreeFromXMLString(sreply.str());
+                        mergeStats(stats, cstats, 1);
+                    }
+                toXML(stats, reply);
+                return true;
+            }
         }
+        return false;
     }
     void getActivityMetrics(StringBuffer &reply) const
     {
@@ -1659,7 +1664,8 @@ public:
     {
         ForEachItemIn(idx, allQueryPackages)
         {
-            allQueryPackages.item(idx).getStats(id, action, graphName, reply, logctx);
+            if (allQueryPackages.item(idx).getStats(id, action, graphName, reply, logctx))
+               return;
         }
     }
 
@@ -2994,27 +3000,22 @@ void mergeNode(IPropertyTree *s1, IPropertyTree *s2, unsigned level)
 void mergeStats(IPropertyTree *s1, IPropertyTree *s2, unsigned level)
 {
     MergeInfo & mi = mergeTable[level];
-    Owned<IPropertyTreeIterator> elems = s1->getElements(mi.element);
+    Owned<IPropertyTreeIterator> elems = s2->getElements(mi.element);
     ForEach(*elems)
     {
-        IPropertyTree &e1 = elems->query();
+        IPropertyTree &e2 = elems->query();
         StringBuffer xpath;
         if (mi.attribute)
-            xpath.appendf("%s[%s='%s']", mi.element, mi.attribute, e1.queryProp(mi.attribute));
+            xpath.appendf("%s[%s='%s']", mi.element, mi.attribute, e2.queryProp(mi.attribute));
         else
             xpath.append(mi.element);
-        IPropertyTree *e2 = s2->queryPropTree(xpath.str());
-        if (e2)
+        IPropertyTree *e1 = s1->queryPropTree(xpath.str());
+        if (e1)
         {
-            mi.f(&e1, e2, level+1);
-            s2->removeTree(e2);
+            mi.f(e1, &e2, level+1);
         }
-    }
-    elems.setown(s2->getElements(mi.element));
-    ForEach(*elems)
-    {
-        IPropertyTree &e2 = elems->query();
-        s1->addPropTree(mi.element, LINK(&e2));
+        else
+            s1->addPropTree(mi.element, LINK(&e2));
     }
 }
 
@@ -3259,7 +3260,8 @@ static const char *expected =
 class MergeStatsTest : public CppUnit::TestFixture
 {
     CPPUNIT_TEST_SUITE( MergeStatsTest );
-        CPPUNIT_TEST(test1);
+    CPPUNIT_TEST(test1);
+    // CPPUNIT_TEST(test2);  Handy for debugging problem cases...
     CPPUNIT_TEST_SUITE_END();
 
 protected:
@@ -3274,6 +3276,33 @@ protected:
         toXML(e, s2);
         CPPUNIT_ASSERT(strcmp(s1, s2)==0);
     }
+    void test2()
+    {
+        Owned<IPropertyTree> mergedReply = createPTree("Merged");
+        Owned<IPropertyTree> p1 = createPTreeFromXMLFile("stats1.xml");
+        Owned<IPropertyTreeIterator> meat = p1->getElements("Endpoint");
+        ForEach(*meat)
+        {
+            if (mergedReply)
+            {
+                mergeStats(mergedReply, &meat->query());
+            }
+        }
+        Owned<IPropertyTree> p2 = createPTreeFromXMLFile("stats2.xml");
+        meat.setown(p2->getElements("Endpoint"));
+        ForEach(*meat)
+        {
+            if (mergedReply)
+            {
+                mergeStats(mergedReply, &meat->query());
+            }
+        }
+        StringBuffer s1;
+        toXML(mergedReply, s1);
+        //toXML(e, s2);
+        //CPPUNIT_ASSERT(strcmp(s1, s2)==0);
+        printf("%s", s1.str());
+    }
 
 };