HPCC-11087 - Track subs swapped as part of swap/promote

SwapSuperFile and PromoteSuperFile were not tracking the subs
removed/added between supers in a transaction. As a consequence
if there were incompatibilities between old and new subs, then
the transaction mechanism spuriously detected the old+new as
incompatible and threw and error, even though they weren't in the
same super at the same time.

Signed-off-by: Jake Smith <jake.smith@lexisnexis.com>

dali/base/dadfs.cpp

@@ -849,6 +849,7 @@ interface IDistributedFileTransactionExt : extends IDistributedFileTransaction
     virtual void clearFiles()=0;
     virtual void noteAddSubFile(IDistributedSuperFile *super, const char *superName, IDistributedFile *sub) = 0;
     virtual void noteRemoveSubFile(IDistributedSuperFile *super, IDistributedFile *sub) = 0;
+    virtual void noteSuperSwap(IDistributedSuperFile *super1, IDistributedSuperFile *super2) = 0;
     virtual void clearSubFiles(IDistributedSuperFile *super) = 0;
     virtual void noteRename(IDistributedFile *file, const char *newName) = 0;
     virtual void validateAddSubFile(IDistributedSuperFile *super, IDistributedFile *sub, const char *subName) = 0;
@@ -1406,6 +1407,25 @@ public:
         if (trackedSuper)
             trackedSuper->clearSubs();
     }
+    virtual void noteSuperSwap(IDistributedSuperFile *super1, IDistributedSuperFile *super2)
+    {
+        CTransactionFile *trackedSuper1 = lookupTrackedFile(super1);
+        CTransactionFile *trackedSuper2 = lookupTrackedFile(super2);
+        assertex(trackedSuper1 && trackedSuper2);
+        ICopyArrayOf<IDistributedFile> super1Subs, super2Subs;
+        Owned<IDistributedFileIterator> iter = trackedSuper1->getSubFiles();
+        ForEach(*iter)
+            super1Subs.append(iter->query());
+        trackedSuper1->clearSubs();
+        iter.setown(trackedSuper2->getSubFiles());
+        ForEach(*iter)
+            super2Subs.append(iter->query());
+        trackedSuper2->clearSubs();
+        ForEachItemIn(s, super2Subs)
+            trackedSuper1->noteAddSubFile(&super2Subs.item(s));
+        ForEachItemIn(s2, super1Subs)
+            trackedSuper2->noteAddSubFile(&super1Subs.item(s2));
+    }
     void noteRename(IDistributedFile *file, const char *newName)
     {
         CTransactionFile *trackedFile = lookupTrackedFile(file);
@@ -4584,45 +4604,46 @@ class CDistributedSuperFile: public CDistributedFileBase<IDistributedSuperFile>
      */
     class cSwapFileAction: public CDFAction
     {
-        Linked<IDistributedSuperFile> parent;
-        Linked<IDistributedSuperFile> file;
-        StringAttr parentlname;
-        StringAttr filelname;
+        Linked<IDistributedSuperFile> super1, super2;
+        StringAttr super1Name, super2Name;
     public:
-        cSwapFileAction(const char *_parentlname,const char *_filelname)
-            : parentlname(_parentlname), filelname(_filelname)
+        cSwapFileAction(const char *_super1Name, const char *_super2Name)
+            : super1Name(_super1Name), super2Name(_super2Name)
         {
         }
         bool prepare()
         {
-            parent.setown(transaction->lookupSuperFile(parentlname));
-            if (!parent)
-                throw MakeStringException(-1,"swapSuperFile: SuperFile %s cannot be found",parentlname.get());
-            file.setown(transaction->lookupSuperFile(filelname));
-            if (!file)
+            super1.setown(transaction->lookupSuperFile(super1Name));
+            if (!super1)
+                throw MakeStringException(-1,"swapSuperFile: SuperFile %s cannot be found", super1Name.get());
+            super2.setown(transaction->lookupSuperFile(super2Name));
+            if (!super2)
             {
-                parent.clear();
-                throw MakeStringException(-1,"swapSuperFile: SuperFile %s cannot be found",filelname.get());
+                super1.clear();
+                throw MakeStringException(-1,"swapSuperFile: SuperFile %s cannot be found", super2Name.get());
             }
             // Try to lock all files
-            addFileLock(parent);
-            for (unsigned i=0; i<parent->numSubFiles(); i++)
-                addFileLock(&parent->querySubFile(i));
-            addFileLock(file);
-            for (unsigned i=0; i<file->numSubFiles(); i++)
-                addFileLock(&file->querySubFile(i));
+            addFileLock(super1);
+            for (unsigned i=0; i<super1->numSubFiles(); i++)
+                addFileLock(&super1->querySubFile(i));
+            addFileLock(super2);
+            for (unsigned i=0; i<super2->numSubFiles(); i++)
+                addFileLock(&super2->querySubFile(i));
             if (lock())
+            {
+                transaction->noteSuperSwap(super1, super2);
                 return true;
+            }
             unlock();
-            parent.clear();
-            file.clear();
+            super1.clear();
+            super2.clear();
             return false;
         }
         void run()
         {
-            CDistributedSuperFile *sf = QUERYINTERFACE(parent.get(),CDistributedSuperFile);
+            CDistributedSuperFile *sf = QUERYINTERFACE(super1.get(),CDistributedSuperFile);
             if (sf)
-                sf->doSwapSuperFile(file,transaction);
+                sf->doSwapSuperFile(super2,transaction);
         }
     };
 
@@ -7446,6 +7467,10 @@ class CRemoveSuperFileAction: public CDFAction
         {
             transaction->noteRemoveSubFile(super, sub);
         }
+        virtual void noteSuperSwap(IDistributedSuperFile *super1, IDistributedSuperFile *super2)
+        {
+            transaction->noteSuperSwap(super1, super2);
+        }
         virtual void clearSubFiles(IDistributedSuperFile *super)
         {
             transaction->clearSubFiles(super);

testing/unittests/dalitests.cpp

@@ -958,6 +958,19 @@ public:
     {
         setupDFS("trans");
 
+        unsigned timeout = 1000; // 1s
+
+        /* Make the meta info of one of the subfiles mismatch the rest, as subfiles are promoted through
+         * the super files, this should _not_ cause an issue, as no single super file will contain
+         * mismatched subfiles.
+        */
+        Owned<IDistributedFile> sub1 = dir.lookup("regress::trans::sub1", user, false, false, NULL, timeout);
+        assertex(sub1);
+        sub1->lockProperties();
+        sub1->queryAttributes().setPropBool("@local", true);
+        sub1->unlockProperties();
+        sub1.clear();
+
         Owned<IDistributedFileTransaction> transaction = createDistributedFileTransaction(user);
 
         // ===============================================================================
@@ -967,7 +980,6 @@ public:
         };
         bool delsub = false;
         bool createonlyone = true;
-        unsigned timeout = 1000; // 1s
         // ===============================================================================
         StringArray outlinked;
 
@@ -1038,14 +1050,14 @@ public:
             ASSERT(sub1.get() && "promote failed, sub1 was physically deleted");
         }
 
-        logctx.CTXLOG("Promote ([1,2], 4, 3) - fifth iteration, two in-files");
-        dir.promoteSuperFiles(3, sfnames, "regress::trans::sub1,regress::trans::sub2", delsub, createonlyone, user, timeout, outlinked);
+        logctx.CTXLOG("Promote ([2,3], 4, 3) - fifth iteration, two in-files");
+        dir.promoteSuperFiles(3, sfnames, "regress::trans::sub2,regress::trans::sub3", delsub, createonlyone, user, timeout, outlinked);
         {
             Owned<IDistributedSuperFile> sfile1 = dir.lookupSuperFile("regress::trans::super1", user, NULL, timeout);
             ASSERT(sfile1.get() && "promote failed, super1 doesn't exist");
             ASSERT(sfile1->numSubFiles() == 2 && "promote failed, super1 should have two subfiles");
-            ASSERT(strcmp(sfile1->querySubFile(0).queryLogicalName(), "regress::trans::sub1") == 0 && "promote failed, wrong name for sub1");
-            ASSERT(strcmp(sfile1->querySubFile(1).queryLogicalName(), "regress::trans::sub2") == 0 && "promote failed, wrong name for sub2");
+            ASSERT(strcmp(sfile1->querySubFile(0).queryLogicalName(), "regress::trans::sub2") == 0 && "promote failed, wrong name for sub1");
+            ASSERT(strcmp(sfile1->querySubFile(1).queryLogicalName(), "regress::trans::sub3") == 0 && "promote failed, wrong name for sub2");
             Owned<IDistributedSuperFile> sfile2 = dir.lookupSuperFile("regress::trans::super2", user, NULL, timeout);
             ASSERT(sfile2.get() && "promote failed, super2 doesn't exist");
             ASSERT(sfile2->numSubFiles() == 1 && "promote failed, super2 should have one subfile");