Skip to content

Commit

Permalink
Remove fixmissing flag
Browse files Browse the repository at this point in the history 
This flag controlling an internal process to the
superfile, arguably on the wrong place, was being
passed around many methods and exposed to the
directory and even FileServices.

The assumption that it should fail on some cases
and only warn on others is wrong. This method
should not attempt to bypass problems in the
DFS, as it'll only silently propagate the original
problem.

The behaviour now is to throw at the slightest
sign of trouble, only warning when the file is
foreign (external Dali) or the number of subfiles
is different. The latter might be an error, too,
but I'll leave it for later consideration.
  • Loading branch information
@rengolin
rengolin committed Jul 20, 2012
1 parent 2175827 commit 5f908b1
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 123 deletions.
159 changes: 65 additions & 94 deletions dali/base/dadfs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ protected: friend class CDistributedFile;
redirection.setown(createDFSredirection());
}

IDistributedFile *dolookup(const CDfsLogicalFileName &logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction,bool fixmissing, unsigned timeout);
IDistributedFile *dolookup(const CDfsLogicalFileName &logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction, unsigned timeout);

IDistributedFile *lookup(const char *_logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction,unsigned timeout);
IDistributedFile *lookup(const CDfsLogicalFileName &logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction,unsigned timeout);
Expand Down Expand Up @@ -912,7 +912,7 @@ protected: friend class CDistributedFile;
bool renamePhysical(const char *oldname,const char *newname,IMultiException *exceptions,IUserDescriptor *user);
void removeEmptyScope(const char *name);

IDistributedSuperFile *lookupSuperFile(const char *logicalname,IUserDescriptor *user,IDistributedFileTransaction *transaction,bool fixmissing=false,unsigned timeout=INFINITE);
IDistributedSuperFile *lookupSuperFile(const char *logicalname,IUserDescriptor *user,IDistributedFileTransaction *transaction,unsigned timeout=INFINITE);

int getFilePermissions(const char *lname,IUserDescriptor *user,unsigned auditflags);
int getNodePermissions(const IpAddress &ip,IUserDescriptor *user,unsigned auditflags);
Expand Down Expand Up @@ -1327,7 +1327,7 @@ class CDistributedFileTransaction: public CInterface, implements IDistributedFil
return ret;
}

IDistributedSuperFile *lookupSuperFile(const char *name, bool fixmissing,unsigned timeout)
IDistributedSuperFile *lookupSuperFile(const char *name, unsigned timeout)
{
IDistributedSuperFile *ret;
IDistributedFile * f = findFile(name);
Expand All @@ -1336,7 +1336,7 @@ class CDistributedFileTransaction: public CInterface, implements IDistributedFil
if (ret)
return LINK(ret);
}
ret = queryDistributedFileDirectory().lookupSuperFile(name,udesc,this,fixmissing,timeout);
ret = queryDistributedFileDirectory().lookupSuperFile(name,udesc,this,timeout);
if (!ret)
return NULL;
if (isactive) {
Expand Down Expand Up @@ -3859,40 +3859,6 @@ static unsigned findSubFileOrd(const char *name)
return NotFound;
}

struct SuperFileSubTreeCache
{
unsigned n;
IPropertyTree **subs;
SuperFileSubTreeCache(IPropertyTree *root,bool fixerr)
{
IArrayOf<IPropertyTree> todelete;
n=root->getPropInt("@numsubfiles");
subs = (IPropertyTree **)calloc(sizeof(IPropertyTree *),n);
Owned<IPropertyTreeIterator> subit = root->getElements("SubFile");
ForEach (*subit) {
IPropertyTree &sub = subit->query();
unsigned sn = sub.getPropInt("@num",0);
if ((sn>0)&&(sn<=n))
subs[sn-1] = &sub;
else {
const char *name = root->queryProp("OrigName");
if (!name)
name = "UNKNOWN";
WARNLOG("CDistributedSuperFile: SuperFile %s: corrupt subfile part number %d of %d",name,sn,n);
if (fixerr)
todelete.append(sub);
}
}
ForEachItemIn(i,todelete) {
root->removeTree(&todelete.item(i));
}
}
~SuperFileSubTreeCache()
{
free(subs);
}
};

class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>
{
void checkNotForeign()
Expand Down Expand Up @@ -4111,80 +4077,85 @@ class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>
return path.append("SubFile[@num=\"").append(idx+1).append("\"]");
}

void loadSubFiles(bool fixerr,IDistributedFileTransaction *transaction, unsigned timeout)
void loadSubFiles(IDistributedFileTransaction *transaction, unsigned timeout)
{
partscache.kill();
StringBuffer path;
StringBuffer subname;
subfiles.kill();

// First, find all subfiles listed in the superfile
unsigned n=root->getPropInt("@numsubfiles");
IPropertyTree **subs;
try {
SuperFileSubTreeCache subtrees(root,fixerr);
for (unsigned i=0;i<subtrees.n;i++) {
IPropertyTree *sub = subtrees.subs[i];
if (!sub) {
StringBuffer s;
s.appendf("CDistributedSuperFile: SuperFile %s: corrupt subfile file part %d cannot be found",logicalName.get(),i+1);
if (fixerr) {
WARNLOG("%s",s.str());
break;
}
throw MakeStringException(-1,"%s",s.str());
}
// MORE - use ArrayOf<IPropertyTree>
subs = (IPropertyTree **)calloc(sizeof(IPropertyTree *),n);
Owned<IPropertyTreeIterator> subit = root->getElements("SubFile");
// Find all reported indexes and bail on bad range
ForEach (*subit) {
IPropertyTree &sub = subit->query();
unsigned sn = sub.getPropInt("@num",0);
if ((sn>0)&&(sn<=n))
subs[sn-1] = &sub;
else
ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: corrupt subfile part number %d of %d",logicalName.get(),sn,n);
}
// Pre-check, to make sure we got all files in the range before we lock any sub-file
for (unsigned i=0;i<n;i++) {
if (!subs[i])
ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: corrupt subfile file part %d cannot be found",logicalName.get(),i+1);
}
// Now try to resolve them all (file/superfile) MORE - fix this with URIs
for (unsigned i=0;i<n;i++) {
IPropertyTree *sub = subs[i];
sub->getProp("@name",subname.clear());
Owned<IDistributedFile> subfile;
if (!fixerr)
subfile.setown(transaction?transaction->lookupFile(subname.str(),timeout):parent->lookup(subname.str(),udesc,false,transaction,timeout));
else {
try {
subfile.setown(transaction?transaction->lookupFile(subname.str(),timeout):parent->lookup(subname.str(),udesc,false,transaction,timeout));
}
catch (IException *e) {
// bit of a kludge to handle subfiles missing
subfile.setown(transaction?transaction->lookupSuperFile(subname.str(),fixerr,timeout):parent->lookupSuperFile(subname.str(),udesc,transaction,fixerr,timeout));
if (!subfile.get())
throw;
e->Release();

}
}
subfile.setown(transaction?transaction->lookupFile(subname.str(),timeout):parent->lookup(subname.str(),udesc,false,transaction,timeout));
if (!subfile.get())
subfile.setown(transaction?transaction->lookupSuperFile(subname.str(),timeout):parent->lookupSuperFile(subname.str(),udesc,transaction,timeout));
// Some files are ok not to exist
if (!subfile.get()) {
StringBuffer s;
s.appendf("CDistributedSuperFile: SuperFile %s is missing sub-file file %s",logicalName.get(),subname.str());
CDfsLogicalFileName tmpsub;
tmpsub.set(subname);
if (fixerr||tmpsub.isForeign()) {
WARNLOG("%s",s.str());
if (tmpsub.isForeign()) {
// MORE - This code is ugly
WARNLOG("CDistributedSuperFile: SuperFile %s's sub-file file %s is foreign, removing it from super",logicalName.get(),subname.str());
root->removeTree(sub);
for (unsigned j=i+1;j<subtrees.n; j++) {
sub = subtrees.subs[j];
for (unsigned j=i+1;j<n; j++) {
sub = subs[j];
if (sub)
sub->setPropInt("@num",j);
if (j==1) {
resetFileAttr(createPTreeFromIPT(sub->queryPropTree("Attr")));
}
subtrees.subs[j-1] = sub;
subtrees.subs[j] = NULL;
subs[j-1] = sub;
subs[j] = NULL;
}
subtrees.n--;
root->setPropInt("@numsubfiles",subtrees.n);
if ((i==0)&&(subtrees.n==0)) {
n--;
if ((i==0)&&(n==0)) {
resetFileAttr(getEmptyAttr());
}
i--; // will get incremented by for
continue;
}
throw MakeStringException(-1,"%s",s.str());
else
ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: corrupt subfile file part %d cannot be found",logicalName.get(),i+1);
}
subfiles.append(*subfile.getClear());
}
if (subfiles.ordinality()<subtrees.n)
if (subfiles.ordinality()<n)
{
WARNLOG("CDistributedSuperFile: SuperFile %s: less subfiles than expected: found %d, expecting %d",logicalName.get(), n, subfiles.ordinality());
root->setPropInt("@numsubfiles",subfiles.ordinality());
}
}
catch (IException *) {
partscache.kill();
subfiles.kill(); // one out, all out
free(subs);
throw;
}
free(subs);
}

void addItem(unsigned pos,IDistributedFile *_file)
Expand Down Expand Up @@ -4359,7 +4330,7 @@ class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>

IMPLEMENT_IINTERFACE;

void init(CDistributedFileDirectory *_parent, IPropertyTree *_root, const CDfsLogicalFileName &_name, IUserDescriptor* user, bool fixerr, IDistributedFileTransaction *transaction, unsigned timeout=INFINITE)
void init(CDistributedFileDirectory *_parent, IPropertyTree *_root, const CDfsLogicalFileName &_name, IUserDescriptor* user, IDistributedFileTransaction *transaction, unsigned timeout=INFINITE)
{
assertex(_name.isSet());
setUserDescriptor(udesc,user);
Expand All @@ -4371,26 +4342,26 @@ class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>
interleaved = atoi(val);
else
interleaved = strToBool(val)?1:0;
loadSubFiles(fixerr,transaction,timeout);
loadSubFiles(transaction,timeout);
}

CDistributedSuperFile(CDistributedFileDirectory *_parent, IPropertyTree *_root,const CDfsLogicalFileName &_name,IUserDescriptor* user)
{
init(_parent,_root,_name,user,false,NULL);
init(_parent,_root,_name,user,NULL);
}

CDistributedSuperFile(CDistributedFileDirectory *_parent, IRemoteConnection *_conn,const CDfsLogicalFileName &_name,IUserDescriptor* user, bool fixerr, IDistributedFileTransaction *transaction,bool fixmissing,unsigned timeout)
CDistributedSuperFile(CDistributedFileDirectory *_parent, IRemoteConnection *_conn,const CDfsLogicalFileName &_name,IUserDescriptor* user, IDistributedFileTransaction *transaction,unsigned timeout)
{
conn.setown(_conn);
init(_parent,conn->queryRoot(),_name,user,fixmissing,transaction,timeout);
init(_parent,conn->queryRoot(),_name,user,transaction,timeout);
}

CDistributedSuperFile(CDistributedFileDirectory *_parent,const CDfsLogicalFileName &_name, IUserDescriptor* user, bool fixerr, IDistributedFileTransaction *transaction)
CDistributedSuperFile(CDistributedFileDirectory *_parent,const CDfsLogicalFileName &_name, IUserDescriptor* user, IDistributedFileTransaction *transaction)
{
// temp super file
assertex(_name.isMulti());
Owned<IPropertyTree> tree = _name.createSuperTree();
init(_parent,tree,_name,user,false,transaction);
init(_parent,tree,_name,user,transaction);
}

~CDistributedSuperFile()
Expand Down Expand Up @@ -5068,7 +5039,7 @@ class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>
if (pos) {
DistributedFilePropertyLock lock(this);
if (lock.needsReload())
loadSubFiles(true,transaction,1000*60*10);
loadSubFiles(transaction,1000*60*10);
pos = subfiles.ordinality();
if (pos) {
do {
Expand Down Expand Up @@ -6380,12 +6351,12 @@ IDistributedFile *CDistributedFileDirectory::lookup(const char *_logicalname,IUs
return lookup(logicalname,user,writeattr,transaction,timeout);
}

IDistributedFile *CDistributedFileDirectory::dolookup(const CDfsLogicalFileName &_logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction,bool fixmissing,unsigned timeout)
IDistributedFile *CDistributedFileDirectory::dolookup(const CDfsLogicalFileName &_logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction,unsigned timeout)
{
const CDfsLogicalFileName *logicalname = &_logicalname;
if (logicalname->isMulti())
// don't bother checking because the sub file creation will
return new CDistributedSuperFile(this,*logicalname,user,true,transaction); // temp superfile
return new CDistributedSuperFile(this,*logicalname,user,transaction); // temp superfile
Owned<IDfsLogicalFileNameIterator> redmatch;
loop {
checkLogicalName(*logicalname,user,true,writeattr,true,NULL);
Expand Down Expand Up @@ -6423,7 +6394,7 @@ IDistributedFile *CDistributedFileDirectory::dolookup(const CDfsLogicalFileName
start = msTick();
unsigned elapsed;
try {
return new CDistributedSuperFile(this,fcl.detach(),*logicalname,user,true,transaction,fixmissing,SDS_SUB_LOCK_TIMEOUT);
return new CDistributedSuperFile(this,fcl.detach(),*logicalname,user,transaction,SDS_SUB_LOCK_TIMEOUT);
}
catch (IDFS_Exception *e) {
elapsed = msTick()-start;
Expand Down Expand Up @@ -6455,14 +6426,14 @@ IDistributedFile *CDistributedFileDirectory::dolookup(const CDfsLogicalFileName

IDistributedFile *CDistributedFileDirectory::lookup(const CDfsLogicalFileName &logicalname,IUserDescriptor *user,bool writeattr,IDistributedFileTransaction *transaction, unsigned timeout)
{
return dolookup(logicalname,user,writeattr,transaction,false,timeout);
return dolookup(logicalname,user,writeattr,transaction,timeout);
}

IDistributedSuperFile *CDistributedFileDirectory::lookupSuperFile(const char *_logicalname,IUserDescriptor *user,IDistributedFileTransaction *transaction, bool fixmissing, unsigned timeout)
IDistributedSuperFile *CDistributedFileDirectory::lookupSuperFile(const char *_logicalname,IUserDescriptor *user,IDistributedFileTransaction *transaction, unsigned timeout)
{
CDfsLogicalFileName logicalname;
logicalname.set(_logicalname);
IDistributedFile *file = dolookup(logicalname,user,false,transaction,fixmissing,timeout);
IDistributedFile *file = dolookup(logicalname,user,false,transaction,timeout);
if (file) {
IDistributedSuperFile *sf = file->querySuperFile();
if (sf)
Expand Down Expand Up @@ -6564,7 +6535,7 @@ class cCreateSuperFileAction: public CDFAction
{
logicalname.set(_flname);
// We *have* to make sure the file doesn't exist here
IDistributedSuperFile *sfile = parent->lookupSuperFile(logicalname.get(), user, transaction, false, SDS_SUB_LOCK_TIMEOUT);
IDistributedSuperFile *sfile = parent->lookupSuperFile(logicalname.get(), user, transaction, SDS_SUB_LOCK_TIMEOUT);
if (sfile) {
super.setown(sfile);
} else {
Expand Down
3 changes: 1 addition & 2 deletions dali/base/dadfs.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ interface IDistributedFileTransaction: extends IInterface
virtual bool active()=0;
virtual bool setActive(bool on)=0; // returns old value (used internally)
virtual IDistributedFile *lookupFile(const char *lfn,unsigned timeout=INFINITE)=0;
virtual IDistributedSuperFile *lookupSuperFile(const char *slfn,bool fixmissing=false,unsigned timeout=INFINITE)=0;
virtual IDistributedSuperFile *lookupSuperFile(const char *slfn,unsigned timeout=INFINITE)=0;
virtual IUserDescriptor *queryUser()=0;
virtual bool addDelayedDelete(const char *lfn,bool remphys,IUserDescriptor *user)=0; // used internally to delay deletes untill commit
virtual void addAction(CDFAction *action)=0; // internal
Expand Down Expand Up @@ -473,7 +473,6 @@ interface IDistributedFileDirectory: extends IInterface
virtual IDistributedSuperFile *createSuperFile(const char *logicalname,bool interleaved,bool ifdoesnotexist=false,IUserDescriptor *user=NULL,IDistributedFileTransaction *transaction=NULL) = 0;
virtual IDistributedSuperFile *lookupSuperFile(const char *logicalname,IUserDescriptor *user=NULL,
IDistributedFileTransaction *transaction=NULL, // transaction only used for looking up sub files
bool fixmissing=false, // used when removing
unsigned timeout=INFINITE

) = 0; // NB lookup will also return superfiles
Expand Down
Loading

0 comments on commit 5f908b1

Please sign in to comment.