Bläddra i källkod

HPCC-8048 Add Remove Super File to transactions

Adding removal of super files inside transactions, with full
roll-back capabilities. Tests included.

Signed-off-by: Renato Golin <rengolin@hpccsystems.com>
Renato Golin 12 år sedan
förälder
incheckning
dd759d337b

+ 81 - 8
dali/base/dadfs.cpp

@@ -882,6 +882,7 @@ public:
     }
     IDistributedFile *createNew(IPropertyTree *tree,bool ignoregroup);
     IDistributedSuperFile *createSuperFile(const char *logicalname,IUserDescriptor *user,bool interleaved,bool ifdoesnotexist,IDistributedFileTransaction *transaction=NULL);
+    void removeSuperFile(const char *_logicalname, bool delSubs, IUserDescriptor *user, IDistributedFileTransaction *transaction);
 
     IDistributedFileIterator *getIterator(const char *wildname, bool includesuper,IUserDescriptor *user);
     IDFAttributesIterator *getDFAttributesIterator(const char *wildname, IUserDescriptor *user, bool recursive, bool includesuper,INode *foreigndali,unsigned foreigndalitimeout);
@@ -1025,6 +1026,12 @@ public:
         state = TAS_RETRY;
         unlock();
     }
+    // MORE: In the rare event of a commit failure, not all actions can be rolled back.
+    // Since all actions today occur during "run", and since commit phases does very little,
+    // this chance is minimal and will probably be caused by corrupted file descriptors.
+    // The problem is that the state of the sub removals and the order in which they occur might not
+    // be trivial on such a low level error, and there's no way to atomically do operations in SDS
+    // at present time. We need more thought about this.
     virtual void commit()
     {
         state = TAS_SUCCESS;
@@ -6555,7 +6562,7 @@ IDistributedFile *CDistributedFileDirectory::createNew(IFileDescriptor *fdesc,co
 /**
  * Creates a super-file within a transaction.
  */
-class cCreateSuperFileAction: public CDFAction
+class CCreateSuperFileAction: public CDFAction
 {
     CDfsLogicalFileName logicalname;
     CDistributedFileDirectory *parent;
@@ -6564,7 +6571,7 @@ class cCreateSuperFileAction: public CDFAction
     IPropertyTree *root;
     bool created;
 public:
-    cCreateSuperFileAction(IDistributedFileTransaction *_transaction,
+    CCreateSuperFileAction(IDistributedFileTransaction *_transaction,
                            CDistributedFileDirectory *_parent,
                            IUserDescriptor *_user,
                            const char *_flname,
@@ -6573,10 +6580,8 @@ public:
     {
         logicalname.set(_flname);
         // We *have* to make sure the file doesn't exist here
-        IDistributedSuperFile *sfile = parent->lookupSuperFile(logicalname.get(), user, transaction, SDS_SUB_LOCK_TIMEOUT);
-        if (sfile) {
-            super.setown(sfile);
-        } else {
+        super.setown(transaction->lookupSuperFile(logicalname.get(), SDS_SUB_LOCK_TIMEOUT));
+        if (!super) {
             // Create file and link to transaction, so subsequent lookups won't fail
             root = createPTree();
             root->setPropInt("@interleaved",interleaved?2:0); // this is ill placed
@@ -6585,7 +6590,7 @@ public:
         }
         addFileLock(super);
     }
-    virtual ~cCreateSuperFileAction() {}
+    virtual ~CCreateSuperFileAction() {}
     bool prepare()
     {
         // Attach the file to DFS, if wasn't there already
@@ -6616,6 +6621,52 @@ public:
     }
 };
 
+/**
+ * Removes a super-file within a transaction.
+ */
+class CRemoveSuperFileAction: public CDFAction
+{
+    CDfsLogicalFileName logicalname;
+    Linked<IDistributedSuperFile> super;
+    IUserDescriptor *user;
+    bool delSub;
+public:
+    CRemoveSuperFileAction(IDistributedFileTransaction *_transaction,
+                           IUserDescriptor *_user,
+                           const char *_flname,
+                           bool _delSub)
+        : CDFAction(_transaction), user(_user), delSub(_delSub)
+    {
+        logicalname.set(_flname);
+        // We *have* to make sure the file exists here
+        super.setown(transaction->lookupSuperFile(logicalname.get(), SDS_SUB_LOCK_TIMEOUT));
+        if (!super)
+            ThrowStringException(-1, "Super File %s doesn't exist in the file system", logicalname.get());
+        addFileLock(super);
+        // Adds actions to transactions before this one and gets executed only on commit
+        if (delSub)
+            super->removeSubFile(NULL, true, true, false, transaction);
+    }
+    virtual ~CRemoveSuperFileAction() {}
+    bool prepare()
+    {
+        if (lock())
+            return true;
+        unlock();
+        return false;
+    }
+    void run()
+    {
+        // Removing here would make it hard to re-attach the sub files on rollback (FIXME?)
+    }
+    void commit()
+    {
+        super->detach();
+        CDFAction::commit();
+    }
+};
+
+// MORE: This should be implemented in DFSAccess later on
 IDistributedSuperFile *CDistributedFileDirectory::createSuperFile(const char *_logicalname,IUserDescriptor *user, bool _interleaved,bool ifdoesnotexist,IDistributedFileTransaction *transaction)
 {
     CDfsLogicalFileName logicalname;
@@ -6643,13 +6694,35 @@ IDistributedSuperFile *CDistributedFileDirectory::createSuperFile(const char *_l
     }
 
     // action is owned by transaction (acquired on CDFAction's c-tor) so don't unlink or delete!
-    cCreateSuperFileAction *action = new cCreateSuperFileAction(localtrans,this,user,_logicalname,_interleaved);
+    CCreateSuperFileAction *action = new CCreateSuperFileAction(localtrans,this,user,_logicalname,_interleaved);
 
     localtrans->autoCommit();
 
     return localtrans->lookupSuperFile(_logicalname);
 }
 
+// MORE: This should be implemented in DFSAccess later on
+void CDistributedFileDirectory::removeSuperFile(const char *_logicalname, bool delSubs, IUserDescriptor *user, IDistributedFileTransaction *transaction)
+{
+    CDfsLogicalFileName logicalname;
+    logicalname.set(_logicalname);
+    checkLogicalName(logicalname,user,true,true,false,"have a superfile with");
+
+    // Create a local transaction that will be destroyed (but never touch the external transaction)
+    Linked<IDistributedFileTransaction> localtrans;
+    if (transaction) {
+        localtrans.set(transaction);
+    } else {
+        // TODO: Make it explicit in the API that a transaction is required
+        localtrans.setown(new CDistributedFileTransaction(user));
+    }
+
+    // action is owned by transaction (acquired on CDFAction's c-tor) so don't unlink or delete!
+    CRemoveSuperFileAction *action = new CRemoveSuperFileAction(localtrans, user, _logicalname, delSubs);
+
+    localtrans->autoCommit();
+}
+
 // MORE - this should go when remove file gets into transactions
 bool CDistributedFileDirectory::cannotRemove(CDfsLogicalFileName &dlfn,IUserDescriptor *user,StringBuffer &reason,bool ignoresub, unsigned timeoutms)
 {

+ 2 - 3
dali/base/dadfs.hpp

@@ -472,9 +472,8 @@ interface IDistributedFileDirectory: extends IInterface
     virtual IDistributedSuperFile *createSuperFile(const char *logicalname,IUserDescriptor *user,bool interleaved,bool ifdoesnotexist=false,IDistributedFileTransaction *transaction=NULL) = 0;
     virtual IDistributedSuperFile *lookupSuperFile(const char *logicalname,IUserDescriptor *user,
                                                     IDistributedFileTransaction *transaction=NULL, // transaction only used for looking up sub files
-                                                    unsigned timeout=INFINITE
-
-                                                ) = 0;  // NB lookup will also return superfiles 
+                                                    unsigned timeout=INFINITE) = 0;  // NB lookup will also return superfiles
+    virtual void removeSuperFile(const char *_logicalname, bool delSubs=false, IUserDescriptor *user=NULL, IDistributedFileTransaction *transaction=NULL)=0;
 
     virtual int getFilePermissions(const char *lname,IUserDescriptor *user,unsigned auditflags=0)=0; // see dasess for auditflags values
     virtual void setDefaultUser(IUserDescriptor *user)=0;

+ 12 - 7
plugins/fileservices/fileservices.cpp

@@ -1041,18 +1041,23 @@ FILESERVICES_API bool FILESERVICES_CALL fsSuperFileExists(ICodeContext *ctx, con
 
 FILESERVICES_API void FILESERVICES_CALL fsDeleteSuperFile(ICodeContext *ctx, const char *lsuperfn,bool deletesub)
 {
-    // Note because deleting a superfile, not within transaction (currently)
+    IDistributedFileTransaction *transaction = ctx->querySuperFileTransaction();
+    Linked<IUserDescriptor> udesc = ctx->queryUserDescriptor();
     Owned<IDistributedSuperFile> file;
     StringBuffer lsfn;
     bool found = lookupSuperFile(ctx, lsuperfn, file, false, lsfn, false);
+    file.clear(); // MORE: this should really be exists(file)
+    StringBuffer s("DeleteSuperFile ('");
+    s.append(lsfn).appendf("')");
     if (found) {
-        CheckNotInTransaction(ctx,"DeleteSuperFile");
-        if (deletesub)
-            file->removeSubFile(NULL,true,true,false);
-        file->detach();
+        queryDistributedFileDirectory().removeSuperFile(lsfn.str(), deletesub, udesc, transaction);
+        if (transaction->active())
+            s.append(" action added to transaction");
+        else
+            s.append(" done");
+    } else {
+        s.append(" file not found");
     }
-    StringBuffer s("DeleteSuperFile ('");
-    s.append(lsfn).appendf("') %s",found?"done":"not found");
     WUmessage(ctx,ExceptionSeverityInformation,NULL,s.str());
     if (found)
         AuditMessage(ctx,"DeleteSuperFile",lsfn.str());

+ 83 - 0
testing/ecl/key/superfile7.xml

@@ -0,0 +1,83 @@
+<Dataset name='Result 1'>
+</Dataset>
+<Dataset name='Result 2'>
+</Dataset>
+<Dataset name='Result 3'>
+</Dataset>
+<Dataset name='Result 4'>
+</Dataset>
+<Dataset name='Result 5'>
+ <Row><Result_5>true</Result_5></Row>
+</Dataset>
+<Dataset name='Result 6'>
+ <Row><Result_6>4</Result_6></Row>
+</Dataset>
+<Dataset name='Result 7'>
+ <Row><i>1</i><id>A</id></Row>
+ <Row><i>1</i><id>B</id></Row>
+ <Row><i>1</i><id>C</id></Row>
+ <Row><i>2</i><id>D</id></Row>
+ <Row><i>2</i><id>E</id></Row>
+ <Row><i>3</i><id>F</id></Row>
+ <Row><i>3</i><id>G</id></Row>
+ <Row><i>3</i><id>H</id></Row>
+</Dataset>
+<Dataset name='Result 8'>
+ <Row><Result_8>true</Result_8></Row>
+</Dataset>
+<Dataset name='Result 9'>
+ <Row><Result_9>4</Result_9></Row>
+</Dataset>
+<Dataset name='Result 10'>
+ <Row><i>1</i><id>A</id></Row>
+ <Row><i>1</i><id>B</id></Row>
+ <Row><i>1</i><id>C</id></Row>
+ <Row><i>2</i><id>D</id></Row>
+ <Row><i>2</i><id>E</id></Row>
+ <Row><i>3</i><id>F</id></Row>
+ <Row><i>3</i><id>G</id></Row>
+ <Row><i>3</i><id>H</id></Row>
+</Dataset>
+<Dataset name='Result 11'>
+ <Row><Result_11>true</Result_11></Row>
+</Dataset>
+<Dataset name='Result 12'>
+ <Row><Result_12>true</Result_12></Row>
+</Dataset>
+<Dataset name='Result 13'>
+ <Row><Result_13>true</Result_13></Row>
+</Dataset>
+<Dataset name='Result 14'>
+ <Row><Result_14>true</Result_14></Row>
+</Dataset>
+<Dataset name='Result 15'>
+ <Row><Result_15>true</Result_15></Row>
+</Dataset>
+<Dataset name='Result 16'>
+ <Row><Result_16>4</Result_16></Row>
+</Dataset>
+<Dataset name='Result 17'>
+ <Row><i>1</i><id>A</id></Row>
+ <Row><i>1</i><id>B</id></Row>
+ <Row><i>1</i><id>C</id></Row>
+ <Row><i>2</i><id>D</id></Row>
+ <Row><i>2</i><id>E</id></Row>
+ <Row><i>3</i><id>F</id></Row>
+ <Row><i>3</i><id>G</id></Row>
+ <Row><i>3</i><id>H</id></Row>
+</Dataset>
+<Dataset name='Result 18'>
+ <Row><Result_18>false</Result_18></Row>
+</Dataset>
+<Dataset name='Result 19'>
+ <Row><Result_19>false</Result_19></Row>
+</Dataset>
+<Dataset name='Result 20'>
+ <Row><Result_20>false</Result_20></Row>
+</Dataset>
+<Dataset name='Result 21'>
+ <Row><Result_21>false</Result_21></Row>
+</Dataset>
+<Dataset name='Result 22'>
+ <Row><Result_22>false</Result_22></Row>
+</Dataset>

+ 53 - 0
testing/ecl/key/superfile8.xml

@@ -0,0 +1,53 @@
+<Dataset name='Result 1'>
+</Dataset>
+<Dataset name='Result 2'>
+</Dataset>
+<Dataset name='Result 3'>
+</Dataset>
+<Dataset name='Result 4'>
+</Dataset>
+<Dataset name='Result 5'>
+ <Row><Result_5>false</Result_5></Row>
+</Dataset>
+<Dataset name='Result 6'>
+ <Row><Result_6>true</Result_6></Row>
+</Dataset>
+<Dataset name='Result 7'>
+ <Row><Result_7>true</Result_7></Row>
+</Dataset>
+<Dataset name='Result 8'>
+ <Row><Result_8>true</Result_8></Row>
+</Dataset>
+<Dataset name='Result 9'>
+ <Row><Result_9>true</Result_9></Row>
+</Dataset>
+<Dataset name='Result 10'>
+ <Row><Result_10>false</Result_10></Row>
+</Dataset>
+<Dataset name='Result 11'>
+ <Row><Result_11>true</Result_11></Row>
+</Dataset>
+<Dataset name='Result 12'>
+ <Row><Result_12>true</Result_12></Row>
+</Dataset>
+<Dataset name='Result 13'>
+ <Row><Result_13>true</Result_13></Row>
+</Dataset>
+<Dataset name='Result 14'>
+ <Row><Result_14>true</Result_14></Row>
+</Dataset>
+<Dataset name='Result 15'>
+ <Row><Result_15>false</Result_15></Row>
+</Dataset>
+<Dataset name='Result 16'>
+ <Row><Result_16>false</Result_16></Row>
+</Dataset>
+<Dataset name='Result 17'>
+ <Row><Result_17>false</Result_17></Row>
+</Dataset>
+<Dataset name='Result 18'>
+ <Row><Result_18>false</Result_18></Row>
+</Dataset>
+<Dataset name='Result 19'>
+ <Row><Result_19>false</Result_19></Row>
+</Dataset>

+ 87 - 0
testing/ecl/superfile7.ecl

@@ -0,0 +1,87 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2012 HPCC Systems.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+############################################################################## */
+
+import Std.System.Thorlib;
+import Std.File AS FileServices;
+import Std.Str;
+// Super File regression test
+//noRoxie
+
+rec :=
+RECORD
+        integer i;
+    string1 id;
+END;
+
+ds1 := DATASET([{1,'A'}, {1,'B'}, {1,'C'}], rec);
+ds2 := DATASET([{2,'D'}, {2,'E'}], rec);
+ds3 := DATASET([{3,'F'}, {3,'G'}, {3,'H'}], rec);
+ds4 := DATASET([],rec);
+
+clusterLFNPrefix := thorlib.getExpandLogicalName('regress::');
+
+string stripPrefix(string qlfn) := IF (Str.Find(qlfn, clusterLFNprefix, 1) = 1, Str.FindReplace(qlfn, clusterLFNPrefix, ''), qlfn);
+
+
+SEQUENTIAL(
+  // Prepare
+  FileServices.DeleteSuperFile('regress::superfile7'),
+  OUTPUT(ds1,,'regress::subfile1',overwrite),
+  OUTPUT(ds2,,'regress::subfile2',overwrite),
+  OUTPUT(ds3,,'regress::subfile3',overwrite),
+  OUTPUT(ds4,,'regress::subfile4',overwrite),
+  FileServices.StartSuperFileTransaction(),
+  FileServices.CreateSuperFile('regress::superfile7'),
+  FileServices.AddSuperFile('regress::superfile7','regress::subfile1'),
+  FileServices.AddSuperFile('regress::superfile7','regress::subfile2'),
+  FileServices.AddSuperFile('regress::superfile7','regress::subfile3'),
+  FileServices.AddSuperFile('regress::superfile7','regress::subfile4'),
+  FileServices.FinishSuperFileTransaction(),
+  OUTPUT(FileServices.SuperFileExists('regress::superfile7')), // true
+  OUTPUT(FileServices.GetSuperFileSubCount('regress::superfile7')), // 4
+  OUTPUT(dataset ('regress::superfile7', rec, flat)),
+
+  // Delete Super + Rollback (keep subs)
+  FileServices.StartSuperFileTransaction(),
+  FileServices.DeleteSuperFile('regress::superfile7'),
+  FileServices.FinishSuperFileTransaction(true),    // rollback
+  OUTPUT(FileServices.SuperFileExists('regress::superfile7')), // true
+  OUTPUT(FileServices.GetSuperFileSubCount('regress::superfile7')), // 4
+  OUTPUT(dataset ('regress::superfile7', rec, flat)),
+
+  // Delete Super + Rollback (del subs, not really)
+  FileServices.StartSuperFileTransaction(),
+  FileServices.DeleteSuperFile('regress::superfile7'),
+  FileServices.FinishSuperFileTransaction(true),    // rollback
+  OUTPUT(FileServices.SuperFileExists('regress::superfile7')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile1')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile2')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile3')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile4')), // true
+  OUTPUT(FileServices.GetSuperFileSubCount('regress::superfile7')), // 4
+  OUTPUT(dataset ('regress::superfile7', rec, flat)),
+
+  // Delete Super + Commit (del subs, yes really)
+  FileServices.StartSuperFileTransaction(),
+  FileServices.DeleteSuperFile('regress::superfile7', true), // del subs
+  FileServices.FinishSuperFileTransaction(),        // commit
+  OUTPUT(FileServices.SuperFileExists('regress::superfile7')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile1')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile2')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile3')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile4')), // false
+);

+ 92 - 0
testing/ecl/superfile8.ecl

@@ -0,0 +1,92 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2012 HPCC Systems.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+############################################################################## */
+
+import Std.System.Thorlib;
+import Std.File AS FileServices;
+import Std.Str;
+// Super File regression test
+//noRoxie
+
+rec :=
+RECORD
+        integer i;
+    string1 id;
+END;
+
+ds1 := DATASET([{1,'A'}, {1,'B'}, {1,'C'}], rec);
+ds2 := DATASET([{2,'D'}, {2,'E'}], rec);
+ds3 := DATASET([{3,'F'}, {3,'G'}, {3,'H'}], rec);
+ds4 := DATASET([],rec);
+
+clusterLFNPrefix := thorlib.getExpandLogicalName('regress::');
+
+string stripPrefix(string qlfn) := IF (Str.Find(qlfn, clusterLFNprefix, 1) = 1, Str.FindReplace(qlfn, clusterLFNPrefix, ''), qlfn);
+
+
+SEQUENTIAL(
+  // Prepare
+  FileServices.DeleteSuperFile('regress::superfile8'),
+  OUTPUT(ds1,,'regress::subfile5',overwrite),
+  OUTPUT(ds2,,'regress::subfile6',overwrite),
+  OUTPUT(ds3,,'regress::subfile7',overwrite),
+  OUTPUT(ds4,,'regress::subfile8',overwrite),
+
+  // Delete Super + Rollback (keep subs)
+  FileServices.StartSuperFileTransaction(),
+  FileServices.CreateSuperFile('regress::superfile8'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile5'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile6'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile7'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile8'),
+  FileServices.DeleteSuperFile('regress::superfile8'),
+  FileServices.FinishSuperFileTransaction(true),    // rollback
+  OUTPUT(FileServices.SuperFileExists('regress::superfile8')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile5')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile6')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile7')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile8')), // true
+
+  // Delete Super + Rollback (del subs, not really)
+  FileServices.StartSuperFileTransaction(),
+  FileServices.CreateSuperFile('regress::superfile8'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile5'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile6'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile7'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile8'),
+  FileServices.DeleteSuperFile('regress::superfile8'),
+  FileServices.FinishSuperFileTransaction(true),    // rollback
+  OUTPUT(FileServices.SuperFileExists('regress::superfile8')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile5')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile6')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile7')), // true
+  OUTPUT(FileServices.FileExists('regress::subfile8')), // true
+
+  // Delete Super + Commit (del subs, yes really)
+  FileServices.StartSuperFileTransaction(),
+  FileServices.CreateSuperFile('regress::superfile8'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile5'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile6'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile7'),
+  FileServices.AddSuperFile('regress::superfile8','regress::subfile8'),
+  FileServices.DeleteSuperFile('regress::superfile8', true), // del subs
+  FileServices.FinishSuperFileTransaction(),        // commit
+  OUTPUT(FileServices.SuperFileExists('regress::superfile8')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile5')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile6')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile7')), // false
+  OUTPUT(FileServices.FileExists('regress::subfile8')), // false
+);