Merge remote-tracking branch 'origin/candidate-10.0.x' into candidate-10.2.x

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>

Author: GordonSmith

.github/instructions/esp-service-interface.instructions.md

@@ -8,7 +8,8 @@ applyTo: '**/*.ecm'
 - Add a version attribute to any new or deprecated element.
 - Use the next highest version number in the service's `*.ecm` file for versioning changes.
 - Increment the `version` and `default_client_version` attributes of the `ESPservice` element.
-- Include the `exceptions_inline` attribute in any new `ESPresponse` and `ESPmethod` element.
+- Include the `exceptions_inline` attribute in any new `ESPresponse` element.
+- Include the `exceptions_inline` attribute in any new `ESPmethod` element only if its parent `ESPservice` element doesn't have an `exceptions_inline` attribute.
 
 ## Naming Conventions
 - Use `PascalCase` for element names.

.github/workflows/build-clienttools-windows-2022.yml

@@ -145,7 +145,7 @@ jobs:
           generateReleaseNotes: false
           prerelease: ${{ contains(github.ref, '-rc') }}
           makeLatest: ${{ inputs.make-latest }}
-          artifacts: "${{ steps.build_clienttools.outputs.packages }}"
+          artifacts: "./build/hpccsystems-clienttools-community*.exe"
 
       - name: Release Internal Clienttools to JFrog Repository
         if: ${{ inputs.ln-ref != '' && github.repository_owner == 'hpcc-systems' }}

common/eventconsumption/eventdumpstream.cpp

@@ -493,10 +493,16 @@ class CDumpCSVEventVisitor : public CDumpStreamEventVisitor
     }
 
 protected:
-    // No attributes to be skipped at this time. Retain for possible future use.
     bool skipAttribute(unsigned id) const
     {
-       return false;
+        switch (id)
+        {
+        case EvAttrProcessDescriptor:
+            // Only appears in RecordingSource, which is never visited.
+            return true;
+        default:
+            return false;
+        }
     }
 
     virtual void recordAttribute(EventAttr id, const char*, const char* value, bool) override

common/eventconsumption/eventsaveas.cpp

@@ -52,7 +52,7 @@ bool CEventSavingOp::ready() const
 bool CEventSavingOp::doOp()
 {
     EventRecorder& recorder = queryRecorder();
-    if (!recorder.startRecording(options, outputPath, false))
+    if (!recorder.startRecording(options, outputPath, nullptr, 0, 0, 0, false))
         return false;
     Owned<IEventVisitor> terminalVisitor = new CEventSavingVisitor(*this);
     try

dali/base/dacsds.cpp

@@ -2252,7 +2252,8 @@ unsigned CClientSDSManager::queryCount(const char *xpath)
 #define MIN_UPDTENV_SVER "3.9"
 bool CClientSDSManager::updateEnvironment(IPropertyTree *newEnv, bool forceGroupUpdate, StringBuffer &response)
 {
-    CDaliVersion serverVersionNeeded(MIN_QUERYCOUNT_SVER);
+    // NB: legacy, and for bare-metal only
+    CDaliVersion serverVersionNeeded(MIN_UPDTENV_SVER);
     if (queryDaliServerVersion().compare(serverVersionNeeded) < 0)
     {
         // have to do the old fashioned way, from client
@@ -2274,7 +2275,7 @@ bool CClientSDSManager::updateEnvironment(IPropertyTree *newEnv, bool forceGroup
             conn->commit();
             conn->close();
             StringBuffer messages;
-            initClusterGroups(forceGroupUpdate, messages, oldEnvironment);
+            initClusterAndStoragePlaneGroups(messages, forceGroupUpdate, oldEnvironment);
             if (messages.length())
                 PROGLOG("CClientSDSManager::updateEnvironment: %s", messages.str());
             PROGLOG("Environment and node groups updated");

dali/base/dadfs.cpp

@@ -10640,8 +10640,10 @@ class CInitGroups
         // see if identical
         const char *oldKind = oldClusterGroup->queryProp("@kind");
         const char *oldDir = oldClusterGroup->queryProp("@dir");
+        bool oldHasProtected = oldClusterGroup->hasProp("@protected");
         const char *newKind = newClusterGroup->queryProp("@kind");
         const char *newDir = newClusterGroup->queryProp("@dir");
+        bool newHasProtected = newClusterGroup->hasProp("@protected");
         if (oldKind)
         {
             if (newKind)
@@ -10666,7 +10668,15 @@ class CInitGroups
         }
         else if (NULL!=newDir)
             return false;
-
+        if (oldHasProtected != newHasProtected)
+            return false;
+        else if (oldHasProtected && newHasProtected)
+        {
+            bool oldProtected = oldClusterGroup->getPropBool("@protected");
+            bool newProtected = newClusterGroup->getPropBool("@protected");
+            if (oldProtected != newProtected)
+                return false;
+        }
         unsigned oldGroupCount = oldClusterGroup->getCount("Node");
         unsigned newGroupCount = newClusterGroup->getCount("Node");
         if (oldGroupCount != newGroupCount)
@@ -10912,6 +10922,7 @@ class CInitGroups
 
     bool constructGroup(const IPropertyTree &cluster, const char *altName, IPropertyTree *oldEnvCluster, GroupType groupType, bool force, StringBuffer &messages)
     {
+        dbgassertex(!isContainerized());
         /* a 'realCluster' is a cluster who's name matches it's nodeGroup
          * if the nodeGroup differs it implies it's sharing the nodeGroup with other thor instance(s).
          */
@@ -10955,6 +10966,10 @@ class CInitGroups
         IPropertyTree *existingClusterGroup = queryExistingGroup(gname);
         bool matchOldEnv = false;
         Owned<IPropertyTree> newClusterGroup = createClusterGroupFromEnvCluster(groupType, cluster, defDir, realCluster, true);
+        // All BM groups are protected by default, except dropzones.
+        // Meaning, they will not automatically be updated if the Environment cluster definition changes.
+        if (newClusterGroup && (grp_dropzone != groupType))
+            newClusterGroup->setPropBool("@protected", true);
         bool matchExisting = !force && clusterGroupCompare(newClusterGroup, existingClusterGroup);
         if (oldEnvCluster)
         {
@@ -10993,6 +11008,10 @@ class CInitGroups
             VStringBuffer msg("New cluster layout for cluster %s", gname.str());
             UWARNLOG("%s", msg.str());
             messages.append(msg).newline();
+            // All BM groups are protected by default, except dropzones.
+            // Meaning, they will not automatically be updated if the Environment cluster definition changes.
+            if (grp_dropzone != groupType)
+                newClusterGroup->setPropBool("@protected", true);
             addClusterGroup(gname.str(), newClusterGroup.getClear(), realCluster);
             return true;
         }
@@ -11019,6 +11038,7 @@ class CInitGroups
                     if (ins>1)
                         gname.append('_').append(ins);
                     Owned<IPropertyTree> clusterGroup = createClusterGroup(grp_hthor, { na }, nullptr, &cluster, true, false);
+                    clusterGroup->setPropBool("@protected", true);
                     addClusterGroup(gname.str(), clusterGroup.getClear(), true);
                 }
             }
@@ -11094,6 +11114,7 @@ class CInitGroups
     }
     bool resetClusterGroup(const char *clusterName, const char *type, bool spares, StringBuffer &messages)
     {
+        dbgassertex(!isContainerized());
         Owned<IRemoteConnection> conn = querySDS().connect("/Environment", myProcessSession(), RTM_LOCK_READ, SDS_CONNECT_TIMEOUT);
         if (!conn)
             return false;
@@ -11209,20 +11230,61 @@ class CInitGroups
         }
         return true;
     }
-    void clearLZGroups()
+    void clearUnprotectedGroups()
     {
         if (!writeLock)
-            throw makeStringException(0, "CInitGroups::clearLZGroups called in read-only mode");
-        IPropertyTree *root = groupsconnlock.conn->queryRoot();
-        std::vector<IPropertyTree *> toDelete;
-        Owned<IPropertyTreeIterator> groups = root->getElements("Group[@kind='dropzone']");
+            throw makeStringException(0, "CInitGroups::clearUnprotectedGroups called in read-only mode");
+
+        Owned<IPropertyTree> globalConfig = getGlobalConfig();
+        IPropertyTree * storage = globalConfig->queryPropTree("storage");
+        if (!storage)
+            return;
+        std::vector<IPropertyTree *> toRemove;
+        IPropertyTree *groupsRoot = groupsconnlock.conn->queryRoot();
+        Owned<IPropertyTreeIterator> groups = groupsRoot->getElements("Group");
+        bool firstBareMetalProtectedRun = true;
+        if (isContainerized())
+            firstBareMetalProtectedRun = false;
+        else
+        {
+            // check if any protected groups. Unless this is the 1st BM run since this feature was added,
+            // there will be >0 (one for each cluster group in the environment from previous runs).
+            ForEach(*groups)
+            {
+                IPropertyTree &group = groups->query();
+                if (group.hasProp("@protected"))
+                {
+                    firstBareMetalProtectedRun = false;
+                    break;
+                }
+            }
+        }
         ForEach(*groups)
-            toDelete.push_back(&groups->query());
-        for (auto &group: toDelete)
-            root->removeTree(group);
+        {
+            IPropertyTree &group = groups->query();
+            bool doDelete = false;
+            if (firstBareMetalProtectedRun)
+            {
+                doDelete = strsame("dropzone", group.queryProp("@kind"));
+                if (!doDelete)
+                {
+                    // Protect all non dropzone BM groups created on 1st run
+                    // This preserves legacy semantics, where Dali groups will not automatically be
+                    // overwritten by a change in the topology in the environment.
+                    group.setPropBool("@protected", true);
+                }
+            }
+            else
+                doDelete = !group.getPropBool("@protected");
+            if (doDelete)
+                toRemove.push_back(&group);
+        }
+        for (auto &group: toRemove)
+            groupsRoot->removeTree(group);
     }
     void constructGroups(bool force, StringBuffer &messages, IPropertyTree *oldEnvironment)
     {
+        dbgassertex(!isContainerized());
         Owned<IRemoteConnection> conn = querySDS().connect("/Environment/Software", myProcessSession(), RTM_LOCK_READ, SDS_CONNECT_TIMEOUT);
         if (!conn)
             return;
@@ -11301,11 +11363,13 @@ class CInitGroups
         return createClusterGroup(grp_unknown, hosts, path, nullptr, false, false);
     }
 
-    void ensureConsistentStorageGroup(bool force, const char * name, IPropertyTree * newClusterGroup, StringBuffer & messages)
+    void ensureConsistentStorageGroup(const char * name, IPropertyTree * newClusterGroup, StringBuffer & messages)
     {
         IPropertyTree *existingClusterGroup = queryExistingGroup(name);
         bool matchExisting = clusterGroupCompare(newClusterGroup, existingClusterGroup);
-        if (!existingClusterGroup || !matchExisting)
+        bool oldProtected = existingClusterGroup ? existingClusterGroup->getPropBool("@protected") : false;
+        bool newProtected = newClusterGroup->getPropBool("@protected");
+        if (!existingClusterGroup || !matchExisting || (oldProtected != newProtected))
         {
             if (!existingClusterGroup)
             {
@@ -11314,7 +11378,7 @@ class CInitGroups
                 messages.append(msg).newline();
                 addClusterGroup(name, LINK(newClusterGroup), false);
             }
-            else if (force)
+            else if (!oldProtected || !newProtected) // i.e. allow overwrite if either old wasn't protected, or if was, but new isn't
             {
                 VStringBuffer msg("Forcing new group layout for storageplane %s", name);
                 UWARNLOG("%s", msg.str());
@@ -11330,17 +11394,7 @@ class CInitGroups
         }
     }
 
-    void ensureStorageGroup(bool force, const char * name, unsigned numDevices, const char * path, StringBuffer & messages)
-    {
-        //Lower case the group name - see CNamedGroupStore::dolookup which lower cases before resolving.
-        StringBuffer gname;
-        gname.append(name).toLowerCase();
-
-        Owned<IPropertyTree> newClusterGroup = createStorageGroup(gname, numDevices, path);
-        ensureConsistentStorageGroup(force, gname, newClusterGroup, messages);
-    }
-
-    void constructStorageGroups(bool force, StringBuffer &messages)
+    void constructStorageGroups(StringBuffer &messages)
     {
         Owned<IPropertyTree> globalConfig = getGlobalConfig();
         IPropertyTree * storage = globalConfig->queryPropTree("storage");
@@ -11394,7 +11448,29 @@ class CInitGroups
                     unsigned numDevices = plane.getPropInt("@numDevices", 1);
                     newClusterGroup.setown(createStorageGroup(gname, numDevices, prefix));
                 }
-                ensureConsistentStorageGroup(force, gname, newClusterGroup, messages);
+                // Storage planes are the single source of truth for storage layout.
+                // Dali groups are created to reflect storage plane definitions.
+                // If a plane definition changes, the corresponding Dali group is overwritten
+                // (with a warning issued).
+                //
+                // Setting @protectGroup=true on an individual plane prevents a changed plane
+                // definition from overwriting the existing Dali group layout.
+                //
+                // Note: For bare-metal Environment-based groups, the existing group is protected
+                // by default. In containerized environments, the plane definition always takes
+                // precedence unless it is explicitly protected.
+                //
+                // The @protectGroup option should only be used if existing logical files reference
+                // an old plane layout and there is a need to prevent them from pointing to a new group
+                // layout (which could make physical file parts inaccessible). However, such plane
+                // topology changes should be avoided. Instead, a new plane should be defined, with the
+                // existing plane definition being left untouched, so that existing files that reference
+                // it are unaffected.
+                if (plane.getPropBool("@protectGroup"))
+                    newClusterGroup->setPropBool("@protected", true);
+                else
+                    newClusterGroup->removeProp("@protected");
+                ensureConsistentStorageGroup(gname, newClusterGroup, messages);
             }
         }
     }
@@ -11411,31 +11487,29 @@ class CInitGroups
     }
 };
 
-void initClusterGroups(bool force, StringBuffer &response, IPropertyTree *oldEnvironment, unsigned timems)
+void initClusterAndStoragePlaneGroups(StringBuffer &response, bool force, IPropertyTree *oldEnvironment, unsigned timems)
 {
     CInitGroups init(timems, true);
-    init.clearLZGroups(); // clear existing LZ groups, current ones will be recreated
-    init.constructGroups(force, response, oldEnvironment);
-}
-
-void initClusterAndStoragePlaneGroups(bool force, IPropertyTree *oldEnvironment, unsigned timems)
-{
-    CInitGroups init(timems, true);
-    init.clearLZGroups(); // clear existing LZ groups, current ones will be recreated
-
-    StringBuffer response;
-    init.constructGroups(force, response, oldEnvironment);
-    if (response.length())
-        MLOG("DFS group initialization : %s", response.str()); // should this be a syslog?
-
-    response.clear();
-    init.constructStorageGroups(false, response);
-    if (response.length())
-        MLOG("StoragePlane group initialization : %s", response.str()); // should this be a syslog?
+    // clearUnprotectedGroups clears all unprotected groups - BM hthor, thor, roxie Environment groups are
+    // protected to maintain existing semantics, and to protect logical files which reference them.
+    init.clearUnprotectedGroups();
+    if (!isContainerized())
+    {
+        // Create groups based on the Environment
+        // Detects mismatches between existing Dali groups and new Environment definitions
+        // and avoids replacing them unless forced. This is to avoid situations where existing
+        // logical files reference existing groups, changing their definition may render the
+        // logical file parts inaccessible.
+        // NB: these groups derived from Environment, are tagged with @kind thor, roxie, hthor, dropzone etc.
+        init.constructGroups(force, response, oldEnvironment);
+    }
+    // Create storage plane groups based on the global config storage/planes definitions
+    init.constructStorageGroups(response);
 }
 
 bool resetClusterGroup(const char *clusterName, const char *type, bool spares, StringBuffer &response, unsigned timems)
 {
+    dbgassertex(!isContainerized());
     CInitGroups init(timems, true);
     return init.resetClusterGroup(clusterName, type, spares, response);
 }

dali/base/dadfs.hpp

@@ -877,8 +877,7 @@ interface IDaliServer;
 extern da_decl IDaliServer *createDaliDFSServer(); // called for coven members
 
 // to initialize clustergroups after clusters change in the environment
-extern da_decl void initClusterGroups(bool force, StringBuffer &response, IPropertyTree *oldEnvironment, unsigned timems=INFINITE);
-extern da_decl void initClusterAndStoragePlaneGroups(bool force, IPropertyTree *oldEnvironment, unsigned timems=INFINITE);
+extern da_decl void initClusterAndStoragePlaneGroups(StringBuffer &response, bool force, IPropertyTree *oldEnvironment, unsigned timems=INFINITE);
 extern da_decl bool resetClusterGroup(const char *clusterName, const char *type, bool spares, StringBuffer &response, unsigned timems=INFINITE);
 extern da_decl bool addClusterSpares(const char *clusterName, const char *type, const std::vector<std::string> &hosts, StringBuffer &response, unsigned timems=INFINITE);
 extern da_decl bool removeClusterSpares(const char *clusterName, const char *type, const std::vector<std::string> &hosts, StringBuffer &response, unsigned timems=INFINITE);

dali/base/dasds.cpp

@@ -6184,13 +6184,15 @@ static CConfigUpdateHook configUpdateHook;
 static void initializeStorageGroups(IPropertyTree *oldEnvironment) // oldEnvironment always null in containerized
 {
     bool forceGroupUpdate = getComponentConfigSP()->getPropBool("dfs/@forceGroupUpdate");
-    initClusterAndStoragePlaneGroups(forceGroupUpdate, oldEnvironment);
+    StringBuffer response;
+    initClusterAndStoragePlaneGroups(response, forceGroupUpdate, oldEnvironment);
     if (isContainerized())
     {
         auto updateFunc = [](const IPropertyTree *oldComponentConfiguration, const IPropertyTree *oldGlobalConfiguration)
         {
             bool forceGroupUpdate = getComponentConfigSP()->getPropBool("dfs/@forceGroupUpdate");
-            initClusterAndStoragePlaneGroups(forceGroupUpdate, nullptr);
+            StringBuffer response;
+            initClusterAndStoragePlaneGroups(response, forceGroupUpdate, nullptr);
         };
         configUpdateHook.installOnce(updateFunc, false);
     }
@@ -8467,6 +8469,7 @@ void CCovenSDSManager::blockingSave(unsigned *writeTransactions)
 
 bool CCovenSDSManager::updateEnvironment(IPropertyTree *newEnv, bool forceGroupUpdate, StringBuffer &response)
 {
+    // for BM only
     Owned<IRemoteConnection> conn = querySDS().connect("/",myProcessSession(),0, INFINITE);
     if (conn)
     {
@@ -8482,7 +8485,7 @@ bool CCovenSDSManager::updateEnvironment(IPropertyTree *newEnv, bool forceGroupU
         conn->commit();
         conn->close();
         StringBuffer messages;
-        initClusterGroups(forceGroupUpdate, messages, oldEnvironment);
+        initClusterAndStoragePlaneGroups(messages, forceGroupUpdate, oldEnvironment);
         response.append(messages);
         PROGLOG("Environment and node groups updated");
     }