Browse Source

HPCC-10413 - Reinstate non-sequential subfile orders

A regression was introduced way back in 3.10.0, which
meant that the orders of subfiles were ignored, meaning
AddSuperFile @ position, was effectively being ignored.
Worse than that, it also meant the internal representation
of order of subfiles differed from the DFS meta file order,
which consequently cause bugs like HPCC-10287.

This commit:
1) Reinstates ordered subfiles as defined by SubFile @num
2) Rollsback the previous 4.2 commit which asserted the SubFile's
must have sequential order
3) Rollsback the daliadmin checksuperfile addition for OOO
SubFile's
4) Removes spurious "subfile:" tracing added in HPCC-9218

Signed-off-by: Jake Smith <jake.smith@lexisnexis.com>
Jake Smith 11 years ago
parent
commit
ff47ef9210
2 changed files with 11 additions and 28 deletions
  1. 11 20
      dali/base/dadfs.cpp
  2. 0 8
      dali/daliadmin/daliadmin.cpp

+ 11 - 20
dali/base/dadfs.cpp

@@ -1311,13 +1311,6 @@ class CDistributedFileTransaction: public CInterface, implements IDistributedFil
                     }
                 }
             }
-            SuperHashIteratorOf<HTMapping> iter(subFilesByName);
-            ForEach(iter)
-            {
-                HTMapping &map = iter.query();
-                PROGLOG("subfile: %s", map.queryFindString());
-            }
-
             return false;
         }
         const void *queryFindParam() const { return &file; }
@@ -4792,11 +4785,12 @@ protected:
         unsigned n = root->getPropInt("@numsubfiles");
         if (n == 0)
             return;
-        try {
+        try
+        {
             // Find all reported indexes and bail on bad range (before we lock any file)
             Owned<IPropertyTreeIterator> subit = root->getElements("SubFile");
-            OwnedMalloc<unsigned> subFiles(n, true);
-            unsigned expectedN = 1;
+            // Adding a sub 'before' another get the list out of order (but still valid)
+            OwnedMalloc<IPropertyTree *> orderedSubFiles(n, true);
             ForEach (*subit)
             {
                 IPropertyTree &sub = subit->query();
@@ -4805,23 +4799,19 @@ protected:
                     ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: bad subfile part number %d of %d", logicalName.get(), sn, n);
                 if (sn > n)
                     ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: out-of-range subfile part number %d of %d", logicalName.get(), sn, n);
-                if (subFiles[sn-1])
+                if (orderedSubFiles[sn-1])
                     ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: duplicated subfile part number %d of %d", logicalName.get(), sn, n);
-                if (sn != expectedN)
-                    ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: bad part number %d, expected %d", logicalName.get(), sn, expectedN);
-                subFiles[sn-1] = 1;
-                ++expectedN;
+                orderedSubFiles[sn-1] = &sub;
             }
             for (unsigned i=0; i<n; i++)
             {
-                if (!subFiles[i])
+                if (!orderedSubFiles[i])
                     ThrowStringException(-1, "CDistributedSuperFile: SuperFile %s: missing subfile part number %d of %d", logicalName.get(), i+1, n);
             }
-
             // Now try to resolve them all (file/superfile)
-            ForEach (*subit)
+            for (unsigned f=0; f<n; f++)
             {
-                IPropertyTree &sub = subit->query();
+                IPropertyTree &sub = *(orderedSubFiles[f]);
                 sub.getProp("@name",subname.clear());
                 Owned<IDistributedFile> subfile;
                 subfile.setown(transaction?transaction->lookupFile(subname.str(),timeout):parent->lookup(subname.str(), udesc, false, false, transaction, timeout));
@@ -4852,7 +4842,8 @@ protected:
                 root->setPropInt("@numsubfiles", subfiles.ordinality());
             }
         }
-        catch (IException *) {
+        catch (IException *)
+        {
             partscache.kill();
             subfiles.kill();    // one out, all out
             throw;

+ 0 - 8
dali/daliadmin/daliadmin.cpp

@@ -1153,7 +1153,6 @@ static void checksuperfile(const char *lfn,bool fix=false)
         root->setPropInt("@numsubfiles",subnum);
     i = 0;
     byte fixstate = 0;
-    bool outOfSequence = false;
     loop {
         bool err = false;
         IPropertyTree *sub = root->queryPropTree(path.clear().appendf("SubFile[%d]",i+1).str());
@@ -1172,13 +1171,6 @@ static void checksuperfile(const char *lfn,bool fix=false)
                     i--;
                 }
             }
-            else if (pn != i+1) {
-                if (!outOfSequence)
-                    ERRLOG("SuperFile %s: corrupt, subfile file part @num values out of sequence, starting at part: %d", lname.get(), pn);
-                if (fix && (outOfSequence || doFix()))
-                    sub->setPropInt("@num", i+1);
-                outOfSequence = true;
-            }
         }
         else
             break;