Merge pull request #7383 from afishbeck/improveUnusedFilesDelete

HPCC-12507 Cli roxie-unused-files --delete improvements

Reviewed-By: Kevin Wang <kevin.wang@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/eclcmd/eclcmd_common.hpp

@@ -89,6 +89,8 @@ typedef IEclCommand *(*EclCommandFactory)(const char *cmdname);
 #define ECLOPT_UPDATE_DFS "--update-dfs"
 #define ECLOPT_GLOBAL_SCOPE "--global-scope"
 #define ECLOPT_DELETE_FILES "--delete"
+#define ECLOPT_DELETE_SUBFILES "--delete-subfiles"
+#define ECLOPT_DELETE_RECURSIVE "--delete-recursive"
 
 #define ECLOPT_MAIN "--main"
 #define ECLOPT_MAIN_S "-main"  //eclcc compatible format

ecl/eclcmd/roxie/ecl-roxie.cpp

@@ -363,7 +363,7 @@ private:
 class EclCmdRoxieUnusedFiles : public EclCmdCommon
 {
 public:
-    EclCmdRoxieUnusedFiles() : optCheckPackageMaps(false)
+    EclCmdRoxieUnusedFiles() : optCheckPackageMaps(false), optDeleteFiles(false), optDeleteSubFiles(false), optDeleteRecursive(false)
     {
     }
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
@@ -384,6 +384,10 @@ public:
             }
             if (iter.matchFlag(optCheckPackageMaps, ECLOPT_CHECK_PACKAGEMAPS))
                 continue;
+            if (iter.matchFlag(optDeleteRecursive, ECLOPT_DELETE_RECURSIVE))
+                continue;
+            if (iter.matchFlag(optDeleteSubFiles, ECLOPT_DELETE_SUBFILES))
+                continue;
             if (iter.matchFlag(optDeleteFiles, ECLOPT_DELETE_FILES))
                 continue;
             if (EclCmdCommon::matchCommandLineOption(iter, true)!=EclCmdOptionMatch)
@@ -400,12 +404,16 @@ public:
             fputs("process cluster must be specified.\n\n", stderr);
             return false;
         }
+        if (optDeleteRecursive)
+            optDeleteSubFiles = true; //implied
+        if (optDeleteSubFiles)
+            optDeleteFiles = true; //implied
         return true;
     }
 
     virtual int processCMD()
     {
-        Owned<IClientWsDFUXRef> client = createCmdClient(WsDFUXRef, *this);
+        Owned<IClientWsDFUXRef> client = createCmdClientExt(WsDFUXRef, *this, "?ver_=1.29");
         Owned<IClientDFUXRefUnusedFilesRequest> req = client->createDFUXRefUnusedFilesRequest();
         req->setProcessCluster(optProcess);
         req->setCheckPackageMaps(optCheckPackageMaps);
@@ -437,18 +445,31 @@ public:
         {
             fputs("Deleting...\n", stderr);
 
-            Owned<IClientWsDfu> dfuClient = createCmdClient(WsDfu, *this);
+            Owned<IClientWsDfu> dfuClient = createCmdClientExt(WsDfu, *this, "?ver_=1.29");
             Owned<IClientDFUArrayActionRequest> dfuReq = dfuClient->createDFUArrayActionRequest();
             dfuReq->setType("Delete");
             dfuReq->setLogicalFiles(filesToDelete);
+            dfuReq->setRemoveFromSuperfiles(optDeleteSubFiles);
+            dfuReq->setRemoveRecursively(optDeleteRecursive);
 
             Owned<IClientDFUArrayActionResponse> dfuResp = dfuClient->DFUArrayAction(dfuReq);
             if (dfuResp->getExceptions().ordinality())
                 outputMultiExceptions(dfuResp->getExceptions());
 
             IArrayOf<IConstDFUActionInfo> &results = dfuResp->getActionResults();
-            ForEachItemIn(i, results)
-                fprintf(stdout, "  %s\n", results.item(i).getActionResult()); //result text already has filename
+            ForEachItemIn(i1, results) //list successes first
+            {
+                IConstDFUActionInfo &info = results.item(i1);
+                if (!info.getFailed())
+                    fprintf(stdout, "  %s\n", info.getActionResult()); //result text already has filename
+            }
+            fputs("\n", stdout);
+            ForEachItemIn(i2, results) //then errors
+            {
+                IConstDFUActionInfo &info = results.item(i2);
+                if (info.getFailed())
+                    fprintf(stdout, "  %s\n", info.getActionResult()); //result text already has filename
+            }
             fputs("\n", stdout);
         }
 
@@ -470,6 +491,10 @@ public:
         fputs("\n"
             "   --check-packagemaps    Exclude files referenced in active packagemaps\n"
             "   --delete               Delete unused files from DFS\n"
+            "   --delete-subfiles      Delete unused files from DFS and remove them from\n"
+            "                          superfiles.\n"
+            "   --delete-recursive     Delete unused files from DFS and remove them from\n"
+            "                          superfiles recursively.\n"
             " Common Options:\n",
             stdout);
         EclCmdCommon::usage();
@@ -478,6 +503,8 @@ private:
     StringAttr optProcess;
     bool optCheckPackageMaps;
     bool optDeleteFiles;
+    bool optDeleteSubFiles;
+    bool optDeleteRecursive;
 };
 
 IEclCommand *createEclRoxieCommand(const char *cmdname)

esp/scm/ws_dfu.ecm

@@ -237,6 +237,8 @@ DFUArrayActionRequest
     bool NoDelete;
     [min_ver("1.04")] string BackToPage;
     ESParray<string> LogicalFiles;
+    bool removeFromSuperfiles(false);
+    bool removeRecursively(false);
 };
 
 ESPresponse 

esp/services/ws_dfu/ws_dfuService.cpp

@@ -1134,7 +1134,7 @@ bool CWsDfuEx::onAddtoSuperfile(IEspContext &context, IEspAddtoSuperfileRequest
     return true;
 }
 
-void CWsDfuEx::setDeleteFileResults(const char* fileName, const char* nodeGroup, bool failed, const char* actionResult, StringBuffer& resultString,
+void setDeleteFileResults(const char* fileName, const char* nodeGroup, bool failed, const char *start, const char* text, StringBuffer& resultString,
     IArrayOf<IEspDFUActionInfo>& actionResults)
 {
     if (!fileName || !*fileName)
@@ -1144,121 +1144,182 @@ void CWsDfuEx::setDeleteFileResults(const char* fileName, const char* nodeGroup,
     resultObj->setFailed(failed);
     if (nodeGroup && *nodeGroup)
         resultObj->setNodeGroup(nodeGroup);
-    if (actionResult && *actionResult)
-    {
-        resultObj->setActionResult(actionResult);
-        resultString.appendf("<Message><Value>%s</Value></Message>", actionResult);
-    }
+
+    StringBuffer message;
+    if (start)
+        message.append(start).append(' ');
+    message.append(fileName);
+    if (nodeGroup && *nodeGroup)
+        message.append(" on ").append(nodeGroup);
+    if (text && *text)
+        message.append(failed ? ": " : " ").append(text);
+    resultObj->setActionResult(message);
+    resultString.appendf("<Message><Value>%s</Value></Message>", message.str());
+
     actionResults.append(*resultObj.getClear());
-    return;
 }
 
-bool CWsDfuEx::DFUDeleteFiles(IEspContext &context, IEspDFUArrayActionRequest &req, IEspDFUArrayActionResponse &resp)
-{
-    double version = context.getClientVersion();
-    StringBuffer username;
-    context.getUserID(username);
+typedef enum {
+    DeleteActionSuccess,
+    DeleteActionFailure,
+    DeleteActionSkip
+} DeleteActionResult;
 
-    Owned<IUserDescriptor> userdesc;
-    if(username.length() > 0)
-    {
-        const char* passwd = context.queryPassword();
-        userdesc.setown(createUserDescriptor());
-        userdesc->set(username.str(), passwd);
-    }
-    StringBuffer returnStr;
 
-    StringArray superFileNames, filesCannotBeDeleted;
-    IArrayOf<IEspDFUActionInfo> actionResults;
-    for(int j = 0; j < 2; j++) //j=0: delete superfiles first
+DeleteActionResult doDeleteFile(const char *fn, IUserDescriptor *userdesc, StringArray &superFiles, StringArray &failedFiles, StringBuffer& returnStr, IArrayOf<IEspDFUActionInfo>& actionResults, bool superFilesOnly, bool removeFromSuperfiles, bool deleteRecursively);
+
+bool doRemoveFileFromSuperfiles(const char *lfn, IUserDescriptor *userdesc, StringArray &superFiles, StringArray &failedFiles, bool deleteRecursively, StringBuffer& returnStr, IArrayOf<IEspDFUActionInfo>& actionResults)
+{
+    StringArray emptySuperFiles;
+    IDistributedFileDirectory &fdir = queryDistributedFileDirectory();
     {
-        for(unsigned i = 0; i < req.getLogicalFiles().length(); i++)
+        Owned<IDistributedFile> df = fdir.lookup(lfn, userdesc, true);
+        if(!df)
+            return false;
+        Owned<IDistributedSuperFileIterator> supers = df->getOwningSuperFiles();
+        ForEach(*supers)
         {
-            const char* fileNameAndNodeGroup = req.getLogicalFiles().item(i);
-            if(!fileNameAndNodeGroup || !*fileNameAndNodeGroup)
-                continue;
-
-            const char* fileName = NULL;
-            const char* nodeGroup = NULL;
-            StringArray fileNameOrNodeGroup;
-            fileNameOrNodeGroup.appendListUniq(fileNameAndNodeGroup, "@");
-            fileName = fileNameOrNodeGroup.item(0);
-            if (fileNameOrNodeGroup.length() > 1)
-            {
-                nodeGroup = fileNameOrNodeGroup.item(1);
-                if (!*nodeGroup || strieq(nodeGroup, "null")) //null is used by new ECLWatch for a superfile
-                    nodeGroup = NULL;
-            }
-            if (j>0)
-            { // 2nd pass, now we want to skip superfiles and the files which cannot do the lookup.
-                if (superFileNames.contains(fileNameAndNodeGroup) || filesCannotBeDeleted.contains(fileNameAndNodeGroup))
-                    continue;
-            }
-
+            IDistributedSuperFile &super = supers->query();
             try
             {
-                IDistributedFileDirectory &fdir = queryDistributedFileDirectory();
-                {
-                    Owned<IDistributedFile> df = fdir.lookup(fileNameAndNodeGroup, userdesc, true);
-                    if(!df)
-                    {
-                        StringBuffer message;
-                        if (!nodeGroup || !*nodeGroup)
-                            message.appendf("Cannot delete %s: file not found", fileName);
-                        else
-                            message.appendf("Cannot delete %s on %s: file not found", fileName, nodeGroup);
-                        PROGLOG("CWsDfuEx::DFUDeleteFiles: %s", message.str());
-                        setDeleteFileResults(fileName, nodeGroup, true, message, returnStr, actionResults);
-                        filesCannotBeDeleted.append(fileNameAndNodeGroup);
-                        continue;
-                    }
-                    if (0==j) // skip non-super files on 1st pass
-                    {
-                        if(!df->querySuperFile())
-                            continue;
-
-                        superFileNames.append(fileNameAndNodeGroup);
-                    }
-                }
-                fdir.removeEntry(fileNameAndNodeGroup, userdesc, NULL, REMOVE_FILE_SDS_CONNECT_TIMEOUT, true);
-                StringBuffer message;
-                if (!nodeGroup || !*nodeGroup)
-                    message.appendf("File %s deleted", fileName);
-                else
-                    message.appendf("File %s deleted on %s", fileName, nodeGroup);
-                setDeleteFileResults(fileName, nodeGroup, false, message, returnStr, actionResults);
+                super.removeSubFile(lfn, false, false, NULL);
+                VStringBuffer text("from superfile %s", super.queryLogicalName());
+                setDeleteFileResults(lfn, NULL, false, "Removed subfile", text, returnStr, actionResults);
             }
             catch(IException* e)
             {
-                filesCannotBeDeleted.append(fileNameAndNodeGroup);
-
                 StringBuffer emsg;
-                e->errorMessage(emsg);
-                if((e->errorCode() == DFSERR_CreateAccessDenied) && (req.getType() != NULL))
-                    emsg.replaceString("Create ", "Delete ");
-
-                StringBuffer message;
-                if (!nodeGroup || !*nodeGroup)
-                    message.appendf("Cannot delete %s: %s", fileName, emsg.str());
-                else
-                    message.appendf("Cannot delete %s on %s: %s", fileName, nodeGroup, emsg.str());
-                setDeleteFileResults(fileName, nodeGroup, true, message, returnStr, actionResults);
+                VStringBuffer text("from superfile %s: %s", super.queryLogicalName(), e->errorMessage(emsg).str());
+                setDeleteFileResults(lfn, NULL, true, "Could not remove subfile ", text, returnStr, actionResults);
                 e->Release();
+                return false;
             }
             catch(...)
             {
-                StringBuffer message;
-                if (!nodeGroup || !*nodeGroup)
-                    message.appendf("Cannot delete %s: unknown exception.", fileName);
-                else
-                    message.appendf("Cannot delete %s on %s: unknown exception.", fileName, nodeGroup);
-                setDeleteFileResults(fileName, nodeGroup, true, message, returnStr, actionResults);
+                VStringBuffer text("from superfile %s", super.queryLogicalName());
+                setDeleteFileResults(lfn, NULL, true, "Could not remove subfile ", text, returnStr, actionResults);
+                return false;
+            }
+            if (deleteRecursively && super.numSubFiles(false)==0)
+                emptySuperFiles.appendUniq(super.queryLogicalName());
+        }
+    }
+    ForEachItemIn(i, emptySuperFiles)
+        doDeleteFile(emptySuperFiles.item(i), userdesc, superFiles, failedFiles, returnStr, actionResults, false, true, deleteRecursively);
+
+    return true;
+}
+
+DeleteActionResult doDeleteFile(const char *fn, IUserDescriptor *userdesc, StringArray &superFiles, StringArray &failedFiles, StringBuffer& returnStr, IArrayOf<IEspDFUActionInfo>& actionResults,
+        bool superFilesOnly, bool removeFromSuperfiles, bool deleteRecursively)
+{
+    StringArray parsed;
+    parsed.appendListUniq(fn, "@");
+    const char *lfn = parsed.item(0);
+    const char *group = NULL;
+    if (parsed.length() > 1)
+    {
+        group = parsed.item(1);
+        if (group && (!*group || strieq(group, "null"))) //null is used by new ECLWatch for a superfile
+            group = NULL;
+    }
+
+    bool isSuper = false;
+    if (superFiles.contains(fn) || failedFiles.contains(fn))
+        return DeleteActionSkip;
+    try
+    {
+        IDistributedFileDirectory &fdir = queryDistributedFileDirectory();
+        {
+            Owned<IDistributedFile> df = fdir.lookup(lfn, userdesc, true);
+            if(!df)
+            {
+                PROGLOG("CWsDfuEx::DFUDeleteFiles: %s not found", lfn);
+                setDeleteFileResults(lfn, group, true, "File not found", NULL, returnStr, actionResults);
+                return DeleteActionFailure;
+            }
+            isSuper = df->querySuperFile()!=NULL;
+            if (superFilesOnly) // skip non-super files on 1st pass
+            {
+                if(!isSuper)
+                    return DeleteActionSkip;
+                superFiles.append(fn);
             }
         }
+        fdir.removeEntry(fn, userdesc, NULL, REMOVE_FILE_SDS_CONNECT_TIMEOUT, true);
+        setDeleteFileResults(lfn, group, false, isSuper ? "Deleted Superfile" : "Deleted File", NULL, returnStr, actionResults);
+    }
+    catch(IException* e)
+    {
+        StringBuffer emsg;
+        e->errorMessage(emsg);
+        if (removeFromSuperfiles && strstr(emsg, "owned by"))
+        {
+            if (!doRemoveFileFromSuperfiles(lfn, userdesc, superFiles, failedFiles, deleteRecursively, returnStr, actionResults))
+                return DeleteActionFailure;
+            return doDeleteFile(fn, userdesc, superFiles, failedFiles, returnStr, actionResults, superFilesOnly, false, false);
+        }
+        if (e->errorCode() == DFSERR_CreateAccessDenied)
+            emsg.replaceString("Create ", "Delete ");
+
+        setDeleteFileResults(lfn, group, true, "Could not delete", emsg.str(), returnStr, actionResults);
+        e->Release();
+        return DeleteActionFailure;
+    }
+    catch(...)
+    {
+        setDeleteFileResults(lfn, group, true, "Could not delete", "unknown exception", returnStr, actionResults);
+        return DeleteActionFailure;
     }
+    return DeleteActionSuccess;
+}
+
+void doDeleteFiles(StringArray &files, IUserDescriptor *userdesc, StringArray &superFiles, StringArray &failedFiles, StringBuffer &returnStr, IArrayOf<IEspDFUActionInfo> &actionResults,
+        bool superFilesOnly, bool removeFromSuperfiles, bool deleteRecursively)
+{
+    ForEachItemIn(i, files)
+    {
+        const char* fn = files.item(i);
+        if(!fn || !*fn)
+            continue;
+
+        DeleteActionResult ar;
+        if (DeleteActionFailure==doDeleteFile(fn, userdesc, superFiles, failedFiles, returnStr, actionResults, superFilesOnly, removeFromSuperfiles, deleteRecursively))
+            failedFiles.appendUniq(fn);
+    }
+
+}
+
+inline void doDeleteSuperFiles(StringArray &files, IUserDescriptor *userdesc, StringArray &superFiles, StringArray &failedFiles, StringBuffer &returnStr, IArrayOf<IEspDFUActionInfo> &actionResults,
+        bool removeFromSuperfiles, bool deleteRecursively)
+{
+    doDeleteFiles(files, userdesc, superFiles, failedFiles, returnStr, actionResults, true, removeFromSuperfiles, deleteRecursively);
+}
+
+inline void doDeleteSubFiles(StringArray &files, IUserDescriptor *userdesc, StringArray &superFiles, StringArray &failedFiles, StringBuffer &returnStr, IArrayOf<IEspDFUActionInfo> &actionResults,
+        bool removeFromSuperfiles, bool deleteRecursively)
+{
+    doDeleteFiles(files, userdesc, superFiles, failedFiles, returnStr, actionResults, false, removeFromSuperfiles, deleteRecursively);
+}
+
+bool CWsDfuEx::DFUDeleteFiles(IEspContext &context, IEspDFUArrayActionRequest &req, IEspDFUArrayActionResponse &resp)
+{
+    Owned<IUserDescriptor> userdesc;
+    const char *username = context.queryUserId();
+    if(username && *username)
+    {
+        userdesc.setown(createUserDescriptor());
+        userdesc->set(username, context.queryPassword());
+    }
+
+    StringBuffer returnStr;
+    IArrayOf<IEspDFUActionInfo> actionResults;
+
+    StringArray superFiles, failedFiles;
+    doDeleteSuperFiles(req.getLogicalFiles(), userdesc, superFiles, failedFiles, returnStr, actionResults, req.getRemoveFromSuperfiles(), req.getRemoveRecursively());
+    doDeleteSubFiles(req.getLogicalFiles(), userdesc, superFiles, failedFiles, returnStr, actionResults, req.getRemoveFromSuperfiles(), req.getRemoveRecursively());
 
-    if (version >= 1.27)
-        resp.setActionResults(actionResults);
+    resp.setActionResults(actionResults);
     resp.setDFUArrayActionResult(returnStr.str());//Used by legacy
     return true;
 }

esp/services/ws_dfu/ws_dfuService.hpp

@@ -135,8 +135,6 @@ private:
         const char* beforeSubFile, bool existingSuperfile, bool autocreatesuper, bool deleteFile, bool removeSuperfile =  true);
     void getFilePartsOnClusters(IEspContext &context, const char* clusterReq, StringArray& clusters, IDistributedFile* df, IEspDFUFileDetail& FileDetails,
         offset_t& mn, offset_t& mx, offset_t& sum, offset_t& count);
-    void setDeleteFileResults(const char* fileName, const char* nodeGroup, bool failed, const char* message, StringBuffer& resultString,
-        IArrayOf<IEspDFUActionInfo>& actionResults);
 private:
     bool         m_disableUppercaseTranslation;
     StringBuffer m_clusterName;

initfiles/etc/bash_completion/ecl

@@ -112,12 +112,16 @@ _ecl_opts_roxie()
     local subcmdname="${COMP_WORDS[subcmdpos]}"
     case "${subcmdname}" in
         "" | ecl)
-            echo "--help attach detach check reload"
+            echo "--help attach detach check reload unused-files"
             ;;
         attach | detach | check | reload)
             echo -n "--wait= "
             _ecl_opts_common
             ;;
+        "unused-files")
+            echo -n "--delete-recursive --delete-subfiles --delete --check-packagemaps --wait="
+            _ecl_opts_common
+            ;;
          *)
             ;;
     esac