From 19dd2f0e073c2fa07b226e4b0b257991a52b415d Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 25 Apr 2024 08:55:20 -0400 Subject: [PATCH 01/32] HPCC-30365 Add XREF Sasha service to K8s --- dali/base/dadfs.cpp | 4 ++ dali/dfuXRefLib/dfuxreflib.cpp | 58 +++++++++++++++---- dali/sasha/saserver.cpp | 4 +- esp/services/ws_dfu/ws_dfuXRefService.cpp | 20 ++++++- esp/src/src-react/components/Xrefs.tsx | 22 ++++--- .../vault-pki-remote/values-hpcc1.yaml | 2 + helm/hpcc/templates/_helpers.tpl | 4 +- helm/hpcc/values.schema.json | 11 ++++ helm/hpcc/values.yaml | 3 +- 9 files changed, 104 insertions(+), 24 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index 0160d4a7371..4e4545843b4 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -261,6 +261,10 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partm StringBuffer fullname; if (findPathSepChar(name)==NULL) addPathSepChar(fullname.append(partdir)); +#ifdef _CONTAINERIZED + if (partmax>1) + addPathSepChar(fullname.append(partno+1)); // If there are multiple files they will be striped by part number +#endif fullname.append(name); unsigned n; unsigned d; diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index e858a0cf496..d67d99c320a 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -32,6 +32,7 @@ #include "rmtfile.hpp" #include "dautils.hpp" #include "jptree.hpp" +#include "jcontainerized.hpp" #include "XRefNodeManager.hpp" @@ -942,6 +943,17 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns s++; } if ((mn!=0)&&((*s==0)||(*s=='.'))&&(mn>=pn)) { // NB allow trailing extension +#ifdef _CONTAINERIZED + if (mn>1) + { + StringBuffer stripeDir; + stripeDir.append(getPathSepChar(mname)).append(pn).append(getPathSepChar(mname)); + StringBuffer stripeMask; + stripeMask.append(getPathSepChar(mname)).append("$P$").append(getPathSepChar(mname)); + // Replace part number in stripe directory with mask + mname.replaceString(stripeDir, stripeMask); + } +#endif mname.append("._$P$_of_").append(mn); if (*s) mname.append(s); @@ -1867,7 +1879,11 @@ class CXRefManager: public CXRefManagerBase Owned results; { CriticalUnblock unblock(manager.logsect); +#ifdef _CONTAINERIZED + unsigned short port = 0; // No need to connect to Dafs, data is available through mounted storage planes +#else unsigned short port = getDafsPort(node.endpoint(),numfails,&crit); +#endif results.setown(getDirectory(dirlist,&node,port)); } manager.log("Crossreferencing %s",msg.str()); @@ -2600,7 +2616,11 @@ class CXRefManager: public CXRefManagerBase Owned g; unsigned j; if (!nclusters) { +#ifdef _CONTAINERIZED + error("XREF","No storage planes specified\n"); +#else error("XREF","No clusters specified\n"); +#endif return NULL; } if (!numdirs) { @@ -2618,18 +2638,29 @@ class CXRefManager: public CXRefManagerBase else g.setown(g->combine(gsub.get())); } - totalSizeOrphans =0; + totalSizeOrphans = 0; totalNumOrphans = 0; - logicalnamelist.kill(); dirlist.kill(); orphanlist.kill(); - + +#ifdef _CONTAINERIZED + const char *storageDir[1]; + for (int i = 0; i < nclusters; i++) + { + const char *storagePlane = clusters[i]; // clusters holds a list of storage plane names + storageDir[0] = dirbaselist[i]; // dirbaselist holds the storage plane directories + + loadFromDFS(*this,g,1,storageDir,storagePlane); + xrefRemoteDirectories(g,numdirs,storageDir,numthreads); + } +#else const char* cluster = clusters[0]; loadFromDFS(*this,g,numdirs,dirbaselist,cluster); xrefRemoteDirectories(g,numdirs,dirbaselist,numthreads); +#endif StringBuffer filename; filename.clear().append("xrefrpt"); addFileTimestamp(filename, true); @@ -2669,24 +2700,31 @@ IPropertyTree * runXRef(unsigned nclusters,const char **clusters,IXRefProgressC if (nclusters==0) return NULL; CXRefManager xrefmanager; - const char *dirs[2]; - unsigned numdirs; #ifdef _WIN32 bool islinux = false; #else bool islinux = true; #endif - // assume all nodes same OS - Owned group = queryNamedGroupStore().lookup(clusters[0]); #ifdef _CONTAINERIZED - WARNLOG("CONTAINERIZED(runXRef calls queryOS())"); + DBGLOG("CONTAINERIZED(runXRef)"); + const char *dirs[nclusters]; // nclusters is the number of storage planes + unsigned numdirs = nclusters; + + for (int i = 0; i < numdirs; i++) + { + Owned storagePlane = getStoragePlane(clusters[i]); + dirs[i] = storagePlane->queryProp("@prefix"); + } #else + const char *dirs[2]; + unsigned numdirs = 2; + // assume all nodes same OS + Owned group = queryNamedGroupStore().lookup(clusters[0]); if (group) islinux = queryOS(group->queryNode(0).endpoint())==MachineOsLinux; -#endif dirs[0] = queryBaseDirectory(grp_unknown, 0,islinux?DFD_OSunix:DFD_OSwindows); // MORE - should use the info from the group store dirs[1] = queryBaseDirectory(grp_unknown, 1,islinux?DFD_OSunix:DFD_OSwindows); - numdirs = 2; +#endif IPropertyTree *ret=NULL; try { ret = xrefmanager.process(nclusters,clusters,numdirs,dirs,PMtreeoutput,callback,numthreads); diff --git a/dali/sasha/saserver.cpp b/dali/sasha/saserver.cpp index 16f808542cd..5a426fb1633 100644 --- a/dali/sasha/saserver.cpp +++ b/dali/sasha/saserver.cpp @@ -435,8 +435,8 @@ int main(int argc, const char* argv[]) servers.append(*createSashaFileExpiryServer()); else if (strieq(service, "thor-qmon")) servers.append(*createSashaQMonitorServer()); - //else if (strieq(service, "xref")) // TODO - // servers.append(*createSashaXrefServer()); + else if (strieq(service, "xref")) + servers.append(*createSashaXrefServer()); else throw makeStringExceptionV(0, "Unrecognised 'service': %s", service); #else diff --git a/esp/services/ws_dfu/ws_dfuXRefService.cpp b/esp/services/ws_dfu/ws_dfuXRefService.cpp index 759df1a7144..55abb19344e 100644 --- a/esp/services/ws_dfu/ws_dfuXRefService.cpp +++ b/esp/services/ws_dfu/ws_dfuXRefService.cpp @@ -603,11 +603,25 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r { try { -#ifdef _CONTAINERIZED - IERRLOG("CONTAINERIZED(CWsDfuXRefEx::onDFUXRefList)"); -#else context.ensureFeatureAccess(XREF_FEATURE_URL, SecAccess_Read, ECLWATCH_DFU_XREF_ACCESS_DENIED, "WsDfuXRef::DFUXRefList: Permission denied."); +#ifdef _CONTAINERIZED + DBGLOG("CONTAINERIZED(CWsDfuXRefEx::onDFUXRefList)"); + Owned planesIter = getPlanesIterator("data", nullptr); + + BoolHash uniquePlanes; + Owned xrefNodeTree = createPTree("XRefNodes"); + + ForEach(*planesIter) + { + IPropertyTree &item = planesIter->query(); + + addUniqueXRefNode(item.queryProp("@name"), uniquePlanes, xrefNodeTree); + } + + StringBuffer buf; + resp.setDFUXRefListResult(formatResult(context, xrefNodeTree, buf)); +#else CConstWUClusterInfoArray clusters; getEnvironmentClusterInfo(clusters); diff --git a/esp/src/src-react/components/Xrefs.tsx b/esp/src/src-react/components/Xrefs.tsx index 39f37e1a09b..56e7273807b 100644 --- a/esp/src/src-react/components/Xrefs.tsx +++ b/esp/src/src-react/components/Xrefs.tsx @@ -66,13 +66,21 @@ export const Xrefs: React.FunctionComponent = ({ .then(({ DFUXRefListResponse }) => { const xrefNodes = DFUXRefListResponse?.DFUXRefListResult?.XRefNode; if (xrefNodes) { - setData(xrefNodes.map((item, idx) => { - return { - name: item.Name, - modified: item.Modified, - status: item.Status - }; - })); + if (Array.isArray(xrefNodes)) { + setData(xrefNodes.map((item, idx) => { + return { + name: item.Name, + modified: item.Modified, + status: item.Status + }; + })); + } else { + setData([{ + name: xrefNodes.Name, + modified: xrefNodes.Modified, + status: xrefNodes.Status + }]); + } } }) .catch(err => logger.error(err)) diff --git a/helm/examples/vault-pki-remote/values-hpcc1.yaml b/helm/examples/vault-pki-remote/values-hpcc1.yaml index a21e9ccc765..97e58232591 100644 --- a/helm/examples/vault-pki-remote/values-hpcc1.yaml +++ b/helm/examples/vault-pki-remote/values-hpcc1.yaml @@ -52,6 +52,8 @@ sasha: disabled: true file-expiry: disabled: true + xref: + disabled: true esp: - name: eclwatch diff --git a/helm/hpcc/templates/_helpers.tpl b/helm/hpcc/templates/_helpers.tpl index e383472a70f..1f180d1173e 100644 --- a/helm/hpcc/templates/_helpers.tpl +++ b/helm/hpcc/templates/_helpers.tpl @@ -1687,7 +1687,7 @@ prometheusMetricsReporter: "yes" {{- end -}} {{/* -Return access permssions for a given service +Return access permissions for a given service */}} {{- define "hpcc.getSashaServiceAccess" }} {{- if (eq "coalescer" .name) -}} @@ -1702,6 +1702,8 @@ dali dali data {{- else if (eq "thor-qmon" .name) -}} dali queues +{{- else if (eq "xref" .name) -}} +dali dalidata {{- else -}} {{- $_ := fail (printf "Unknown sasha service:" .name ) -}} {{- end -}} diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json index 321ea56c09e..4abce8eb2cf 100644 --- a/helm/hpcc/values.schema.json +++ b/helm/hpcc/values.schema.json @@ -2994,6 +2994,14 @@ }, "additionalProperties": false }, + "sasha-xref": { + "type": "object", + "allOf": [{ "$ref": "#/definitions/sashacommon" }], + "properties": { + "disabled": {} + }, + "additionalProperties": false + }, "sashaservice": { "oneOf": [ { @@ -3018,6 +3026,9 @@ "thor-qmon": { "$ref": "#/definitions/sasha-thor-qmon" }, + "xref": { + "$ref": "#/definitions/sasha-xref" + }, "disabled": { "type": "boolean" } diff --git a/helm/hpcc/values.yaml b/helm/hpcc/values.yaml index b6a6d05ccba..6424ce9957c 100644 --- a/helm/hpcc/values.yaml +++ b/helm/hpcc/values.yaml @@ -508,7 +508,8 @@ sasha: #disabled: true #switchMinTime: 1 #queues: "*" - + + xref: {} # NB: no properties defined, use defaults dfuserver: - name: dfuserver From 98e89209d3787abe0c86c00e877b402eaaf43b1e Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Mon, 8 Jul 2024 10:21:09 -0400 Subject: [PATCH 02/32] Add constructPartFilename to CLogicalNameEntry --- dali/dfuXRefLib/dfuxreflib.cpp | 64 +++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index d67d99c320a..6859b50d4b0 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -645,17 +645,19 @@ struct CLogicalNameEntry: public CInterface grouped = file.getPropInt("Attr/@grouped", 0)!=0; const char *partmask = file.queryProp("@partmask"); StringBuffer tmp; + partDir = file.queryProp("@directory"); if (partmask&&*partmask) { if (!containsPathSepChar(partmask)) { - const char *dir = file.queryProp("@directory"); - if (dir&&*dir) - tmp.append(dir).append(getPathSepChar(dir)); + if (!partDir.isEmpty()) + tmp.append(partDir).append(getPathSepChar(partDir)); } tmp.append(partmask); } tmp.toLowerCase(); substnum(tmp,"$n$",max); dirpartmask.set(tmp.str()); + replicateOffset = file.getPropInt("@replicateOffset",1); + dirPerPart = max>1?getDataStoragePlane(grpname, true)->queryDirPerPart():false; } ~CLogicalNameEntry() { @@ -758,6 +760,36 @@ struct CLogicalNameEntry: public CInterface return false; } + RemoteFilename &constructPartFilename(unsigned partNo, unsigned copy, RemoteFilename &rfn) + { + partNo--; + StringBuffer partName; + if (pmask.isEmpty()) + { + pmask.set("!ERROR!._$P$_of_$N$"); + IERRLOG("No partmask for CLogicalNameEntry::constructPartFilename"); + } + expandMask(partName, pmask, partNo, max); + + StringBuffer fullname; + addPathSepChar(fullname.append(partDir)); + if (dirPerPart) + addPathSepChar(fullname.append(partNo+1)); + fullname.append(partName); + + ClusterPartDiskMapSpec mspec; + mspec.replicateOffset = replicateOffset; + unsigned n; + unsigned d; + mspec.calcPartLocation(partNo, max, copy, grp?grp->ordinality():max, n, d); + setReplicateFilename(fullname, d); + SocketEndpoint ep; + if (grp) + ep = grp->queryNode(n).endpoint(); + rfn.setPath(ep, fullname.toLowerCase().str()); + return rfn; + } + void resolve(CFileEntry *entry); IPropertyTree *addFileBranch(IPropertyTree *dst,unsigned flags); @@ -794,6 +826,9 @@ struct CLogicalNameEntry: public CInterface bool grouped; StringAttr dirpartmask; CXRefManagerBase &manager; + StringAttr partDir; + int replicateOffset; + bool dirPerPart; }; @@ -1529,18 +1564,14 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch } else { bool replicate=false; - const char *partname = part.queryProp("@name"); - const char *partmask = file.queryProp("@partmask"); - const char *partdir = file.queryProp("@directory"); - int replicateoffset = file.getPropInt("@replicateOffset",1); for (;;) { - RemoteFilename rfn; + RemoteFilename rfn; IGroup *grp = lnentry->queryGroup(); if (!grp) { manager.warn(lnentry->lname.get(),"No group found, ignoring logical file"); return; } - constructPartFilename(grp,partno,numparts,partname,partmask,partdir,replicate,replicateoffset,rfn); + lnentry->constructPartFilename(partno, replicate, rfn); SocketEndpoint rep=rfn.queryEndpoint(); if (manager.EndpointTable.find(rep)!=NULL) { rfn.getLocalPath(localname.clear()); @@ -1556,9 +1587,9 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch if (dirmatch) { rfn.getRemotePath(remotename.clear()); remotename.toLowerCase(); - + CFileEntry *entry= new CFileEntry(remotename.str(),lnentry,partno,replicate,part.getPropInt64("@size", -1),part.getPropInt("@rowCompression", 0)!=0,part.getPropInt("@compressedSize", -1)); - + CFileEntry *oldentry= manager.filemap.find(remotename.str()); if (oldentry) { @@ -1581,7 +1612,6 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch else { manager.filemap.add(*entry); } - } else { lnentry->outsidedir++; @@ -1596,30 +1626,24 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch if (replicate) break; replicate = true; - } + } } } } scanner(manager,grp,numdirs,dirbaselist); - + manager.log("Loading Files branch from SDS"); Owned conn = querySDS().connect(SDS_DFS_ROOT,myProcessSession(),RTM_LOCK_READ, INFINITE); if (!conn) { throw MakeStringException(-1,"Could not connect to Files"); - } conn->changeMode(RTM_NONE); manager.log("Files loaded, scanning"); scanner.scan(conn); manager.log("Scanning done"); - } - - - - class CPhysicalXREF { unsigned numdirs; From 8264482245d8d0064cb1e9ef68f5a97b542b1b1a Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Mon, 8 Jul 2024 13:44:22 -0400 Subject: [PATCH 03/32] Check if running on localhost before setting Dafs port to 0. --- dali/dfuXRefLib/dfuxreflib.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 6859b50d4b0..e49bace082f 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -1903,11 +1903,9 @@ class CXRefManager: public CXRefManagerBase Owned results; { CriticalUnblock unblock(manager.logsect); -#ifdef _CONTAINERIZED - unsigned short port = 0; // No need to connect to Dafs, data is available through mounted storage planes -#else - unsigned short port = getDafsPort(node.endpoint(),numfails,&crit); -#endif + unsigned short port = 0; + if (!strieq(msg, "localhost")) + port = getDafsPort(node.endpoint(),numfails,&crit); results.setown(getDirectory(dirlist,&node,port)); } manager.log("Crossreferencing %s",msg.str()); From a5171b1e7a6deeb537abaf4232ce84fa4bea6927 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Mon, 8 Jul 2024 13:46:11 -0400 Subject: [PATCH 04/32] Change error message to report missing storage planes rather than clusters. --- dali/dfuXRefLib/dfuxreflib.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index e49bace082f..6fe096c3e23 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -2638,11 +2638,7 @@ class CXRefManager: public CXRefManagerBase Owned g; unsigned j; if (!nclusters) { -#ifdef _CONTAINERIZED error("XREF","No storage planes specified\n"); -#else - error("XREF","No clusters specified\n"); -#endif return NULL; } if (!numdirs) { From 6c650528c2f49849e2b92b2b490719e7ad7ccb88 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 9 Jul 2024 09:59:06 -0400 Subject: [PATCH 05/32] Move group combination to BM only. Only store a single storage plane in dirs. --- dali/dfuXRefLib/dfuxreflib.cpp | 114 ++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 52 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 6fe096c3e23..0ca6d61406c 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -2634,28 +2634,18 @@ class CXRefManager: public CXRefManagerBase msgcallback.set(_msgcallback); IPropertyTree *out=NULL; - - Owned g; - unsigned j; - if (!nclusters) { + + if (!nclusters) + { error("XREF","No storage planes specified\n"); return NULL; } - if (!numdirs) { + if (!numdirs) + { error("XREF","No directories specified\n"); return NULL; } - for (j=0;j gsub = queryNamedGroupStore().lookup(clusters[j]); - if (!gsub) { - error(clusters[j],"Could not find cluster group"); - return NULL; - } - if (!g) - g.set(gsub.get()); - else - g.setown(g->combine(gsub.get())); - } + totalSizeOrphans = 0; totalNumOrphans = 0; @@ -2663,41 +2653,62 @@ class CXRefManager: public CXRefManagerBase dirlist.kill(); orphanlist.kill(); -#ifdef _CONTAINERIZED - const char *storageDir[1]; - for (int i = 0; i < nclusters; i++) + if (!isContainerized()) { - const char *storagePlane = clusters[i]; // clusters holds a list of storage plane names - storageDir[0] = dirbaselist[i]; // dirbaselist holds the storage plane directories + Owned g; + unsigned j; + + for (j=0;j gsub = queryNamedGroupStore().lookup(clusters[j]); + if (!gsub) + { + error(clusters[j], "Could not find cluster group"); + return NULL; + } + if (!g) + g.set(gsub.get()); + else + g.setown(g->combine(gsub.get())); + } - loadFromDFS(*this,g,1,storageDir,storagePlane); - xrefRemoteDirectories(g,numdirs,storageDir,numthreads); + const char* cluster = clusters[0]; + loadFromDFS(*this, g, numdirs, dirbaselist, cluster); + xrefRemoteDirectories(g, numdirs, dirbaselist, numthreads); + } + else + { + const char *storageDir[1]; + for (int i = 0; i < nclusters; i++) + { + const char *storagePlane = clusters[i]; // clusters holds a list of storage plane names + storageDir[0] = dirbaselist[i]; // dirbaselist holds the storage plane directories + Owned g = queryNamedGroupStore().lookup(clusters[i]); + + loadFromDFS(*this, g, 1, storageDir, storagePlane); + xrefRemoteDirectories(g, numdirs, storageDir, numthreads); + } } -#else - const char* cluster = clusters[0]; - loadFromDFS(*this,g,numdirs,dirbaselist,cluster); - xrefRemoteDirectories(g,numdirs,dirbaselist,numthreads); -#endif StringBuffer filename; filename.clear().append("xrefrpt"); addFileTimestamp(filename, true); filename.append(".txt"); - - if (flags&PMtextoutput) + + if (flags&PMtextoutput) outputTextReport(filename.str()); filename.clear().append("xrefrpt"); addFileTimestamp(filename, true); filename.append(".txt"); - if (flags&PMcsvoutput) + if (flags&PMcsvoutput) outputCsvReport(filename.str()); - if (flags&PMbackupoutput) + if (flags&PMbackupoutput) outputBackupReport(); - if (flags&PMtreeoutput) - out = outputTree(); + if (flags&PMtreeoutput) + out = outputTree(); logicalnamemap.kill(); filemap.kill(); @@ -2723,26 +2734,25 @@ IPropertyTree * runXRef(unsigned nclusters,const char **clusters,IXRefProgressC #else bool islinux = true; #endif -#ifdef _CONTAINERIZED - DBGLOG("CONTAINERIZED(runXRef)"); - const char *dirs[nclusters]; // nclusters is the number of storage planes - unsigned numdirs = nclusters; - - for (int i = 0; i < numdirs; i++) - { - Owned storagePlane = getStoragePlane(clusters[i]); - dirs[i] = storagePlane->queryProp("@prefix"); - } -#else const char *dirs[2]; unsigned numdirs = 2; - // assume all nodes same OS - Owned group = queryNamedGroupStore().lookup(clusters[0]); - if (group) - islinux = queryOS(group->queryNode(0).endpoint())==MachineOsLinux; - dirs[0] = queryBaseDirectory(grp_unknown, 0,islinux?DFD_OSunix:DFD_OSwindows); // MORE - should use the info from the group store - dirs[1] = queryBaseDirectory(grp_unknown, 1,islinux?DFD_OSunix:DFD_OSwindows); -#endif + if (isContainerized()) + { + DBGLOG("CONTAINERIZED(runXRef)"); + numdirs = 1; + Owned storagePlane = getStoragePlane(clusters[0]); + dirs[0] = storagePlane->queryProp("@prefix"); + } + else + { + + // assume all nodes same OS + Owned group = queryNamedGroupStore().lookup(clusters[0]); + if (group) + islinux = queryOS(group->queryNode(0).endpoint())==MachineOsLinux; + dirs[0] = queryBaseDirectory(grp_unknown, 0,islinux?DFD_OSunix:DFD_OSwindows); // MORE - should use the info from the group store + dirs[1] = queryBaseDirectory(grp_unknown, 1,islinux?DFD_OSunix:DFD_OSwindows); + } IPropertyTree *ret=NULL; try { ret = xrefmanager.process(nclusters,clusters,numdirs,dirs,PMtreeoutput,callback,numthreads); From 42e3436e4f31e47db83200f273fc47f0af9eb484 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 9 Jul 2024 15:58:49 -0400 Subject: [PATCH 06/32] Common up XRefNodes code with containerized case --- esp/services/ws_dfu/ws_dfuXRefService.cpp | 30 ----------------------- 1 file changed, 30 deletions(-) diff --git a/esp/services/ws_dfu/ws_dfuXRefService.cpp b/esp/services/ws_dfu/ws_dfuXRefService.cpp index 55abb19344e..f177c17f5ad 100644 --- a/esp/services/ws_dfu/ws_dfuXRefService.cpp +++ b/esp/services/ws_dfu/ws_dfuXRefService.cpp @@ -605,7 +605,6 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r { context.ensureFeatureAccess(XREF_FEATURE_URL, SecAccess_Read, ECLWATCH_DFU_XREF_ACCESS_DENIED, "WsDfuXRef::DFUXRefList: Permission denied."); -#ifdef _CONTAINERIZED DBGLOG("CONTAINERIZED(CWsDfuXRefEx::onDFUXRefList)"); Owned planesIter = getPlanesIterator("data", nullptr); @@ -621,35 +620,6 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r StringBuffer buf; resp.setDFUXRefListResult(formatResult(context, xrefNodeTree, buf)); -#else - CConstWUClusterInfoArray clusters; - getEnvironmentClusterInfo(clusters); - - BoolHash uniqueProcesses; - Owned xrefNodeTree = createPTree("XRefNodes"); - ForEachItemIn(c, clusters) - { - IConstWUClusterInfo &cluster = clusters.item(c); - switch (cluster.getPlatform()) - { - case ThorLCRCluster: - { - const StringArray &primaryThorProcesses = cluster.getPrimaryThorProcesses(); - ForEachItemIn(i, primaryThorProcesses) - addUniqueXRefNode(primaryThorProcesses.item(i), uniqueProcesses, xrefNodeTree); - } - break; - case RoxieCluster: - SCMStringBuffer roxieProcess; - addUniqueXRefNode(cluster.getRoxieProcess(roxieProcess).str(), uniqueProcesses, xrefNodeTree); - break; - } - } - addXRefNode("SuperFiles", xrefNodeTree); - - StringBuffer buf; - resp.setDFUXRefListResult(formatResult(context, xrefNodeTree, buf)); -#endif } catch(IException *e) { From a54359c67ac553c93c1d092ddc0236400223cd26 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Wed, 10 Jul 2024 14:14:01 -0400 Subject: [PATCH 07/32] Add call to calcStripeNumber and use makePhysucalPartName --- dali/dfuXRefLib/dfuxreflib.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 0ca6d61406c..01acf089576 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -645,11 +645,11 @@ struct CLogicalNameEntry: public CInterface grouped = file.getPropInt("Attr/@grouped", 0)!=0; const char *partmask = file.queryProp("@partmask"); StringBuffer tmp; - partDir = file.queryProp("@directory"); if (partmask&&*partmask) { if (!containsPathSepChar(partmask)) { - if (!partDir.isEmpty()) - tmp.append(partDir).append(getPathSepChar(partDir)); + const char *dir = file.queryProp("@directory"); + if (dir&&*dir) + tmp.append(dir).append(getPathSepChar(dir)); } tmp.append(partmask); } @@ -657,7 +657,6 @@ struct CLogicalNameEntry: public CInterface substnum(tmp,"$n$",max); dirpartmask.set(tmp.str()); replicateOffset = file.getPropInt("@replicateOffset",1); - dirPerPart = max>1?getDataStoragePlane(grpname, true)->queryDirPerPart():false; } ~CLogicalNameEntry() { @@ -772,10 +771,11 @@ struct CLogicalNameEntry: public CInterface expandMask(partName, pmask, partNo, max); StringBuffer fullname; - addPathSepChar(fullname.append(partDir)); - if (dirPerPart) - addPathSepChar(fullname.append(partNo+1)); - fullname.append(partName); + Owned plane = getDataStoragePlane(grpname, true); + const char * prefix = plane->queryPrefix(); + unsigned stripeNum = calcStripeNumber(partNo, lname, plane->numDevices()); + bool dirPerPart = max>1?plane->queryDirPerPart():false; + makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); ClusterPartDiskMapSpec mspec; mspec.replicateOffset = replicateOffset; @@ -826,9 +826,7 @@ struct CLogicalNameEntry: public CInterface bool grouped; StringAttr dirpartmask; CXRefManagerBase &manager; - StringAttr partDir; int replicateOffset; - bool dirPerPart; }; From 3106f80daaa97d412459fe3ecf5a1c3d8fdf07e4 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 11 Jul 2024 09:17:12 -0400 Subject: [PATCH 08/32] Use lfnHash from file metadata instead of logical filename --- dali/dfuXRefLib/dfuxreflib.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 01acf089576..50dbfb2179a 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -657,6 +657,7 @@ struct CLogicalNameEntry: public CInterface substnum(tmp,"$n$",max); dirpartmask.set(tmp.str()); replicateOffset = file.getPropInt("@replicateOffset",1); + lfnHash = file.getPropInt("@lfnHash"); } ~CLogicalNameEntry() { @@ -773,7 +774,7 @@ struct CLogicalNameEntry: public CInterface StringBuffer fullname; Owned plane = getDataStoragePlane(grpname, true); const char * prefix = plane->queryPrefix(); - unsigned stripeNum = calcStripeNumber(partNo, lname, plane->numDevices()); + unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); bool dirPerPart = max>1?plane->queryDirPerPart():false; makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); @@ -827,6 +828,7 @@ struct CLogicalNameEntry: public CInterface StringAttr dirpartmask; CXRefManagerBase &manager; int replicateOffset; + unsigned lfnHash; }; From 0c65a8e8cb245ded16426bea4fd3f163e6a1119c Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 6 Aug 2024 08:11:17 -0400 Subject: [PATCH 09/32] Remove ifdef _CONTAINERIZED from constructPartFilename --- dali/base/dadfs.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index 4e4545843b4..d283dab0234 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -245,8 +245,8 @@ extern da_decl cost_type calcDiskWriteCost(const StringArray & clusters, stat_ty return writeCost; } -// JCSMORE - I suspect this function should be removed/deprecated. It does not deal with dirPerPart or striping. -// makePhysicalPartName supports both, but does not deal with groups/endpoints) + +// Deprecated and should be removed and new feature tested RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,unsigned copy,ClusterPartDiskMapSpec &mspec,RemoteFilename &rfn) { partno--; @@ -261,10 +261,6 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partm StringBuffer fullname; if (findPathSepChar(name)==NULL) addPathSepChar(fullname.append(partdir)); -#ifdef _CONTAINERIZED - if (partmax>1) - addPathSepChar(fullname.append(partno+1)); // If there are multiple files they will be striped by part number -#endif fullname.append(name); unsigned n; unsigned d; From 24017ae024b3d2b8e76ace89581db68c76541d71 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 6 Aug 2024 15:40:35 -0400 Subject: [PATCH 10/32] Fix dir-per-part and stripNum masks in parseFileName. --- dali/dfuXRefLib/dfuxreflib.cpp | 54 +++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 50dbfb2179a..f5acf109d63 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -959,10 +959,49 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns name = nonrepdir.str(); num = 0; max = 0; + unsigned dirPerPart = 0; for (;;) { char c=*name; if (!c) break; + if ((c=='d')&&(isdigit(name[1]))) + { + // Checks that the prefix matches and numDevices > 1 to see if d[0-9]+ is a stripe number + mname.append(c); + name++; + + Owned planesIter = getPlanesIterator("data", nullptr); + ForEach(*planesIter) + { + const IPropertyTree &plane = planesIter->query(); + if (startsWithIgnoreCase(mname.str(), plane.queryProp("@prefix"))) + { + if (plane.getPropInt("@numDevices") > 1) + { + while (*name&&isdigit(*name)) + name++; + mname.append("$P$"); + } + break; + } + } + c = *name; + } + if ((c=='/')&&(isdigit(name[1]))) + { + mname.append(c); + name++; + + // Get dir-per-part # + while (*name&&isdigit(*name)) + { + dirPerPart = dirPerPart*10+(*name-'0'); + name++; + } + + c=*name; + mname.append("$P$"); + } if ((c=='.')&&(name[1]=='_')) { unsigned pn = 0; const char *s = name+2; @@ -970,6 +1009,10 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns pn = pn*10+(*s-'0'); s++; } + + if (dirPerPart&&dirPerPart!=pn) + throw MakeStringException(-1, "dir-per-part # does not match part # of file"); + if (pn&&(memicmp(s,"_of_",4)==0)) { unsigned mn = 0; s += 4; @@ -978,17 +1021,6 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns s++; } if ((mn!=0)&&((*s==0)||(*s=='.'))&&(mn>=pn)) { // NB allow trailing extension -#ifdef _CONTAINERIZED - if (mn>1) - { - StringBuffer stripeDir; - stripeDir.append(getPathSepChar(mname)).append(pn).append(getPathSepChar(mname)); - StringBuffer stripeMask; - stripeMask.append(getPathSepChar(mname)).append("$P$").append(getPathSepChar(mname)); - // Replace part number in stripe directory with mask - mname.replaceString(stripeDir, stripeMask); - } -#endif mname.append("._$P$_of_").append(mn); if (*s) mname.append(s); From 48b4a98b9dedc35811b8429446c867a47919c650 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 8 Aug 2024 09:44:30 -0400 Subject: [PATCH 11/32] Remove Containerized DBGLOG message from generalized BM code in onDFUXRefList --- esp/services/ws_dfu/ws_dfuXRefService.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/esp/services/ws_dfu/ws_dfuXRefService.cpp b/esp/services/ws_dfu/ws_dfuXRefService.cpp index f177c17f5ad..0ae88060ef8 100644 --- a/esp/services/ws_dfu/ws_dfuXRefService.cpp +++ b/esp/services/ws_dfu/ws_dfuXRefService.cpp @@ -605,7 +605,6 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r { context.ensureFeatureAccess(XREF_FEATURE_URL, SecAccess_Read, ECLWATCH_DFU_XREF_ACCESS_DENIED, "WsDfuXRef::DFUXRefList: Permission denied."); - DBGLOG("CONTAINERIZED(CWsDfuXRefEx::onDFUXRefList)"); Owned planesIter = getPlanesIterator("data", nullptr); BoolHash uniquePlanes; From 7efc2619953193e77d6f2a2e869f19f0750d8615 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 8 Aug 2024 09:56:30 -0400 Subject: [PATCH 12/32] Co-locate fullname closer to makePhysicalPartName --- dali/dfuXRefLib/dfuxreflib.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index f5acf109d63..f61574551ca 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -771,11 +771,11 @@ struct CLogicalNameEntry: public CInterface } expandMask(partName, pmask, partNo, max); - StringBuffer fullname; Owned plane = getDataStoragePlane(grpname, true); const char * prefix = plane->queryPrefix(); unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); bool dirPerPart = max>1?plane->queryDirPerPart():false; + StringBuffer fullname; makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); ClusterPartDiskMapSpec mspec; From 99c38f0e1639b6de00d6de8440fe7a0e058e2e3e Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 8 Aug 2024 16:09:17 -0400 Subject: [PATCH 13/32] Properly check for dirPerPart in file flags. --- dali/dfuXRefLib/dfuxreflib.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index f61574551ca..1feedc17aaa 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -760,7 +760,7 @@ struct CLogicalNameEntry: public CInterface return false; } - RemoteFilename &constructPartFilename(unsigned partNo, unsigned copy, RemoteFilename &rfn) + RemoteFilename &constructPartFilename(unsigned partNo, unsigned copy, RemoteFilename &rfn, IPropertyTree &file) { partNo--; StringBuffer partName; @@ -771,10 +771,15 @@ struct CLogicalNameEntry: public CInterface } expandMask(partName, pmask, partNo, max); + // Get stripeNum from storage plane Owned plane = getDataStoragePlane(grpname, true); const char * prefix = plane->queryPrefix(); unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); - bool dirPerPart = max>1?plane->queryDirPerPart():false; + + // Get dir-per-part # from file metadata or storage plan info + FileDescriptorFlags flags = static_cast(file.getPropInt("Attr/@flags")); + bool dirPerPart = FileDescriptorFlags::none != (flags & FileDescriptorFlags::dirperpart) ? true : max>1?plane->queryDirPerPart():false; + StringBuffer fullname; makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); @@ -1603,7 +1608,7 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch manager.warn(lnentry->lname.get(),"No group found, ignoring logical file"); return; } - lnentry->constructPartFilename(partno, replicate, rfn); + lnentry->constructPartFilename(partno, replicate, rfn, file); SocketEndpoint rep=rfn.queryEndpoint(); if (manager.EndpointTable.find(rep)!=NULL) { rfn.getLocalPath(localname.clear()); From 5058f0fa350dac1e1c25b39a760b82dc5f89d88e Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 8 Aug 2024 16:16:28 -0400 Subject: [PATCH 14/32] Add comment specifying meaning of localhost in xrefRemoteDirectories --- dali/dfuXRefLib/dfuxreflib.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 1feedc17aaa..c35fc44bd8f 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -1941,6 +1941,8 @@ class CXRefManager: public CXRefManagerBase { CriticalUnblock unblock(manager.logsect); unsigned short port = 0; + // localhost signifies that this is a locally hosted or mounted storage + // which is standard in the cloud and otherwise needs to be reached via dafilesrv if (!strieq(msg, "localhost")) port = getDafsPort(node.endpoint(),numfails,&crit); results.setown(getDirectory(dirlist,&node,port)); From a3a5a1404756504683b3ba0f62c06fa2f3803ef7 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Fri, 9 Aug 2024 09:47:49 -0400 Subject: [PATCH 15/32] Remove changes to runXref and add directory getting code to CXRefManager::process --- dali/dfuXRefLib/dfuxreflib.cpp | 45 ++++++++++++++++------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index c35fc44bd8f..9a29b7a594a 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -2720,12 +2720,15 @@ class CXRefManager: public CXRefManagerBase const char *storageDir[1]; for (int i = 0; i < nclusters; i++) { - const char *storagePlane = clusters[i]; // clusters holds a list of storage plane names - storageDir[0] = dirbaselist[i]; // dirbaselist holds the storage plane directories - Owned g = queryNamedGroupStore().lookup(clusters[i]); + DBGLOG("CONTAINERIZED(CXRefManager::process)"); - loadFromDFS(*this, g, 1, storageDir, storagePlane); - xrefRemoteDirectories(g, numdirs, storageDir, numthreads); + const char *storagePlaneName = clusters[i]; // clusters holds a list of storage plane names + Owned storagePlane = getStoragePlane(storagePlaneName); + storageDir[0] = storagePlane->queryProp("@prefix"); + Owned g = queryNamedGroupStore().lookup(storagePlaneName); + + loadFromDFS(*this, g, 1, storageDir, storagePlaneName); + xrefRemoteDirectories(g, 1, storageDir, numthreads); } } @@ -2768,30 +2771,24 @@ IPropertyTree * runXRef(unsigned nclusters,const char **clusters,IXRefProgressC if (nclusters==0) return NULL; CXRefManager xrefmanager; + const char *dirs[2]; + unsigned numdirs; #ifdef _WIN32 bool islinux = false; #else bool islinux = true; #endif - const char *dirs[2]; - unsigned numdirs = 2; - if (isContainerized()) - { - DBGLOG("CONTAINERIZED(runXRef)"); - numdirs = 1; - Owned storagePlane = getStoragePlane(clusters[0]); - dirs[0] = storagePlane->queryProp("@prefix"); - } - else - { - - // assume all nodes same OS - Owned group = queryNamedGroupStore().lookup(clusters[0]); - if (group) - islinux = queryOS(group->queryNode(0).endpoint())==MachineOsLinux; - dirs[0] = queryBaseDirectory(grp_unknown, 0,islinux?DFD_OSunix:DFD_OSwindows); // MORE - should use the info from the group store - dirs[1] = queryBaseDirectory(grp_unknown, 1,islinux?DFD_OSunix:DFD_OSwindows); - } + // assume all nodes same OS + Owned group = queryNamedGroupStore().lookup(clusters[0]); +#ifdef _CONTAINERIZED + WARNLOG("CONTAINERIZED(runXRef calls queryOS())"); +#else + if (group) + islinux = queryOS(group->queryNode(0).endpoint())==MachineOsLinux; +#endif + dirs[0] = queryBaseDirectory(grp_unknown, 0,islinux?DFD_OSunix:DFD_OSwindows); // MORE - should use the info from the group store + dirs[1] = queryBaseDirectory(grp_unknown, 1,islinux?DFD_OSunix:DFD_OSwindows); + numdirs = 2; IPropertyTree *ret=NULL; try { ret = xrefmanager.process(nclusters,clusters,numdirs,dirs,PMtreeoutput,callback,numthreads); From f3a179f323f80c07e9df85581459cdf5a2c1c00e Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Wed, 6 Nov 2024 16:08:10 -0500 Subject: [PATCH 16/32] Remove use of addUniqueXrefNode --- esp/services/ws_dfu/ws_dfuXRefService.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/esp/services/ws_dfu/ws_dfuXRefService.cpp b/esp/services/ws_dfu/ws_dfuXRefService.cpp index 0ae88060ef8..17bb036b65f 100644 --- a/esp/services/ws_dfu/ws_dfuXRefService.cpp +++ b/esp/services/ws_dfu/ws_dfuXRefService.cpp @@ -606,15 +606,13 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r context.ensureFeatureAccess(XREF_FEATURE_URL, SecAccess_Read, ECLWATCH_DFU_XREF_ACCESS_DENIED, "WsDfuXRef::DFUXRefList: Permission denied."); Owned planesIter = getPlanesIterator("data", nullptr); - - BoolHash uniquePlanes; Owned xrefNodeTree = createPTree("XRefNodes"); ForEach(*planesIter) { IPropertyTree &item = planesIter->query(); - addUniqueXRefNode(item.queryProp("@name"), uniquePlanes, xrefNodeTree); + addXRefNode(item.queryProp("@name"), xrefNodeTree); } StringBuffer buf; From 2978bd4b4fc03618bcfd0e487c5a508548963b9a Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Wed, 6 Nov 2024 16:25:54 -0500 Subject: [PATCH 17/32] Add example options to xref - Not sure if any options are used --- helm/hpcc/values.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/helm/hpcc/values.yaml b/helm/hpcc/values.yaml index 6424ce9957c..34207c5a3f1 100644 --- a/helm/hpcc/values.yaml +++ b/helm/hpcc/values.yaml @@ -508,8 +508,11 @@ sasha: #disabled: true #switchMinTime: 1 #queues: "*" - + xref: {} # NB: no properties defined, use defaults + #disabled: true + #interval: 1 + #user: sasha dfuserver: - name: dfuserver From 09d8bfe4c9e6ce12e5183cd0d7ea2157d80400e7 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 7 Nov 2024 11:06:35 -0500 Subject: [PATCH 18/32] Move constructPartFilename funtion to dadfs.cpp - Change deprecated constructPartFilename to deprecatedConstructPartFilename - Keeps the constructPartFilename method simply for passing members to the function - Move file level info to creation of lnentry rather than get it for each part - Put guards on variables that can be null/empty --- dali/base/dadfs.cpp | 30 ++++++++++++++++ dali/base/dadfs.hpp | 7 ++-- dali/datest/datest.cpp | 3 +- dali/dfuXRefLib/dfuxreflib.cpp | 62 ++++++++++++++------------------- testing/unittests/dalitests.cpp | 3 +- 5 files changed, 64 insertions(+), 41 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index d283dab0234..155cc89f11d 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -273,6 +273,36 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partm return rfn; } +RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy,unsigned max,unsigned lfnHash,int replicateOffset,bool dirPerPart,const char *lname,const char *prefix,const char *pmask,IStoragePlane *plane,RemoteFilename &rfn) +{ + partNo--; + StringBuffer partName; + if (!pmask) + { + pmask = "!ERROR!._$P$_of_$N$"; + IERRLOG("No partmask for constructPartFilename"); + } + expandMask(partName, pmask, partNo, max); + + // Get stripeNum from storage plane + unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); + + StringBuffer fullname; + makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); + + ClusterPartDiskMapSpec mspec; + mspec.replicateOffset = replicateOffset; + unsigned n; + unsigned d; + mspec.calcPartLocation(partNo, max, copy, grp?grp->ordinality():max, n, d); + setReplicateFilename(fullname, d); + SocketEndpoint ep; + if (grp) + ep = grp->queryNode(n).endpoint(); + rfn.setPath(ep, fullname.toLowerCase().str()); + return rfn; +} + inline void LOGPTREE(const char *title,IPropertyTree *pt) { StringBuffer buf; diff --git a/dali/base/dadfs.hpp b/dali/base/dadfs.hpp index 6cde9f34875..12d142895e4 100644 --- a/dali/base/dadfs.hpp +++ b/dali/base/dadfs.hpp @@ -811,14 +811,15 @@ enum DistributedFileSystemError // utility routines (used by xref and dfu) -extern da_decl RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,unsigned copy,ClusterPartDiskMapSpec &mspec,RemoteFilename &rfn); +extern da_decl RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy,unsigned max,unsigned lfnHash,int replicateOffset,bool dirPerPart,const char *lname,const char *prefix,const char *pmask,IStoragePlane *plane,RemoteFilename &rfn); +extern da_decl RemoteFilename &deprecatedConstructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,unsigned copy,ClusterPartDiskMapSpec &mspec,RemoteFilename &rfn); // legacy version -inline RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,bool replicate,int replicateoffset,RemoteFilename &rfn,bool localmount=false) +inline RemoteFilename &deprecatedConstructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,bool replicate,int replicateoffset,RemoteFilename &rfn,bool localmount=false) { // local mount ignored! ClusterPartDiskMapSpec mspec; mspec.replicateOffset = replicateoffset; - return constructPartFilename(grp,partno,partmax,name,partmask,partdir,replicate?1:0,mspec,rfn); + return deprecatedConstructPartFilename(grp,partno,partmax,name,partmask,partdir,replicate?1:0,mspec,rfn); } extern da_decl IDFPartFilter *createPartFilter(const char *filter); diff --git a/dali/datest/datest.cpp b/dali/datest/datest.cpp index 84db25fe817..701f7bcae12 100644 --- a/dali/datest/datest.cpp +++ b/dali/datest/datest.cpp @@ -80,7 +80,8 @@ static void addTestFile(const char *name,unsigned n) StringBuffer path; for (unsigned m=0; m pp = createPTree("Part"); pp->setPropInt64("@size",1234*(m+1)); diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 9a29b7a594a..680cd0e54a2 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -658,6 +658,15 @@ struct CLogicalNameEntry: public CInterface dirpartmask.set(tmp.str()); replicateOffset = file.getPropInt("@replicateOffset",1); lfnHash = file.getPropInt("@lfnHash"); + plane.setown(getDataStoragePlane(grpname, true)); + if (!plane) + manager.error(_lname, "Plane definition \"%s\" is missing for File", grpname.str()); + prefix = plane->queryPrefix(); + if (prefix.isEmpty()) + manager.error(_lname, "Prefix definition for plane \"%s\" is missing for File", grpname.str()); + // Get dir-per-part # from file metadata or storage plane info + FileDescriptorFlags flags = static_cast(file.getPropInt("Attr/@flags")); + dirPerPart = FileDescriptorFlags::none != (flags & FileDescriptorFlags::dirperpart) ? true : max>1?plane->queryDirPerPart():false; } ~CLogicalNameEntry() { @@ -760,40 +769,9 @@ struct CLogicalNameEntry: public CInterface return false; } - RemoteFilename &constructPartFilename(unsigned partNo, unsigned copy, RemoteFilename &rfn, IPropertyTree &file) + RemoteFilename &constructPartFilename(unsigned partNo, unsigned copy, RemoteFilename &rfn) { - partNo--; - StringBuffer partName; - if (pmask.isEmpty()) - { - pmask.set("!ERROR!._$P$_of_$N$"); - IERRLOG("No partmask for CLogicalNameEntry::constructPartFilename"); - } - expandMask(partName, pmask, partNo, max); - - // Get stripeNum from storage plane - Owned plane = getDataStoragePlane(grpname, true); - const char * prefix = plane->queryPrefix(); - unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); - - // Get dir-per-part # from file metadata or storage plan info - FileDescriptorFlags flags = static_cast(file.getPropInt("Attr/@flags")); - bool dirPerPart = FileDescriptorFlags::none != (flags & FileDescriptorFlags::dirperpart) ? true : max>1?plane->queryDirPerPart():false; - - StringBuffer fullname; - makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); - - ClusterPartDiskMapSpec mspec; - mspec.replicateOffset = replicateOffset; - unsigned n; - unsigned d; - mspec.calcPartLocation(partNo, max, copy, grp?grp->ordinality():max, n, d); - setReplicateFilename(fullname, d); - SocketEndpoint ep; - if (grp) - ep = grp->queryNode(n).endpoint(); - rfn.setPath(ep, fullname.toLowerCase().str()); - return rfn; + return ::constructPartFilename(grp, partNo, copy, max, lfnHash, replicateOffset, dirPerPart, lname, prefix, pmask, plane, rfn); } void resolve(CFileEntry *entry); @@ -832,8 +810,11 @@ struct CLogicalNameEntry: public CInterface bool grouped; StringAttr dirpartmask; CXRefManagerBase &manager; + bool dirPerPart; int replicateOffset; unsigned lfnHash; + StringAttr prefix; + Owned plane; }; @@ -1608,7 +1589,7 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch manager.warn(lnentry->lname.get(),"No group found, ignoring logical file"); return; } - lnentry->constructPartFilename(partno, replicate, rfn, file); + lnentry->constructPartFilename(partno, replicate, rfn); SocketEndpoint rep=rfn.queryEndpoint(); if (manager.EndpointTable.find(rep)!=NULL) { rfn.getLocalPath(localname.clear()); @@ -2724,9 +2705,18 @@ class CXRefManager: public CXRefManagerBase const char *storagePlaneName = clusters[i]; // clusters holds a list of storage plane names Owned storagePlane = getStoragePlane(storagePlaneName); - storageDir[0] = storagePlane->queryProp("@prefix"); + if (!storagePlane) + { + error("XREF", "Could not find storage plane definition for %s", storagePlaneName); + return NULL; + } Owned g = queryNamedGroupStore().lookup(storagePlaneName); - + if (!g) + { + error("XREF", "Could not find cluster group for storage plane %s", storagePlaneName); + return NULL; + } + storageDir[0] = storagePlane->queryProp("@prefix"); loadFromDFS(*this, g, 1, storageDir, storagePlaneName); xrefRemoteDirectories(g, 1, storageDir, numthreads); } diff --git a/testing/unittests/dalitests.cpp b/testing/unittests/dalitests.cpp index c10a7529d2d..b5655b9acbf 100644 --- a/testing/unittests/dalitests.cpp +++ b/testing/unittests/dalitests.cpp @@ -479,7 +479,8 @@ class CDaliTestsStress : public CppUnit::TestFixture RemoteFilename rfn; for (unsigned i=0;i<3;i++) for (unsigned ic=0;ic Date: Thu, 7 Nov 2024 11:07:39 -0500 Subject: [PATCH 19/32] Change to makeStringException --- dali/dfuXRefLib/dfuxreflib.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 680cd0e54a2..7ab41ab1bfa 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -997,7 +997,7 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns } if (dirPerPart&&dirPerPart!=pn) - throw MakeStringException(-1, "dir-per-part # does not match part # of file"); + throw makeStringException(-1, "dir-per-part # does not match part # of file"); if (pn&&(memicmp(s,"_of_",4)==0)) { unsigned mn = 0; From c02e30b81e57758909e09fffaa5b3a91fa62cb58 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 7 Nov 2024 11:08:10 -0500 Subject: [PATCH 20/32] Treat paths as case sensitive --- dali/dfuXRefLib/dfuxreflib.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 7ab41ab1bfa..6b92ba7fce0 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -960,7 +960,7 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns ForEach(*planesIter) { const IPropertyTree &plane = planesIter->query(); - if (startsWithIgnoreCase(mname.str(), plane.queryProp("@prefix"))) + if (startsWith(mname.str(), plane.queryProp("@prefix"))) { if (plane.getPropInt("@numDevices") > 1) { From 58cc7f68b8667056672126b5e187c9b0fe1df123 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Fri, 8 Nov 2024 11:15:10 -0500 Subject: [PATCH 21/32] Match file prefix to storage plane definition - Find match first and throw an error if one is not found - Check next segment for striping if numDevices>1 - Only check last segment before filename for dir-per-part number - avoid copying single chars and copy chunks after we look for dir-per-part --- dali/dfuXRefLib/dfuxreflib.cpp | 93 ++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 6b92ba7fce0..047c67cc334 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -943,61 +943,65 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns replicate = setReplicateDir(name,nonrepdir,false); if (replicate) name = nonrepdir.str(); - num = 0; - max = 0; - unsigned dirPerPart = 0; - for (;;) { - char c=*name; - if (!c) - break; - if ((c=='d')&&(isdigit(name[1]))) - { - // Checks that the prefix matches and numDevices > 1 to see if d[0-9]+ is a stripe number - mname.append(c); - name++; - Owned planesIter = getPlanesIterator("data", nullptr); - ForEach(*planesIter) - { - const IPropertyTree &plane = planesIter->query(); - if (startsWith(mname.str(), plane.queryProp("@prefix"))) - { - if (plane.getPropInt("@numDevices") > 1) - { - while (*name&&isdigit(*name)) - name++; - mname.append("$P$"); - } - break; - } - } - c = *name; - } - if ((c=='/')&&(isdigit(name[1]))) + Owned planesIter = getPlanesIterator("data", nullptr); + ForEach(*planesIter) + { + const IPropertyTree &plane = planesIter->query(); + const char *prefix = plane.queryProp("@prefix"); + if (startsWith(name, prefix)) { - mname.append(c); - name++; - - // Get dir-per-part # - while (*name&&isdigit(*name)) + mname.ensureCapacity(strlen(name)); + mname.append(prefix).append(PATHSEPCHAR); + name += strlen(prefix) + 1; + if (plane.getPropInt("@numDevices") > 1) { - dirPerPart = dirPerPart*10+(*name-'0'); + if (*name&&*name!='d') + throw makeStringExceptionV(-1, "numDevices>1 but no stripe sub-directory in file %s", mname.append(name).str()); name++; + while (*name&&isdigit(*name)) + name++; + mname.append("d$P$"); } - - c=*name; - mname.append("$P$"); + break; } - if ((c=='.')&&(name[1]=='_')) { + } + + if (mname.isEmpty()) + throw makeStringExceptionV(-1, "Could not find matching prefix in plane definition for file %s", name); + + num = 0; + max = 0; + unsigned dirPerPart = 0; + const char * cur = name; + for (;;) { + char c=*cur; + if (!c) + break; + if ((c=='.')&&(cur[1]=='_')) { unsigned pn = 0; - const char *s = name+2; + const char *s = cur+2; while (*s&&isdigit(*s)) { pn = pn*10+(*s-'0'); s++; } - if (dirPerPart&&dirPerPart!=pn) - throw makeStringException(-1, "dir-per-part # does not match part # of file"); + // Check for dir-per-part number + const char *tailSlash = cur; + while (*tailSlash&&*tailSlash!='/') + tailSlash--; + const char *d = tailSlash; + for (int i=1;*(--d)&&isdigit(*d);i++) + dirPerPart = dirPerPart+(*d-'0')*i; + if (*d&&*d=='/') + { + if (dirPerPart!=pn) + throw makeStringException(-1, "dir-per-part # does not match part # of file"); + + mname.append((d+1)-name, name).append("$P$").append(cur-tailSlash, tailSlash); + } + else + mname.append(cur-name,name); if (pn&&(memicmp(s,"_of_",4)==0)) { unsigned mn = 0; @@ -1016,8 +1020,7 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns } } } - mname.append(c); - name++; + cur++; } return false; } From c799d213a8f500bbd3093327f01f5abffca5819c Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Fri, 8 Nov 2024 15:06:48 -0500 Subject: [PATCH 22/32] Add isContainerized() checks --- dali/base/dadfs.cpp | 17 +++++++++++------ dali/dfuXRefLib/dfuxreflib.cpp | 30 +++++++++++++++++++----------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index 155cc89f11d..18df16f6399 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -290,16 +290,21 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy, StringBuffer fullname; makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); - ClusterPartDiskMapSpec mspec; - mspec.replicateOffset = replicateOffset; - unsigned n; - unsigned d; - mspec.calcPartLocation(partNo, max, copy, grp?grp->ordinality():max, n, d); - setReplicateFilename(fullname, d); + unsigned n = 0; + if (!isContainerized()) + { + ClusterPartDiskMapSpec mspec; + mspec.replicateOffset = replicateOffset; + unsigned d; + mspec.calcPartLocation(partNo, max, copy, grp?grp->ordinality():max, n, d); + setReplicateFilename(fullname, d); + } + SocketEndpoint ep; if (grp) ep = grp->queryNode(n).endpoint(); rfn.setPath(ep, fullname.toLowerCase().str()); + return rfn; } diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 047c67cc334..c696afe5a44 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -769,9 +769,9 @@ struct CLogicalNameEntry: public CInterface return false; } - RemoteFilename &constructPartFilename(unsigned partNo, unsigned copy, RemoteFilename &rfn) + RemoteFilename &constructPartFilename(unsigned partNo, bool replicate, RemoteFilename &rfn) { - return ::constructPartFilename(grp, partNo, copy, max, lfnHash, replicateOffset, dirPerPart, lname, prefix, pmask, plane, rfn); + return ::constructPartFilename(grp, partNo, replicate?1:0, max, lfnHash, replicateOffset, dirPerPart, lname, prefix, pmask, plane, rfn); } void resolve(CFileEntry *entry); @@ -939,10 +939,14 @@ static void constructPartname(const char *filename,unsigned n, StringBuffer &pn, static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate) { // takes filename and creates mask filename with $P$ extension - StringBuffer nonrepdir; - replicate = setReplicateDir(name,nonrepdir,false); - if (replicate) - name = nonrepdir.str(); + if (!isContainerized()) + { + // Replicate dir is not supported in containerized environment + StringBuffer nonrepdir; + replicate = setReplicateDir(name,nonrepdir,false); + if (replicate) + name = nonrepdir.str(); + } Owned planesIter = getPlanesIterator("data", nullptr); ForEach(*planesIter) @@ -1644,6 +1648,11 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch if (lnentry->outsidenodes.find(rep)==NotFound) lnentry->outsidenodes.append(rep); } + if (isContainerized()) + { + UWARNLOG("Replication not supported in containerized version"); + break; + } if (replicate) break; replicate = true; @@ -2773,12 +2782,11 @@ IPropertyTree * runXRef(unsigned nclusters,const char **clusters,IXRefProgressC #endif // assume all nodes same OS Owned group = queryNamedGroupStore().lookup(clusters[0]); -#ifdef _CONTAINERIZED - WARNLOG("CONTAINERIZED(runXRef calls queryOS())"); -#else - if (group) + if (isContainerized()) + WARNLOG("CONTAINERIZED(runXRef calls queryOS())"); + else if (group) islinux = queryOS(group->queryNode(0).endpoint())==MachineOsLinux; -#endif + dirs[0] = queryBaseDirectory(grp_unknown, 0,islinux?DFD_OSunix:DFD_OSwindows); // MORE - should use the info from the group store dirs[1] = queryBaseDirectory(grp_unknown, 1,islinux?DFD_OSunix:DFD_OSwindows); numdirs = 2; From e12cbfb7bdb60cf08520bc744841960541462b22 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Wed, 20 Nov 2024 15:33:03 -0500 Subject: [PATCH 23/32] Add tests for parseFileName to datest.cpp - Added the tests for constructPartFilename back and removed deprecated calls --- dali/base/dadfs.cpp | 12 ++-- dali/datest/datest.cmake | 4 +- dali/datest/datest.cpp | 105 +++++++++++++++++++++++++++++++- dali/dfuXRefLib/dfuxreflib.cpp | 29 +++++---- dali/dfuXRefLib/dfuxreflib.hpp | 1 + testing/unittests/dalitests.cpp | 7 ++- testing/unittests/unittests.cpp | 16 ++++- 7 files changed, 148 insertions(+), 26 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index 18df16f6399..fdc755c0291 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -277,13 +277,15 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy, { partNo--; StringBuffer partName; - if (!pmask) + if (!lname||!*lname) { - pmask = "!ERROR!._$P$_of_$N$"; - IERRLOG("No partmask for constructPartFilename"); + if (!pmask) + { + pmask = "!ERROR!._$P$_of_$N$"; + IERRLOG("No partmask for constructPartFilename"); + } + lname = expandMask(partName, pmask, partNo, max); } - expandMask(partName, pmask, partNo, max); - // Get stripeNum from storage plane unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); diff --git a/dali/datest/datest.cmake b/dali/datest/datest.cmake index 60ae18153b6..05d41992772 100644 --- a/dali/datest/datest.cmake +++ b/dali/datest/datest.cmake @@ -38,6 +38,7 @@ include_directories ( ${HPCC_SOURCE_DIR}/fs/dafsstream ${HPCC_SOURCE_DIR}/rtl/include ${HPCC_SOURCE_DIR}/rtl/eclrtl + ${HPCC_SOURCE_DIR}/dali/dfuXRefLib ) HPCC_ADD_EXECUTABLE ( datest ${SRCS} ) @@ -50,7 +51,8 @@ target_link_libraries ( datest dafsstream eclrtl wsdfuaccess - dalibase + dalibase + dfuXRefLib ${CppUnit_LIBRARIES} ) diff --git a/dali/datest/datest.cpp b/dali/datest/datest.cpp index 701f7bcae12..a75ebfd52db 100644 --- a/dali/datest/datest.cpp +++ b/dali/datest/datest.cpp @@ -37,6 +37,8 @@ #include "rtlformat.hpp" +#include "dfuxreflib.hpp" + #include "jptree.hpp" #include "wsdfuaccess.hpp" @@ -78,10 +80,10 @@ static void addTestFile(const char *name,unsigned n) StringBuffer partmask; getPartMask(partmask,name,n); StringBuffer path; + IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned m=0; m pp = createPTree("Part"); pp->setPropInt64("@size",1234*(m+1)); @@ -3318,17 +3320,107 @@ void testlockprop(const char *lfn) printf("done\n"); } +void TestParseFileName() +{ + StringBuffer mname; + unsigned num; + unsigned max; + bool replicate; + + // Standard file name with multiple parts + assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data/test/myname._1_of_3", mname.clear(), num, max, replicate)); + assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data/test/myname._$P$_of_3")==0); + assertex(num==1); + assertex(max==3); + assertex(replicate==true); + + // Standard file name with multiple parts is striped across directories, storage plane has numDevices set for it + assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/myname._10_of_30", mname.clear(), num, max, replicate)); + assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data-two/d$P$/test/myname._$P$_of_30")==0); + assertex(num==10); + assertex(max==30); + assertex(replicate==true); + + // Standard file name with multiple parts is striped across directories and each part has its own directory, storage plane has numDevices set for it + assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._42_of_100", mname.clear(), num, max, replicate)); + assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data-two/d$P$/test/$P$/myname._$P$_of_100")==0); + assertex(num==42); + assertex(max==100); + assertex(replicate==true); + + // Test a longer part number + assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d110/test/12345/myname._12345_of_100000", mname.clear(), num, max, replicate)); + assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data-two/d$P$/test/$P$/myname._$P$_of_100000")==0); + assertex(num==12345); + assertex(max==100000); + assertex(replicate==true); + + // A file without a storage plane throws an exception + try { + testParseFileName("/test/myname._1_of_3", mname.clear(), num, max, replicate); + assertex(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + assertex(strcmp(msg.str(), "Could not find matching prefix in plane definition for file /test/myname._1_of_3")==0); + } + + // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception + try { + testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100", mname.clear(), num, max, replicate); + assertex(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + assertex(strcmp(msg.str(), "In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100")==0); + } + + // A file where the dir-per-part number does not match the part number + try { + testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100", mname.clear(), num, max, replicate); + assertex(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + assertex(strcmp(msg.str(), "Dir-per-part # does not match part # of file /var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100")==0); + } + + printf("All parseFileName tests passed\n"); +} + void usage(const char *error=NULL) { if (error) printf("%s\n", error); printf("usage: DATEST * [/test [] [/NITER ]\n"); - printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY\n"); + printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY | PARSEFILENAME\n"); printf("eg: datest . /test QTEST put -- one coven server running locally, running qtest with param \"put\"\n"); printf(" datest eq0001016 eq0001017 -- two coven servers, use default test %s\n", DEFAULT_TEST); } struct ReleaseAtomBlock { ~ReleaseAtomBlock() { releaseAtoms(); } }; +static constexpr const char * defaultYaml = R"!!( +version: "1.0" +datest: + name: datest +global: + storage: + planes: + - name: mystorageplane + storageClass: "" + storageSize: 1Gi + prefix: "/var/lib/HPCCSystems/hpcc-data" + category: data + - name: mystripedplane + storageClass: "" + storageSize: 1Gi + prefix: "/var/lib/HPCCSystems/hpcc-data-two" + numDevices: 2 + category: data +)!!"; int main(int argc, char* argv[]) { @@ -3338,6 +3430,10 @@ int main(int argc, char* argv[]) EnableSEHtoExceptionMapping(); try { + //NB: required initialization for anything that may call getGlobalConfig*() or getComponentConfig*() + Owned globals = loadConfiguration(defaultYaml, (const char **)argv, "datest", nullptr, nullptr, nullptr, nullptr, false); + initializeStorageGroups(true); + StringBuffer cmd; splitFilename(argv[0], NULL, NULL, &cmd, NULL); StringBuffer lf; @@ -3477,6 +3573,7 @@ int main(int argc, char* argv[]) case 10: TestSubLocks(); break; case 11: TestSDS3(group); break; case 12: TestNodeSubs(); break; + case 13: TestParseFileName(); break; } } else if (TEST("DFS")) @@ -3531,6 +3628,8 @@ int main(int argc, char* argv[]) testDfuStreamWrite(testParams.ordinality() ? testParams.item(0) : nullptr); else if (TEST("DFUSTREAMCOPY")) testDfuStreamCopy(testParams.ordinality() ? testParams.item(0) : nullptr); + else if (TEST("PARSEFILENAME")) + TestParseFileName(); // else if (TEST("DALILOG")) // testDaliLog(testParams.ordinality()&&0!=atoi(testParams.item(0))); else diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index c696afe5a44..931e40db516 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -939,10 +939,11 @@ static void constructPartname(const char *filename,unsigned n, StringBuffer &pn, static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate) { // takes filename and creates mask filename with $P$ extension + const char *filename = name; + StringBuffer nonrepdir; if (!isContainerized()) { // Replicate dir is not supported in containerized environment - StringBuffer nonrepdir; replicate = setReplicateDir(name,nonrepdir,false); if (replicate) name = nonrepdir.str(); @@ -953,15 +954,16 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns { const IPropertyTree &plane = planesIter->query(); const char *prefix = plane.queryProp("@prefix"); - if (startsWith(name, prefix)) + size_t prefixLen = strlen(prefix); + if (startsWith(name, prefix) && name[prefixLen] == PATHSEPCHAR) { mname.ensureCapacity(strlen(name)); mname.append(prefix).append(PATHSEPCHAR); - name += strlen(prefix) + 1; + name += prefixLen + 1; if (plane.getPropInt("@numDevices") > 1) { if (*name&&*name!='d') - throw makeStringExceptionV(-1, "numDevices>1 but no stripe sub-directory in file %s", mname.append(name).str()); + throw makeStringExceptionV(-1, "In storage plane definition numDevices>1, but no stripe sub-directory found in file %s", filename); name++; while (*name&&isdigit(*name)) name++; @@ -972,7 +974,7 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns } if (mname.isEmpty()) - throw makeStringExceptionV(-1, "Could not find matching prefix in plane definition for file %s", name); + throw makeStringExceptionV(-1, "Could not find matching prefix in plane definition for file %s", filename); num = 0; max = 0; @@ -994,13 +996,13 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns const char *tailSlash = cur; while (*tailSlash&&*tailSlash!='/') tailSlash--; - const char *d = tailSlash; - for (int i=1;*(--d)&&isdigit(*d);i++) - dirPerPart = dirPerPart+(*d-'0')*i; + const char *d = tailSlash-1; + for (int i=1;*(d)&&isdigit(*d);i*=10,d--) + dirPerPart += (*d-'0')*i; if (*d&&*d=='/') { if (dirPerPart!=pn) - throw makeStringException(-1, "dir-per-part # does not match part # of file"); + throw makeStringExceptionV(-1, "Dir-per-part # does not match part # of file %s", filename); mname.append((d+1)-name, name).append("$P$").append(cur-tailSlash, tailSlash); } @@ -1027,11 +1029,12 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns cur++; } return false; -} - - - +} +extern DFUXREFLIB_API bool testParseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate) +{ + return parseFileName(name,mname,num,max,replicate); +} class COrphanEntry: public CInterface { diff --git a/dali/dfuXRefLib/dfuxreflib.hpp b/dali/dfuXRefLib/dfuxreflib.hpp index 691aecca6c4..8bb88634a2f 100644 --- a/dali/dfuXRefLib/dfuxreflib.hpp +++ b/dali/dfuXRefLib/dfuxreflib.hpp @@ -42,5 +42,6 @@ extern DFUXREFLIB_API IPropertyTree * runXRefCluster(const char *cluster,IXRefN // this will use sasha if enabled extern DFUXREFLIB_API void testGetDir(); +extern DFUXREFLIB_API bool testParseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate); #endif diff --git a/testing/unittests/dalitests.cpp b/testing/unittests/dalitests.cpp index b5655b9acbf..5dbe150d725 100644 --- a/testing/unittests/dalitests.cpp +++ b/testing/unittests/dalitests.cpp @@ -477,10 +477,11 @@ class CDaliTestsStress : public CppUnit::TestFixture ClusterPartDiskMapSpec mspec; Owned grp = createIGroup("10.150.10.1-3"); RemoteFilename rfn; + IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned i=0;i<3;i++) - for (unsigned ic=0;ic globals = loadConfiguration(defaultYaml, argv, "unittests", nullptr, nullptr, nullptr, nullptr, false); for (int argNo = 1; argNo < argc; argNo++) From cbbc80a94971fce883148333f36339977bd8f2aa Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 21 Nov 2024 09:27:50 -0500 Subject: [PATCH 24/32] Add comments for future work --- dali/base/dadfs.cpp | 2 ++ esp/services/ws_dfu/ws_dfuXRefService.cpp | 1 + 2 files changed, 3 insertions(+) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index fdc755c0291..2efd32abe43 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -292,6 +292,8 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy, StringBuffer fullname; makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); + // revisit: constructPartFilename should be refactored not to deal with replicate directories, by pre-determining the alternate prefix if copy>0 + // If copy>0 it could do calPartLocation, find the replicate plane, get it's prefix, and pass to makePhysicalPartName unsigned n = 0; if (!isContainerized()) { diff --git a/esp/services/ws_dfu/ws_dfuXRefService.cpp b/esp/services/ws_dfu/ws_dfuXRefService.cpp index 17bb036b65f..22c82b287f8 100644 --- a/esp/services/ws_dfu/ws_dfuXRefService.cpp +++ b/esp/services/ws_dfu/ws_dfuXRefService.cpp @@ -587,6 +587,7 @@ void CWsDfuXRefEx::addXRefNode(const char* name, IPropertyTree* pXRefNodeTree) } } +// Use of addUniqueXRefNode may no longer be necessary (and the function could probably be deleted) once storage planes are fully supported because they are guarenteed to be unique. bool CWsDfuXRefEx::addUniqueXRefNode(const char *processName, BoolHash &uniqueProcesses, IPropertyTree *xrefNodeTree) { if (isEmptyString(processName)) From f4b85499832e755870cd6dab50452b28a68be204 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 21 Nov 2024 09:42:28 -0500 Subject: [PATCH 25/32] Change required argument to false --- dali/dfuXRefLib/dfuxreflib.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 931e40db516..b5da56ebcd8 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -658,7 +658,7 @@ struct CLogicalNameEntry: public CInterface dirpartmask.set(tmp.str()); replicateOffset = file.getPropInt("@replicateOffset",1); lfnHash = file.getPropInt("@lfnHash"); - plane.setown(getDataStoragePlane(grpname, true)); + plane.setown(getDataStoragePlane(grpname, false)); if (!plane) manager.error(_lname, "Plane definition \"%s\" is missing for File", grpname.str()); prefix = plane->queryPrefix(); From 85fd240e46697d14dae6a8ab7fae79a8c744ad2c Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 10 Dec 2024 12:13:10 -0500 Subject: [PATCH 26/32] Update parseFilename and move tests - Add copy flag to mirror storage planes and avoid displaying them in xref --- dali/base/dadfs.cpp | 6 +- dali/base/dadfs.hpp | 2 +- dali/base/dafdesc.cpp | 3 + dali/datest/datest.cmake | 4 +- dali/datest/datest.cpp | 105 +--------- dali/dfuXRefLib/CMakeLists.txt | 4 +- dali/dfuXRefLib/dfuxreflib.cpp | 225 +++++++++++++++++++--- dali/dfuXRefLib/dfuxreflib.hpp | 1 - esp/services/ws_dfu/ws_dfuXRefService.cpp | 18 +- testing/unittests/dalitests.cpp | 3 +- testing/unittests/unittests.cpp | 2 +- 11 files changed, 219 insertions(+), 154 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index 2efd32abe43..ee03f9cb942 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -273,7 +273,7 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partm return rfn; } -RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy,unsigned max,unsigned lfnHash,int replicateOffset,bool dirPerPart,const char *lname,const char *prefix,const char *pmask,IStoragePlane *plane,RemoteFilename &rfn) +RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy,unsigned max,unsigned lfnHash,int replicateOffset,bool dirPerPart,const char *lname,const char *prefix,const char *pmask,unsigned numDevices,RemoteFilename &rfn) { partNo--; StringBuffer partName; @@ -286,8 +286,8 @@ RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy, } lname = expandMask(partName, pmask, partNo, max); } - // Get stripeNum from storage plane - unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, plane->numDevices()); + + unsigned stripeNum = calcStripeNumber(partNo+1, lfnHash, numDevices); StringBuffer fullname; makePhysicalPartName(lname, partNo+1, max, fullname, 0, DFD_OSdefault, prefix, dirPerPart, stripeNum); diff --git a/dali/base/dadfs.hpp b/dali/base/dadfs.hpp index 12d142895e4..e79a439a0cf 100644 --- a/dali/base/dadfs.hpp +++ b/dali/base/dadfs.hpp @@ -811,7 +811,7 @@ enum DistributedFileSystemError // utility routines (used by xref and dfu) -extern da_decl RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy,unsigned max,unsigned lfnHash,int replicateOffset,bool dirPerPart,const char *lname,const char *prefix,const char *pmask,IStoragePlane *plane,RemoteFilename &rfn); +extern da_decl RemoteFilename &constructPartFilename(IGroup *grp,unsigned partNo,unsigned copy,unsigned max,unsigned lfnHash,int replicateOffset,bool dirPerPart,const char *lname,const char *prefix,const char *pmask,unsigned numDevices,RemoteFilename &rfn); extern da_decl RemoteFilename &deprecatedConstructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,unsigned copy,ClusterPartDiskMapSpec &mspec,RemoteFilename &rfn); // legacy version inline RemoteFilename &deprecatedConstructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,bool replicate,int replicateoffset,RemoteFilename &rfn,bool localmount=false) diff --git a/dali/base/dafdesc.cpp b/dali/base/dafdesc.cpp index df1c816f8a4..5aedb9988c4 100644 --- a/dali/base/dafdesc.cpp +++ b/dali/base/dafdesc.cpp @@ -3579,7 +3579,10 @@ void GroupInformation::createStoragePlane(IPropertyTree * storage, unsigned copy StringBuffer mirrorname; const char * planeName = name; if (copy != 0) + { planeName = mirrorname.append(name).append("_mirror"); + plane->setPropBool("@copy", true); + } plane->setProp("@name", planeName); diff --git a/dali/datest/datest.cmake b/dali/datest/datest.cmake index 05d41992772..60ae18153b6 100644 --- a/dali/datest/datest.cmake +++ b/dali/datest/datest.cmake @@ -38,7 +38,6 @@ include_directories ( ${HPCC_SOURCE_DIR}/fs/dafsstream ${HPCC_SOURCE_DIR}/rtl/include ${HPCC_SOURCE_DIR}/rtl/eclrtl - ${HPCC_SOURCE_DIR}/dali/dfuXRefLib ) HPCC_ADD_EXECUTABLE ( datest ${SRCS} ) @@ -51,8 +50,7 @@ target_link_libraries ( datest dafsstream eclrtl wsdfuaccess - dalibase - dfuXRefLib + dalibase ${CppUnit_LIBRARIES} ) diff --git a/dali/datest/datest.cpp b/dali/datest/datest.cpp index a75ebfd52db..62a0468da85 100644 --- a/dali/datest/datest.cpp +++ b/dali/datest/datest.cpp @@ -37,8 +37,6 @@ #include "rtlformat.hpp" -#include "dfuxreflib.hpp" - #include "jptree.hpp" #include "wsdfuaccess.hpp" @@ -80,10 +78,9 @@ static void addTestFile(const char *name,unsigned n) StringBuffer partmask; getPartMask(partmask,name,n); StringBuffer path; - IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned m=0; m pp = createPTree("Part"); pp->setPropInt64("@size",1234*(m+1)); @@ -3320,108 +3317,17 @@ void testlockprop(const char *lfn) printf("done\n"); } -void TestParseFileName() -{ - StringBuffer mname; - unsigned num; - unsigned max; - bool replicate; - - // Standard file name with multiple parts - assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data/test/myname._1_of_3", mname.clear(), num, max, replicate)); - assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data/test/myname._$P$_of_3")==0); - assertex(num==1); - assertex(max==3); - assertex(replicate==true); - - // Standard file name with multiple parts is striped across directories, storage plane has numDevices set for it - assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/myname._10_of_30", mname.clear(), num, max, replicate)); - assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data-two/d$P$/test/myname._$P$_of_30")==0); - assertex(num==10); - assertex(max==30); - assertex(replicate==true); - - // Standard file name with multiple parts is striped across directories and each part has its own directory, storage plane has numDevices set for it - assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._42_of_100", mname.clear(), num, max, replicate)); - assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data-two/d$P$/test/$P$/myname._$P$_of_100")==0); - assertex(num==42); - assertex(max==100); - assertex(replicate==true); - - // Test a longer part number - assertex(testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d110/test/12345/myname._12345_of_100000", mname.clear(), num, max, replicate)); - assertex(strcmp(mname.str(), "/var/lib/HPCCSystems/hpcc-data-two/d$P$/test/$P$/myname._$P$_of_100000")==0); - assertex(num==12345); - assertex(max==100000); - assertex(replicate==true); - - // A file without a storage plane throws an exception - try { - testParseFileName("/test/myname._1_of_3", mname.clear(), num, max, replicate); - assertex(false); - } catch (IException *e) { - StringBuffer msg; - e->errorMessage(msg); - e->Release(); - assertex(strcmp(msg.str(), "Could not find matching prefix in plane definition for file /test/myname._1_of_3")==0); - } - - // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception - try { - testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100", mname.clear(), num, max, replicate); - assertex(false); - } catch (IException *e) { - StringBuffer msg; - e->errorMessage(msg); - e->Release(); - assertex(strcmp(msg.str(), "In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100")==0); - } - - // A file where the dir-per-part number does not match the part number - try { - testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100", mname.clear(), num, max, replicate); - assertex(false); - } catch (IException *e) { - StringBuffer msg; - e->errorMessage(msg); - e->Release(); - assertex(strcmp(msg.str(), "Dir-per-part # does not match part # of file /var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100")==0); - } - - printf("All parseFileName tests passed\n"); -} - void usage(const char *error=NULL) { if (error) printf("%s\n", error); printf("usage: DATEST * [/test [] [/NITER ]\n"); - printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY | PARSEFILENAME\n"); + printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY\n"); printf("eg: datest . /test QTEST put -- one coven server running locally, running qtest with param \"put\"\n"); printf(" datest eq0001016 eq0001017 -- two coven servers, use default test %s\n", DEFAULT_TEST); } struct ReleaseAtomBlock { ~ReleaseAtomBlock() { releaseAtoms(); } }; -static constexpr const char * defaultYaml = R"!!( -version: "1.0" -datest: - name: datest -global: - storage: - planes: - - name: mystorageplane - storageClass: "" - storageSize: 1Gi - prefix: "/var/lib/HPCCSystems/hpcc-data" - category: data - - name: mystripedplane - storageClass: "" - storageSize: 1Gi - prefix: "/var/lib/HPCCSystems/hpcc-data-two" - numDevices: 2 - category: data -)!!"; - int main(int argc, char* argv[]) { ReleaseAtomBlock rABlock; @@ -3430,10 +3336,6 @@ int main(int argc, char* argv[]) EnableSEHtoExceptionMapping(); try { - //NB: required initialization for anything that may call getGlobalConfig*() or getComponentConfig*() - Owned globals = loadConfiguration(defaultYaml, (const char **)argv, "datest", nullptr, nullptr, nullptr, nullptr, false); - initializeStorageGroups(true); - StringBuffer cmd; splitFilename(argv[0], NULL, NULL, &cmd, NULL); StringBuffer lf; @@ -3573,7 +3475,6 @@ int main(int argc, char* argv[]) case 10: TestSubLocks(); break; case 11: TestSDS3(group); break; case 12: TestNodeSubs(); break; - case 13: TestParseFileName(); break; } } else if (TEST("DFS")) @@ -3628,8 +3529,6 @@ int main(int argc, char* argv[]) testDfuStreamWrite(testParams.ordinality() ? testParams.item(0) : nullptr); else if (TEST("DFUSTREAMCOPY")) testDfuStreamCopy(testParams.ordinality() ? testParams.item(0) : nullptr); - else if (TEST("PARSEFILENAME")) - TestParseFileName(); // else if (TEST("DALILOG")) // testDaliLog(testParams.ordinality()&&0!=atoi(testParams.item(0))); else diff --git a/dali/dfuXRefLib/CMakeLists.txt b/dali/dfuXRefLib/CMakeLists.txt index 7e931f84074..b99d9d29c77 100644 --- a/dali/dfuXRefLib/CMakeLists.txt +++ b/dali/dfuXRefLib/CMakeLists.txt @@ -46,6 +46,7 @@ include_directories ( ./../../common/environment ./../../common/workunit ./../../system/security/shared + ${HPCC_SOURCE_DIR}/testing/unittests ) ADD_DEFINITIONS( -D_USRDLL -DDFUXREFLIB_EXPORTS ) @@ -57,7 +58,8 @@ target_link_libraries ( dfuXRefLib mp hrpc dafsclient - dalibase + dalibase + ${CppUnit_LIBRARIES} ) if (NOT CONTAINERIZED) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index b5da56ebcd8..dff07c85891 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -771,7 +771,12 @@ struct CLogicalNameEntry: public CInterface RemoteFilename &constructPartFilename(unsigned partNo, bool replicate, RemoteFilename &rfn) { - return ::constructPartFilename(grp, partNo, replicate?1:0, max, lfnHash, replicateOffset, dirPerPart, lname, prefix, pmask, plane, rfn); + if (!plane) + throw MakeStringException(0, "Plane definition \"%s\" is missing for File", grpname.str()); + unsigned numDevices = plane->numDevices(); + bool r = replicate?1:0; + + return ::constructPartFilename(grp, partNo, r, max, lfnHash, replicateOffset, dirPerPart, lname, prefix, pmask, numDevices, rfn); } void resolve(CFileEntry *entry); @@ -936,18 +941,19 @@ static void constructPartname(const char *filename,unsigned n, StringBuffer &pn, } } -static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate) +static void parseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,unsigned &stripeNum,unsigned &dirPerPart,bool &replicate) { // takes filename and creates mask filename with $P$ extension const char *filename = name; StringBuffer nonrepdir; if (!isContainerized()) { - // Replicate dir is not supported in containerized environment replicate = setReplicateDir(name,nonrepdir,false); if (replicate) name = nonrepdir.str(); } + else + replicate = false; // Replicate dir is not supported in containerized environment Owned planesIter = getPlanesIterator("data", nullptr); ForEach(*planesIter) @@ -958,16 +964,26 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns if (startsWith(name, prefix) && name[prefixLen] == PATHSEPCHAR) { mname.ensureCapacity(strlen(name)); - mname.append(prefix).append(PATHSEPCHAR); - name += prefixLen + 1; + mname.append(prefix); + name += prefixLen; + stripeNum = 0; if (plane.getPropInt("@numDevices") > 1) { - if (*name&&*name!='d') + name++; + if (!*name || *name!='d') throw makeStringExceptionV(-1, "In storage plane definition numDevices>1, but no stripe sub-directory found in file %s", filename); name++; + if (*name==PATHSEPCHAR) + throw makeStringExceptionV(-1, "In storage plane definition numDevices>1, but no stripe sub-directory found in file %s", filename); while (*name&&isdigit(*name)) + { + stripeNum = stripeNum*10+(*name-'0'); name++; - mname.append("d$P$"); + } + if (!*name || *name!=PATHSEPCHAR) + throw makeStringExceptionV(-1, "In storage plane definition numDevices>1, but no stripe sub-directory found in file %s", filename); + if (stripeNum>=plane.getPropInt("@numDevices")) + throw makeStringExceptionV(-1, "Stripe number in file %s is greater than numDevices in storage plane definition", filename); } break; } @@ -978,7 +994,7 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns num = 0; max = 0; - unsigned dirPerPart = 0; + dirPerPart = 0; const char * cur = name; for (;;) { char c=*cur; @@ -994,17 +1010,17 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns // Check for dir-per-part number const char *tailSlash = cur; - while (*tailSlash&&*tailSlash!='/') + while (tailSlash!=name&&*tailSlash!='/') tailSlash--; const char *d = tailSlash-1; - for (int i=1;*(d)&&isdigit(*d);i*=10,d--) + for (int i=1;d!=name&&isdigit(*d);i*=10,d--) dirPerPart += (*d-'0')*i; + // If a dir-per-part number was found, check that it matches the part number if (*d&&*d=='/') { if (dirPerPart!=pn) throw makeStringExceptionV(-1, "Dir-per-part # does not match part # of file %s", filename); - - mname.append((d+1)-name, name).append("$P$").append(cur-tailSlash, tailSlash); + mname.append((d+1)-name, name).append(cur-(tailSlash+1), tailSlash+1); } else mname.append(cur-name,name); @@ -1022,19 +1038,17 @@ static bool parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns mname.append(s); num = pn; max = mn; - return true; } + else + throw makeStringExceptionV(-1, "Incorrect max part number(%d) and part number (%d) in file %s", mn, pn, filename); } + else + throw makeStringExceptionV(-1, "Missing part number in file %s", filename); } cur++; } - return false; } -extern DFUXREFLIB_API bool testParseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate) -{ - return parseFileName(name,mname,num,max,replicate); -} class COrphanEntry: public CInterface { @@ -1653,7 +1667,7 @@ void loadFromDFS(CXRefManagerBase &manager,IGroup *grp,unsigned numdirs,const ch } if (isContainerized()) { - UWARNLOG("Replication not supported in containerized version"); + // NB: Replication not supported in containerized version break; } if (replicate) @@ -1754,13 +1768,14 @@ class CPhysicalXREF StringBuffer orphanname; unsigned m; unsigned n; + unsigned stripeNum; + unsigned dirPerPart; bool replicate; - parseFileName(fullname,orphanname,n,m,replicate); - if (n>m) - manager.error(fullname, "Part %d: number greater than max %d",n+1,m); - else { - orphanname.toLowerCase(); + try + { + parseFileName(fullname,orphanname,n,m,stripeNum,dirPerPart,replicate); + orphanname.toLowerCase(); COrphanEntryPtr *entryp = manager.orphanmap.getValue(orphanname.str()); COrphanEntryPtr entry; @@ -1778,6 +1793,13 @@ class CPhysicalXREF n--; entry->add(ep,n,replicate,sz); } + catch (IException *e) + { + StringBuffer s; + e->errorMessage(s.clear()); + e->Release(); + manager.error(fullname, "%s", s.str()); + } } } @@ -3010,3 +3032,158 @@ IPropertyTree * RunProcess(XRefCmd cmd, unsigned nclusters,const char **clusters } return nullptr; } + +#ifdef _USE_CPPUNIT + +#include "unittests.hpp" + +#define CPPUNIT_ASSERT_EQUAL_STR(x, y) CPPUNIT_ASSERT_EQUAL(std::string(x ? x : ""),std::string(y ? y : "")) +class DFUXrefLibTests : public CppUnit::TestFixture +{ + CPPUNIT_TEST_SUITE(DFUXrefLibTests); + CPPUNIT_TEST(TestParseFileName); + CPPUNIT_TEST_SUITE_END(); + +protected: + void TestParseFileName() + { + StringBuffer mname; + unsigned num; + unsigned max; + unsigned stripeNum; + unsigned dirPerPart; + bool replicate; + + // Standard file name with multiple parts + parseFileName("/var/lib/HPCCSystems/hpcc-data/test/myname._1_of_3", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT_EQUAL_STR("/var/lib/HPCCSystems/hpcc-data/test/myname._$P$_of_3",mname.str()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Part number is incorrect",(unsigned)1,num); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Max part number is incorrect",(unsigned)3,max); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Stripe number is incorrect",(unsigned)0,stripeNum); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Dir-per-part number is incorrect",(unsigned)0,dirPerPart); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Replicate is incorrect",true,replicate); + + // Standard file name with multiple parts is striped across directories, storage plane has numDevices set to 111 + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/myname._10_of_30", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT_EQUAL_STR("/var/lib/HPCCSystems/hpcc-data-two/test/myname._$P$_of_30",mname.str()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Part number is incorrect",(unsigned)10,num); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Max part number is incorrect",(unsigned)30,max); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Stripe number is incorrect",(unsigned)1,stripeNum); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Dir-per-part number is incorrect",(unsigned)0,dirPerPart); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Replicate is incorrect",true,replicate); + + // Standard file name with multiple parts is striped across directories and each part has its own directory, storage plane has numDevices set to 111 + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT_EQUAL_STR("/var/lib/HPCCSystems/hpcc-data-two/test/myname._$P$_of_100",mname.str()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Part number is incorrect",(unsigned)42,num); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Max part number is incorrect",(unsigned)100,max); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Stripe number is incorrect",(unsigned)1,stripeNum); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Dir-per-part number is incorrect",(unsigned)42,dirPerPart); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Replicate is incorrect",true,replicate); + + // Test a longer part number + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d110/test/12345/myname._12345_of_100000", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT_EQUAL_STR("/var/lib/HPCCSystems/hpcc-data-two/test/myname._$P$_of_100000",mname.str()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Part number is incorrect",(unsigned)12345,num); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Max part number is incorrect",(unsigned)100000,max); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Stripe number is incorrect",(unsigned)110,stripeNum); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Dir-per-part number is incorrect",(unsigned)12345,dirPerPart); + CPPUNIT_ASSERT_EQUAL_MESSAGE("Replicate is incorrect",true,replicate); + + // A file without a storage plane throws an exception + try { + parseFileName("/test/myname._1_of_3", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("Could not find matching prefix in plane definition for file /test/myname._1_of_3",msg.str()); + } + + // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100",msg.str()); + } + + // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/d/test/myname.42_of_100",msg.str()); + } + + // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/datadir/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/datadir/test/myname.42_of_100",msg.str()); + } + + // A file where the stripe number is equal to numDevices(111) throws an exception + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d111/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("Stripe number in file /var/lib/HPCCSystems/hpcc-data-two/d111/test/myname.42_of_100 is greater than numDevices in storage plane definition",msg.str()); + } + + // A file where the dir-per-part number does not match the part number + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("Dir-per-part # does not match part # of file /var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100",msg.str()); + } + + // A file where the part number is greater than the max part number + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/1000/myname._1000_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("Incorrect max part number(100) and part number (1000) in file /var/lib/HPCCSystems/hpcc-data-two/d1/test/1000/myname._1000_of_100",msg.str()); + } + + // A file where the part number is missing + try { + parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/myname._of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); + CPPUNIT_ASSERT(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + CPPUNIT_ASSERT_EQUAL_STR("Missing part number in file /var/lib/HPCCSystems/hpcc-data-two/d1/test/myname._of_100",msg.str()); + } + + printf("All parseFileName tests passed\n"); + } + +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(DFUXrefLibTests); +CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(DFUXrefLibTests, "DFUXrefLibTests"); + +#endif // _USE_CPPUNIT diff --git a/dali/dfuXRefLib/dfuxreflib.hpp b/dali/dfuXRefLib/dfuxreflib.hpp index 8bb88634a2f..691aecca6c4 100644 --- a/dali/dfuXRefLib/dfuxreflib.hpp +++ b/dali/dfuXRefLib/dfuxreflib.hpp @@ -42,6 +42,5 @@ extern DFUXREFLIB_API IPropertyTree * runXRefCluster(const char *cluster,IXRefN // this will use sasha if enabled extern DFUXREFLIB_API void testGetDir(); -extern DFUXREFLIB_API bool testParseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate); #endif diff --git a/esp/services/ws_dfu/ws_dfuXRefService.cpp b/esp/services/ws_dfu/ws_dfuXRefService.cpp index 22c82b287f8..670a76d480f 100644 --- a/esp/services/ws_dfu/ws_dfuXRefService.cpp +++ b/esp/services/ws_dfu/ws_dfuXRefService.cpp @@ -587,19 +587,6 @@ void CWsDfuXRefEx::addXRefNode(const char* name, IPropertyTree* pXRefNodeTree) } } -// Use of addUniqueXRefNode may no longer be necessary (and the function could probably be deleted) once storage planes are fully supported because they are guarenteed to be unique. -bool CWsDfuXRefEx::addUniqueXRefNode(const char *processName, BoolHash &uniqueProcesses, IPropertyTree *xrefNodeTree) -{ - if (isEmptyString(processName)) - return false; - bool *found = uniqueProcesses.getValue(processName); - if (found && *found) - return false; - uniqueProcesses.setValue(processName, true); - addXRefNode(processName, xrefNodeTree); - return true; -} - bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &req, IEspDFUXRefListResponse &resp) { try @@ -612,8 +599,9 @@ bool CWsDfuXRefEx::onDFUXRefList(IEspContext &context, IEspDFUXRefListRequest &r ForEach(*planesIter) { IPropertyTree &item = planesIter->query(); - - addXRefNode(item.queryProp("@name"), xrefNodeTree); + bool isCopy = item.getPropBool("@copy", false); + if (!isCopy) + addXRefNode(item.queryProp("@name"), xrefNodeTree); } StringBuffer buf; diff --git a/testing/unittests/dalitests.cpp b/testing/unittests/dalitests.cpp index 5dbe150d725..fbc7c21478a 100644 --- a/testing/unittests/dalitests.cpp +++ b/testing/unittests/dalitests.cpp @@ -477,11 +477,10 @@ class CDaliTestsStress : public CppUnit::TestFixture ClusterPartDiskMapSpec mspec; Owned grp = createIGroup("10.150.10.1-3"); RemoteFilename rfn; - IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned i=0;i<3;i++) for (unsigned ic=0;ic Date: Mon, 8 Jul 2024 10:21:09 -0400 Subject: [PATCH 27/32] Add constructPartFilename to CLogicalNameEntry --- dali/dfuXRefLib/dfuxreflib.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index dff07c85891..7d3d98ede5a 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -645,6 +645,7 @@ struct CLogicalNameEntry: public CInterface grouped = file.getPropInt("Attr/@grouped", 0)!=0; const char *partmask = file.queryProp("@partmask"); StringBuffer tmp; + partDir = file.queryProp("@directory"); if (partmask&&*partmask) { if (!containsPathSepChar(partmask)) { const char *dir = file.queryProp("@directory"); From ccab469c577c31afed21c59e4109ec140564741a Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Wed, 20 Nov 2024 15:33:03 -0500 Subject: [PATCH 28/32] Add tests for parseFileName to datest.cpp - Added the tests for constructPartFilename back and removed deprecated calls --- dali/datest/datest.cmake | 4 +- dali/datest/datest.cpp | 83 ++++++++++++++++++++++++++++++++- dali/dfuXRefLib/dfuxreflib.hpp | 1 + testing/unittests/dalitests.cpp | 1 + 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/dali/datest/datest.cmake b/dali/datest/datest.cmake index 60ae18153b6..05d41992772 100644 --- a/dali/datest/datest.cmake +++ b/dali/datest/datest.cmake @@ -38,6 +38,7 @@ include_directories ( ${HPCC_SOURCE_DIR}/fs/dafsstream ${HPCC_SOURCE_DIR}/rtl/include ${HPCC_SOURCE_DIR}/rtl/eclrtl + ${HPCC_SOURCE_DIR}/dali/dfuXRefLib ) HPCC_ADD_EXECUTABLE ( datest ${SRCS} ) @@ -50,7 +51,8 @@ target_link_libraries ( datest dafsstream eclrtl wsdfuaccess - dalibase + dalibase + dfuXRefLib ${CppUnit_LIBRARIES} ) diff --git a/dali/datest/datest.cpp b/dali/datest/datest.cpp index 62a0468da85..0547a739a4d 100644 --- a/dali/datest/datest.cpp +++ b/dali/datest/datest.cpp @@ -37,6 +37,8 @@ #include "rtlformat.hpp" +#include "dfuxreflib.hpp" + #include "jptree.hpp" #include "wsdfuaccess.hpp" @@ -78,6 +80,7 @@ static void addTestFile(const char *name,unsigned n) StringBuffer partmask; getPartMask(partmask,name,n); StringBuffer path; + IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned m=0; merrorMessage(msg); + e->Release(); + assertex(strcmp(msg.str(), "Could not find matching prefix in plane definition for file /test/myname._1_of_3")==0); + } + + // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception + try { + testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100", mname.clear(), num, max, replicate); + assertex(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + assertex(strcmp(msg.str(), "In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100")==0); + } + + // A file where the dir-per-part number does not match the part number + try { + testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100", mname.clear(), num, max, replicate); + assertex(false); + } catch (IException *e) { + StringBuffer msg; + e->errorMessage(msg); + e->Release(); + assertex(strcmp(msg.str(), "Dir-per-part # does not match part # of file /var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100")==0); + } + + printf("All parseFileName tests passed\n"); +} + void usage(const char *error=NULL) { if (error) printf("%s\n", error); printf("usage: DATEST * [/test [] [/NITER ]\n"); - printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY\n"); + printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY | PARSEFILENAME\n"); printf("eg: datest . /test QTEST put -- one coven server running locally, running qtest with param \"put\"\n"); printf(" datest eq0001016 eq0001017 -- two coven servers, use default test %s\n", DEFAULT_TEST); } @@ -3336,6 +3410,10 @@ int main(int argc, char* argv[]) EnableSEHtoExceptionMapping(); try { + //NB: required initialization for anything that may call getGlobalConfig*() or getComponentConfig*() + Owned globals = loadConfiguration(defaultYaml, (const char **)argv, "datest", nullptr, nullptr, nullptr, nullptr, false); + initializeStorageGroups(true); + StringBuffer cmd; splitFilename(argv[0], NULL, NULL, &cmd, NULL); StringBuffer lf; @@ -3475,6 +3553,7 @@ int main(int argc, char* argv[]) case 10: TestSubLocks(); break; case 11: TestSDS3(group); break; case 12: TestNodeSubs(); break; + case 13: TestParseFileName(); break; } } else if (TEST("DFS")) @@ -3529,6 +3608,8 @@ int main(int argc, char* argv[]) testDfuStreamWrite(testParams.ordinality() ? testParams.item(0) : nullptr); else if (TEST("DFUSTREAMCOPY")) testDfuStreamCopy(testParams.ordinality() ? testParams.item(0) : nullptr); + else if (TEST("PARSEFILENAME")) + TestParseFileName(); // else if (TEST("DALILOG")) // testDaliLog(testParams.ordinality()&&0!=atoi(testParams.item(0))); else diff --git a/dali/dfuXRefLib/dfuxreflib.hpp b/dali/dfuXRefLib/dfuxreflib.hpp index 691aecca6c4..8bb88634a2f 100644 --- a/dali/dfuXRefLib/dfuxreflib.hpp +++ b/dali/dfuXRefLib/dfuxreflib.hpp @@ -42,5 +42,6 @@ extern DFUXREFLIB_API IPropertyTree * runXRefCluster(const char *cluster,IXRefN // this will use sasha if enabled extern DFUXREFLIB_API void testGetDir(); +extern DFUXREFLIB_API bool testParseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate); #endif diff --git a/testing/unittests/dalitests.cpp b/testing/unittests/dalitests.cpp index fbc7c21478a..434e873d16d 100644 --- a/testing/unittests/dalitests.cpp +++ b/testing/unittests/dalitests.cpp @@ -477,6 +477,7 @@ class CDaliTestsStress : public CppUnit::TestFixture ClusterPartDiskMapSpec mspec; Owned grp = createIGroup("10.150.10.1-3"); RemoteFilename rfn; + IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned i=0;i<3;i++) for (unsigned ic=0;ic Date: Tue, 10 Dec 2024 12:32:22 -0500 Subject: [PATCH 29/32] Remove old changes --- dali/datest/datest.cmake | 4 +- dali/datest/datest.cpp | 83 +--------------------------------- dali/dfuXRefLib/dfuxreflib.hpp | 1 - 3 files changed, 2 insertions(+), 86 deletions(-) diff --git a/dali/datest/datest.cmake b/dali/datest/datest.cmake index 05d41992772..60ae18153b6 100644 --- a/dali/datest/datest.cmake +++ b/dali/datest/datest.cmake @@ -38,7 +38,6 @@ include_directories ( ${HPCC_SOURCE_DIR}/fs/dafsstream ${HPCC_SOURCE_DIR}/rtl/include ${HPCC_SOURCE_DIR}/rtl/eclrtl - ${HPCC_SOURCE_DIR}/dali/dfuXRefLib ) HPCC_ADD_EXECUTABLE ( datest ${SRCS} ) @@ -51,8 +50,7 @@ target_link_libraries ( datest dafsstream eclrtl wsdfuaccess - dalibase - dfuXRefLib + dalibase ${CppUnit_LIBRARIES} ) diff --git a/dali/datest/datest.cpp b/dali/datest/datest.cpp index 0547a739a4d..62a0468da85 100644 --- a/dali/datest/datest.cpp +++ b/dali/datest/datest.cpp @@ -37,8 +37,6 @@ #include "rtlformat.hpp" -#include "dfuxreflib.hpp" - #include "jptree.hpp" #include "wsdfuaccess.hpp" @@ -80,7 +78,6 @@ static void addTestFile(const char *name,unsigned n) StringBuffer partmask; getPartMask(partmask,name,n); StringBuffer path; - IStoragePlane *plane = getDataStoragePlane("mystorageplane", true); for (unsigned m=0; merrorMessage(msg); - e->Release(); - assertex(strcmp(msg.str(), "Could not find matching prefix in plane definition for file /test/myname._1_of_3")==0); - } - - // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception - try { - testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100", mname.clear(), num, max, replicate); - assertex(false); - } catch (IException *e) { - StringBuffer msg; - e->errorMessage(msg); - e->Release(); - assertex(strcmp(msg.str(), "In storage plane definition numDevices>1, but no stripe sub-directory found in file /var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100")==0); - } - - // A file where the dir-per-part number does not match the part number - try { - testParseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100", mname.clear(), num, max, replicate); - assertex(false); - } catch (IException *e) { - StringBuffer msg; - e->errorMessage(msg); - e->Release(); - assertex(strcmp(msg.str(), "Dir-per-part # does not match part # of file /var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100")==0); - } - - printf("All parseFileName tests passed\n"); -} - void usage(const char *error=NULL) { if (error) printf("%s\n", error); printf("usage: DATEST * [/test [] [/NITER ]\n"); - printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY | PARSEFILENAME\n"); + printf("where name = RANDTEST | DFS | QTEST | QTEST2 | SESSION | LOCKS | SDS1 | SDS2 | XPATHS| STRESS | STRESS2 | SHUTDOWN | EXTERNAL | SUBLOCKS | SUBSCRIPTION | CONNECTIONSUBS | MULTIFILE | NODESUBS | DFUSTREAMREAD | DFUSTREAMWRITE | DFUSTREAMCOPY\n"); printf("eg: datest . /test QTEST put -- one coven server running locally, running qtest with param \"put\"\n"); printf(" datest eq0001016 eq0001017 -- two coven servers, use default test %s\n", DEFAULT_TEST); } @@ -3410,10 +3336,6 @@ int main(int argc, char* argv[]) EnableSEHtoExceptionMapping(); try { - //NB: required initialization for anything that may call getGlobalConfig*() or getComponentConfig*() - Owned globals = loadConfiguration(defaultYaml, (const char **)argv, "datest", nullptr, nullptr, nullptr, nullptr, false); - initializeStorageGroups(true); - StringBuffer cmd; splitFilename(argv[0], NULL, NULL, &cmd, NULL); StringBuffer lf; @@ -3553,7 +3475,6 @@ int main(int argc, char* argv[]) case 10: TestSubLocks(); break; case 11: TestSDS3(group); break; case 12: TestNodeSubs(); break; - case 13: TestParseFileName(); break; } } else if (TEST("DFS")) @@ -3608,8 +3529,6 @@ int main(int argc, char* argv[]) testDfuStreamWrite(testParams.ordinality() ? testParams.item(0) : nullptr); else if (TEST("DFUSTREAMCOPY")) testDfuStreamCopy(testParams.ordinality() ? testParams.item(0) : nullptr); - else if (TEST("PARSEFILENAME")) - TestParseFileName(); // else if (TEST("DALILOG")) // testDaliLog(testParams.ordinality()&&0!=atoi(testParams.item(0))); else diff --git a/dali/dfuXRefLib/dfuxreflib.hpp b/dali/dfuXRefLib/dfuxreflib.hpp index 8bb88634a2f..691aecca6c4 100644 --- a/dali/dfuXRefLib/dfuxreflib.hpp +++ b/dali/dfuXRefLib/dfuxreflib.hpp @@ -42,6 +42,5 @@ extern DFUXREFLIB_API IPropertyTree * runXRefCluster(const char *cluster,IXRefN // this will use sasha if enabled extern DFUXREFLIB_API void testGetDir(); -extern DFUXREFLIB_API bool testParseFileName(const char *name,StringBuffer &mname,unsigned &num,unsigned &max,bool &replicate); #endif From d584c6b649287c8b37679c35618ccc5c6f0d0c87 Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 10 Dec 2024 12:38:52 -0500 Subject: [PATCH 30/32] Fix rebase issue. --- dali/dfuXRefLib/dfuxreflib.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index 7d3d98ede5a..dff07c85891 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -645,7 +645,6 @@ struct CLogicalNameEntry: public CInterface grouped = file.getPropInt("Attr/@grouped", 0)!=0; const char *partmask = file.queryProp("@partmask"); StringBuffer tmp; - partDir = file.queryProp("@directory"); if (partmask&&*partmask) { if (!containsPathSepChar(partmask)) { const char *dir = file.queryProp("@directory"); From 8c3bc488e995836419f2ec9d998e0b15e37f168e Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 17 Dec 2024 12:03:21 -0500 Subject: [PATCH 31/32] Changed name increment - Fixed style issues --- dali/dfuXRefLib/dfuxreflib.cpp | 89 +++++++++++++++++++++++----------- 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/dali/dfuXRefLib/dfuxreflib.cpp b/dali/dfuXRefLib/dfuxreflib.cpp index dff07c85891..51011ef2a94 100644 --- a/dali/dfuXRefLib/dfuxreflib.cpp +++ b/dali/dfuXRefLib/dfuxreflib.cpp @@ -772,7 +772,7 @@ struct CLogicalNameEntry: public CInterface RemoteFilename &constructPartFilename(unsigned partNo, bool replicate, RemoteFilename &rfn) { if (!plane) - throw MakeStringException(0, "Plane definition \"%s\" is missing for File", grpname.str()); + throw makeStringExceptionV(0, "Plane definition \"%s\" is missing for File", grpname.str()); unsigned numDevices = plane->numDevices(); bool r = replicate?1:0; @@ -964,13 +964,12 @@ static void parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns if (startsWith(name, prefix) && name[prefixLen] == PATHSEPCHAR) { mname.ensureCapacity(strlen(name)); - mname.append(prefix); - name += prefixLen; + mname.append(prefix).append(PATHSEPCHAR); + name += prefixLen + 1; stripeNum = 0; if (plane.getPropInt("@numDevices") > 1) { - name++; - if (!*name || *name!='d') + if (*name!='d') throw makeStringExceptionV(-1, "In storage plane definition numDevices>1, but no stripe sub-directory found in file %s", filename); name++; if (*name==PATHSEPCHAR) @@ -980,8 +979,9 @@ static void parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns stripeNum = stripeNum*10+(*name-'0'); name++; } - if (!*name || *name!=PATHSEPCHAR) + if (*name!=PATHSEPCHAR) throw makeStringExceptionV(-1, "In storage plane definition numDevices>1, but no stripe sub-directory found in file %s", filename); + name++; if (stripeNum>=plane.getPropInt("@numDevices")) throw makeStringExceptionV(-1, "Stripe number in file %s is greater than numDevices in storage plane definition", filename); } @@ -996,14 +996,17 @@ static void parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns max = 0; dirPerPart = 0; const char * cur = name; - for (;;) { + for (;;) + { char c=*cur; if (!c) break; - if ((c=='.')&&(cur[1]=='_')) { + if ((c=='.')&&(cur[1]=='_')) + { unsigned pn = 0; const char *s = cur+2; - while (*s&&isdigit(*s)) { + while (*s&&isdigit(*s)) + { pn = pn*10+(*s-'0'); s++; } @@ -1016,7 +1019,7 @@ static void parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns for (int i=1;d!=name&&isdigit(*d);i*=10,d--) dirPerPart += (*d-'0')*i; // If a dir-per-part number was found, check that it matches the part number - if (*d&&*d=='/') + if (*d=='/') { if (dirPerPart!=pn) throw makeStringExceptionV(-1, "Dir-per-part # does not match part # of file %s", filename); @@ -1025,14 +1028,18 @@ static void parseFileName(const char *name,StringBuffer &mname,unsigned &num,uns else mname.append(cur-name,name); - if (pn&&(memicmp(s,"_of_",4)==0)) { + if (pn&&(memicmp(s,"_of_",4)==0)) + { unsigned mn = 0; s += 4; - while (*s&&isdigit(*s)) { + while (*s&&isdigit(*s)) + { mn = mn*10+(*s-'0'); s++; } - if ((mn!=0)&&((*s==0)||(*s=='.'))&&(mn>=pn)) { // NB allow trailing extension + if ((mn!=0)&&((*s==0)||(*s=='.'))&&(mn>=pn)) + { + // NB allow trailing extension mname.append("._$P$_of_").append(mn); if (*s) mname.append(s); @@ -3091,10 +3098,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture CPPUNIT_ASSERT_EQUAL_MESSAGE("Replicate is incorrect",true,replicate); // A file without a storage plane throws an exception - try { + try + { parseFileName("/test/myname._1_of_3", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3102,10 +3112,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3113,10 +3126,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3124,10 +3140,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file with a storage plane that has numDevices>1 but no stripe number in the path throws an exception - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/datadir/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3135,10 +3154,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file where the stripe number is equal to numDevices(111) throws an exception - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d111/test/myname.42_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3146,10 +3168,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file where the dir-per-part number does not match the part number - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/42/myname._43_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3157,10 +3182,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file where the part number is greater than the max part number - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/1000/myname._1000_of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); @@ -3168,10 +3196,13 @@ class DFUXrefLibTests : public CppUnit::TestFixture } // A file where the part number is missing - try { + try + { parseFileName("/var/lib/HPCCSystems/hpcc-data-two/d1/test/myname._of_100", mname.clear(), num, max, stripeNum, dirPerPart, replicate); CPPUNIT_ASSERT(false); - } catch (IException *e) { + } + catch (IException *e) + { StringBuffer msg; e->errorMessage(msg); e->Release(); From c49964c20dbf12e3dceb3aed967b08d9b30758db Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Tue, 17 Dec 2024 12:22:05 -0500 Subject: [PATCH 32/32] Rename deprecated version of constructPartFilename - Redoing this because it must have been lost in the rebase --- dali/base/dadfs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index ee03f9cb942..94d3cd6e90a 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -247,7 +247,7 @@ extern da_decl cost_type calcDiskWriteCost(const StringArray & clusters, stat_ty // Deprecated and should be removed and new feature tested -RemoteFilename &constructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,unsigned copy,ClusterPartDiskMapSpec &mspec,RemoteFilename &rfn) +RemoteFilename &deprecatedConstructPartFilename(IGroup *grp,unsigned partno,unsigned partmax,const char *name,const char *partmask,const char *partdir,unsigned copy,ClusterPartDiskMapSpec &mspec,RemoteFilename &rfn) { partno--; StringBuffer tmp;