Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-29215 Add DFU Copy Ensure functionality #17929

Merged
merged 1 commit into from
Nov 17, 2023
Merged

HPCC-29215 Add DFU Copy Ensure functionality #17929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions dali/dfu/dfurun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,8 @@ class CDFUengine: public CInterface, implements IDFUengine
}
break;
}

bool ensureLfnAlreadyPublished = false;
// fill dstfile for commands that need it
switch (cmd) {
case DFUcmd_copymerge:
Expand Down Expand Up @@ -1542,6 +1544,14 @@ class CDFUengine: public CInterface, implements IDFUengine
Owned<IDistributedFile> oldfile = wsdfs::lookup(tmp.str(),userdesc,AccessMode::tbdWrite,false,false,nullptr,defaultPrivilegedUser,INFINITE);
if (oldfile)
{
if (options->getEnsure())
{
// logical file already exists.
ensureLfnAlreadyPublished = true;
dstFile.setown(oldfile.getClear());
dstName.set(tmp);
break;
}
StringBuffer reason;
bool canRemove = oldfile->canRemove(reason);
oldfile.clear();
Expand Down Expand Up @@ -1700,12 +1710,35 @@ class CDFUengine: public CInterface, implements IDFUengine
}
}
else {
fsys.copy(srcFile,dstFile,recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
if (!abortnotify.abortRequested()) {
if (needrep)
replicating = true;
bool performCopy = true;
if (options->getEnsure())
{
if (ensureLfnAlreadyPublished)
performCopy = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakesmith Just to clarify, this will not do a copy if the meta data is there, but a DFU operation adds the meta data last, so the parts should be there if the file was created by DFU?
I guess then the one scenario that might get confusing is if they first tried to get roxie to copy the file which would create the meta data, but roxie didn't and they tried to do an ensure.
I guess I'm wondering if we should still check dstFile->existsPhysicalPartFiles(0)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure.
If Roxie copied the meta data only, then yes, this 'ensure' would think the file was already there and skip the copy.

but roxie didn't and they tried to do an ensure

In that scenario, why didn't roxie copy physicals, and/or how do yo know if it's in progress and is currently doing so?
Even if it checked physicals, would they be there (with correct names) if Roxie was copying them, until it had finished copying?
@afishbeck

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afishbeck - refreshing reviewing status, per conversation above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking in hypotheticals and I'm not sure that scenario would happen in the real world. I'll finish review as is, we could always cover that case later if it did come up.

else
dstFile->attach(dstName.get(),userdesc);
{
if (dstFile->existsPhysicalPartFiles(0))
{
dstFile->attach(dstName.get(), userdesc);
performCopy = false;
}
}
if (!performCopy)
{
feedback.repmode=cProgressReporter::REPnone;
feedback.displaySummary(nullptr, 0);
Audit("COPYENSURE", userdesc, srcFile?srcName.str():nullptr, dstName.get());
}
}
if (performCopy)
{
fsys.copy(srcFile,dstFile,recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
if (!abortnotify.abortRequested()) {
if (needrep)
replicating = true;
else
dstFile->attach(dstName.get(),userdesc);
}
Audit("COPY",userdesc,srcFile?srcName.str():NULL,dstName.get());
}
}
Expand Down
12 changes: 11 additions & 1 deletion dali/dfu/dfuwu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2002,6 +2002,11 @@ class CDFUoptions: public CLinkedDFUWUchild, implements IDFUoptions
return queryRoot()->getPropInt("@overwrite")!=0;
}

bool getEnsure() const
{
return queryRoot()->getPropInt("@ensure")!=0;
}

DFUreplicateMode getReplicateMode(StringBuffer &cluster, bool &repeatlast,bool &onlyrepeated) const
{
repeatlast = false;
Expand Down Expand Up @@ -2146,7 +2151,7 @@ class CDFUoptions: public CLinkedDFUWUchild, implements IDFUoptions
queryRoot()->setPropInt("@throttle",val);
}

void setTransferBufferSize(unsigned val)
void setTransferBufferSize(size32_t val)
{
queryRoot()->setPropInt("@transferBufferSize",val);
}
Expand All @@ -2161,6 +2166,11 @@ class CDFUoptions: public CLinkedDFUWUchild, implements IDFUoptions
queryRoot()->setPropInt("@overwrite",val?1:0);
}

void setEnsure(bool val=true)
{
queryRoot()->setPropInt("@ensure",val?1:0);
}

void setReplicateMode(DFUreplicateMode val,const char *cluster=NULL,bool repeatlast=false,bool onlyrepeated=false)
{
queryRoot()->setPropInt("@replicatemode",(int)val);
Expand Down
2 changes: 2 additions & 0 deletions dali/dfu/dfuwu.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ interface IConstDFUoptions : extends IInterface
virtual size32_t getTransferBufferSize() const = 0;
virtual bool getVerify() const = 0;
virtual bool getOverwrite() const = 0;
virtual bool getEnsure() const = 0;
virtual DFUreplicateMode getReplicateMode(StringBuffer &cluster, bool &repeatlast,bool &onlyrepeated) const = 0;
virtual const char *queryPartFilter() const = 0;
virtual bool getKeepHeader() const = 0;
Expand Down Expand Up @@ -195,6 +196,7 @@ interface IDFUoptions : extends IConstDFUoptions
virtual void setTransferBufferSize(size32_t val) = 0;
virtual void setVerify(bool val=true) = 0;
virtual void setOverwrite(bool val=true) = 0;
virtual void setEnsure(bool val=true) = 0;
virtual void setReplicateMode(DFUreplicateMode val,const char *cluster=NULL,bool repeatlast=false,bool onlyrepeated=false) = 0;
virtual void setPartFilter(const char *filter) = 0; // format n,n-n,n etc
virtual void setKeepHeader(bool val=true) = 0;
Expand Down
1 change: 1 addition & 0 deletions esp/scm/ws_fs.ecm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ ESPrequest [nil_remove] Copy
string srcusername;
string srcpassword;
bool overwrite;
bool ensure;
bool replicate;
int ReplicateOffset(1);

Expand Down
1 change: 1 addition & 0 deletions esp/services/ws_fs/ws_fsService.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2790,6 +2790,7 @@ bool CFileSprayEx::onCopy(IEspContext &context, IEspCopy &req, IEspCopyResponse
wuFSpecDest->setLogicalName(dstname);
wuFSpecDest->setFileMask(fileMask.str());
wuOptions->setOverwrite(req.getOverwrite());
wuOptions->setEnsure(req.getEnsure());
wuOptions->setPreserveCompression(req.getPreserveCompression());
if (!req.getExpireDays_isNull())
wuOptions->setExpireDays(req.getExpireDays());
Expand Down
Loading