Skip to content

Commit 1d09e97

Browse files
streeterdstreeterd
committed
HPCC-36047 hthor refactor temporary file handling
New temporary file handling interface created. hthor changed to use the new interface. Signed-off-by: Dave Streeter <dave.streeter@lexisnexisrisk.com>
1 parent 5bb5c05 commit 1d09e97

4 files changed

Lines changed: 227 additions & 184 deletions

File tree

‎ecl/eclagent/agentctx.hpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,17 @@ interface IOrderedOutputSerializer;
8080
typedef enum { ofSTD, ofXML, ofRAW } outputFmts;
8181
enum class AccessMode : unsigned;
8282

83-
struct IAgentContext : extends IGlobalCodeContext
83+
interface ITempFileHandler
84+
{
85+
// name is the name/id of the temporary file, not the filename on disk.
86+
virtual const char *noteTemporaryFile(const char *name) = 0;
87+
// name is the name/id of the temporary file, not the filename on disk.
88+
virtual const char *queryTemporaryFile(const char *name) = 0;
89+
// fname is the temporary filename on disk returned from noteTemporaryFile or queryTemporaryFile.
90+
virtual void removeTemporaryFile(const char *fname) = 0;
91+
};
92+
93+
struct IAgentContext : extends IGlobalCodeContext, extends ITempFileHandler
8494
{
8595
virtual void reportProgress(const char *msg, unsigned flags=0) = 0;
8696
virtual bool queryResolveFilesLocally() = 0;
@@ -92,12 +102,9 @@ struct IAgentContext : extends IGlobalCodeContext
92102

93103
virtual IConstWorkUnit *queryWorkUnit() const = 0;
94104
virtual IWorkUnit *updateWorkUnit() const = 0;
95-
105+
96106
virtual ILocalOrDistributedFile *resolveLFN(const char *logicalName, const char *errorTxt, bool optional, bool noteRead, AccessMode accessMode, StringBuffer * expandedlfn, bool isPrivilegedUser) = 0;
97107
virtual StringBuffer & getTempfileBase(StringBuffer & buff) = 0;
98-
virtual const char *noteTemporaryFile(const char *fname) = 0;
99-
virtual const char *noteTemporaryFilespec(const char *fname) = 0;
100-
virtual const char *queryTemporaryFile(const char *fname) = 0;
101108
virtual void reloadWorkUnit() = 0;
102109

103110
virtual char *resolveName(const char *in, char *out, unsigned outlen) = 0;
@@ -110,14 +117,14 @@ struct IAgentContext : extends IGlobalCodeContext
110117
virtual void outputFormattedResult(const char *name, unsigned sequence, bool close) = 0;
111118

112119
virtual const char *queryAllowedPipePrograms() = 0;
113-
120+
114121
virtual IOrderedOutputSerializer * queryOutputSerializer() = 0;
115122

116123
virtual IGroup *getHThorGroup(StringBuffer &grpnameout) = 0;
117124

118125
virtual RecordTranslationMode getLayoutTranslationMode() const = 0;
119126
virtual unsigned __int64 queryStopAfter() = 0;
120-
127+
121128
virtual const char *queryWuid() = 0;
122129

123130
virtual void updateWULogfile(IWorkUnit *outputWU) = 0;

‎ecl/eclagent/eclagent.cpp‎

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -610,51 +610,85 @@ const char *EclAgent::queryTempfilePath()
610610
return agentTempDir.str();
611611
}
612612

613-
StringBuffer & EclAgent::getTempfileBase(StringBuffer & buff)
613+
StringBuffer &EclAgent::getTempfileBase(StringBuffer &buff)
614614
{
615615
return buff.append(queryTempfilePath()).append(PATHSEPCHAR).appendLower(wuid);
616616
}
617617

618-
const char *EclAgent::queryTemporaryFile(const char *fname)
618+
void EclAgent::buildTempFilename(StringBuffer &tempFilename, const char *name)
619619
{
620-
StringBuffer tempfilename;
621-
getTempfileBase(tempfilename).append(PATHSEPCHAR).append(fname);
622-
CriticalBlock crit(tfsect);
623-
ForEachItemIn(idx, tempFiles)
620+
getTempfileBase(tempFilename).append(PATHSEPCHAR).append(name);
621+
}
622+
623+
const char *EclAgent::queryTemporaryFile(const char *name)
624+
{
625+
dbgassertex(!isEmptyString(name));
626+
627+
StringBuffer tempFilename;
628+
buildTempFilename(tempFilename, name);
629+
624630
{
625-
if (strcmp(tempFiles.item(idx), tempfilename.str())==0)
626-
return tempFiles.item(idx);
631+
CriticalBlock crit(tfsect);
632+
633+
auto it = tempFileSet.find(tempFilename.str());
634+
if (it != tempFileSet.end())
635+
return it->c_str();
627636
}
628-
StringBuffer errmsg;
629-
errmsg.append("Attempt to read temp file that has not yet been registered: ").append(tempfilename);
637+
638+
VStringBuffer errmsg("Attempt to read temp file that has not yet been registered: %s", name);
630639
fail(0, errmsg.str());
631-
return 0;
640+
return nullptr;
632641
}
633642

634-
const char *EclAgent::noteTemporaryFile(const char *fname)
643+
const char *EclAgent::noteTemporaryFile(const char *name)
635644
{
636-
StringBuffer tempfilename;
637-
getTempfileBase(tempfilename).append(PATHSEPCHAR).append(fname);
638-
CriticalBlock crit(tfsect);
639-
tempFiles.append(tempfilename.str());
640-
return tempFiles.item(tempFiles.length()-1);
645+
dbgassertex(!isEmptyString(name));
646+
647+
StringBuffer tempFilename;
648+
buildTempFilename(tempFilename, name);
649+
650+
std::pair<std::set<std::string>::iterator, bool> inserted;
651+
{
652+
CriticalBlock crit(tfsect);
653+
654+
inserted = tempFileSet.emplace(tempFilename.str());
655+
}
656+
657+
// The returned pointer refers to the std::string stored in tempFileSet and
658+
// is only valid while that entry remains in the set.
659+
660+
if (!inserted.second)
661+
{
662+
VStringBuffer errmsg("Temp file already registered: %s", name);
663+
fail(0, errmsg.str());
664+
return nullptr;
665+
}
666+
667+
return inserted.first->c_str();
641668
}
642669

643-
const char *EclAgent::noteTemporaryFilespec(const char *fspec)
670+
void EclAgent::removeTemporaryFile(const char *fname)
644671
{
672+
dbgassertex(!isEmptyString(fname));
673+
645674
CriticalBlock crit(tfsect);
646-
tempFiles.append(fspec);
647-
return tempFiles.item(tempFiles.length()-1);
675+
676+
auto it = tempFileSet.find(std::string(fname));
677+
dbgassertex(it != tempFileSet.end());
678+
if (it != tempFileSet.end())
679+
{
680+
remove(it->c_str());
681+
tempFileSet.erase(it);
682+
}
648683
}
649684

650685
void EclAgent::deleteTempFiles()
651686
{
652687
CriticalBlock crit(tfsect);
653-
ForEachItemIn(idx, tempFiles)
654-
{
655-
remove(tempFiles.item(idx));
656-
}
657-
tempFiles.kill();
688+
689+
for (const auto& f : tempFileSet)
690+
remove(f.c_str());
691+
tempFileSet.clear();
658692
}
659693

660694
const char *EclAgent::loadResource(unsigned id)

‎ecl/eclagent/eclagent.ipp‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
#include "thorcommon.hpp"
3636
#include "enginecontext.hpp"
3737

38+
#include <set>
39+
3840
#define MAX_EDGEDATA_LENGTH 30000
3941
#define MAX_HEX_SIZE 500
4042

@@ -172,17 +174,17 @@ public:
172174
{
173175
return ctx->getTempfileBase(buff);
174176
}
175-
virtual const char *noteTemporaryFile(const char *fname)
177+
virtual const char *noteTemporaryFile(const char *name) override
176178
{
177-
return ctx->noteTemporaryFile(fname);
179+
return ctx->noteTemporaryFile(name);
178180
}
179-
virtual const char *noteTemporaryFilespec(const char *fspec)
181+
virtual const char *queryTemporaryFile(const char *name) override
180182
{
181-
return ctx->noteTemporaryFilespec(fspec);
183+
return ctx->queryTemporaryFile(name);
182184
}
183-
virtual const char *queryTemporaryFile(const char *fname)
185+
virtual void removeTemporaryFile(const char *fname) override
184186
{
185-
return ctx->queryTemporaryFile(fname);
187+
ctx->removeTemporaryFile(fname);
186188
}
187189
virtual void reloadWorkUnit()
188190
{
@@ -350,7 +352,6 @@ public:
350352
virtual bool onDebuggerTimeout();
351353
};
352354

353-
354355
class CHThorDebugContext;
355356
class EclAgent : implements IAgentContext, implements ICodeContext, implements IRowAllocatorMetaActIdCacheCallback, implements IEngineContext, public CInterface
356357
{
@@ -378,7 +379,7 @@ private:
378379
outputFmts outputFmt = ofSTD;
379380
unsigned __int64 stopAfter;
380381
mutable CriticalSection wusect;
381-
StringArray tempFiles;
382+
std::set<std::string> tempFileSet; // Set of actual temp file names on disk
382383
CriticalSection tfsect;
383384
IArray persistReadLocks;
384385
StringArray processedPersists;
@@ -404,6 +405,7 @@ private:
404405

405406
private:
406407
void doSetResultString(type_t type, const char * stepname, unsigned sequence, int len, const char *val);
408+
void buildTempFilename(StringBuffer & tempFilename, const char *filename);
407409
IEclProcess *loadProcess();
408410
StringBuffer & getTempfileBase(StringBuffer & buff);
409411
const char *queryTempfilePath();
@@ -615,9 +617,9 @@ public:
615617

616618
virtual unsigned __int64 getDatasetHash(const char * name, unsigned __int64 hash);
617619
virtual void reportProgress(const char *msg, unsigned flags=0);
618-
virtual const char *noteTemporaryFile(const char *fname);
619-
virtual const char *noteTemporaryFilespec(const char *fspec);
620-
virtual const char *queryTemporaryFile(const char *fname);
620+
virtual const char *noteTemporaryFile(const char *name) override;
621+
virtual const char *queryTemporaryFile(const char *name) override;
622+
virtual void removeTemporaryFile(const char *fname) override;
621623
virtual void deleteFile(const char * logicalName);
622624

623625
void addException(ErrorSeverity severity, const char * source, unsigned code, const char * text, const char * filename, unsigned lineno, unsigned column, bool failOnError, bool isAbort);

0 commit comments

Comments
 (0)