Merge pull request #18485 from jakesmith/HPCC-31559-opt-norename

HPCC-31559 Add option to avoid renames (spray and thor for now) Reviewed-by: Mark Kelly [email protected] Reviewed-by: Gavin Halliday <[email protected]> Merged-by: Gavin Halliday <[email protected]>
hpcc-systems · Apr 5, 2024 · 72701f1 · 72701f1
2 parents fd6a9c6 + 6f7f709
commit 72701f1
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 15 deletions.
diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json
@@ -232,8 +232,7 @@
               "$ref" : "#/definitions/egress"
           },
           "expert": {
-            "description": "Custom internal options usually reserved for internal testing",
-            "type": "object"
+            "$ref": "#/definitions/expert"
           },
           "hpa": {
             "$ref": "#/definitions/hpa"
@@ -342,7 +341,7 @@
           }
         },
         "expert": {
-          "description": "Settings for developers, debugging and testing"
+          "$ref": "#/definitions/expert"
         },
         "misc": {
           "description": "Miscellaneous settings",
@@ -1341,6 +1340,9 @@
         "terminationGracePeriodSeconds": {
           "$ref": "#/definitions/terminationGracePeriodSeconds",
           "default": 3600
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -1429,6 +1431,9 @@
         },
         "hpa": {
           "$ref": "#/definitions/hpa"
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -1466,6 +1471,9 @@
         },
         "terminationGracePeriodSeconds": {
           "$ref": "#/definitions/terminationGracePeriodSeconds"
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -1543,8 +1551,7 @@
           "$ref": "#/definitions/hpa"
         },
         "expert": {
-          "description": "Custom internal options usually reserved for internal testing",
-          "type": "object"
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -1634,6 +1641,9 @@
           "type": "boolean",
           "default": false,
           "description": "Require SOAPCALL and HTTPCALL URLs are secrets or mapped to secrets"
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -2339,8 +2349,7 @@
           "description": "Preemptively update secrets that are active and about to expire in the background"
         },
         "expert": {
-          "description": "Custom internal options usually reserved for internal testing",
-          "type": "object"
+          "$ref": "#/definitions/expert"
         },
         "egress": {
           "$ref" : "#/definitions/egress"
@@ -2388,6 +2397,9 @@
         },
         "hpa": {
           "$ref": "#/definitions/hpa"
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -2581,8 +2593,7 @@
           "$ref" : "#/definitions/componentCost"
         },
         "expert": {
-          "description": "Custom internal options usually reserved for internal testing",
-          "type": "object"
+          "$ref": "#/definitions/expert"
         },
         "egress": {
           "$ref" : "#/definitions/egress"
@@ -2675,6 +2686,9 @@
         },
         "egress": {
           "$ref" : "#/definitions/egress"
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -2714,7 +2728,8 @@
             "env": {},
             "annotations": {},
             "labels": {},
-            "egress": {}
+            "egress": {},
+            "expert": {}
           },
           "additionalProperties": false
         }
@@ -2762,7 +2777,8 @@
             "labels": {},
             "limit": {},
             "cutoff": {},
-            "egress": {}    
+            "egress": {},
+            "expert": {}
           },
           "additionalProperties": false
         }
@@ -2790,7 +2806,8 @@
             "labels": {},
             "limit": {},
             "cutoff": {},
-            "egress": {}
+            "egress": {},
+            "expert": {}
           },
           "additionalProperties": false
         }
@@ -2816,7 +2833,8 @@
             "labels": {},
             "limit": {},
             "cutoff": {},
-            "egress": {}
+            "egress": {},
+            "expert": {}
           },
           "additionalProperties": false
         }
@@ -2850,7 +2868,8 @@
         "resources": {},
         "annotations": {},
         "labels": {},
-        "egress": {}
+        "egress": {},
+        "expert": {}
       },
       "additionalProperties": false
     },
@@ -2935,6 +2954,9 @@
         },
         "hpa": {
           "$ref": "#/definitions/hpa"
+        },
+        "expert": {
+          "$ref": "#/definitions/expert"
         }
       }
     },
@@ -3364,6 +3386,10 @@
       "description": "Period permitted for component to terminate gracefully before being killed by Kubernetes",
       "default": 600,
       "minimum": 0
+    },
+    "expert": {
+      "description": "Settings for developers, debugging and testing",
+      "type": "object"
     }
   }
 }
diff --git a/system/jlib/jfile.cpp b/system/jlib/jfile.cpp
@@ -7423,12 +7423,31 @@ void FileIOStats::trace()
 
 static constexpr FileSystemProperties linuxFileSystemProperties     {true, true, true, true, 0x10000};             // 64K
 static constexpr FileSystemProperties defaultUrlFileSystemProperties{false, false, false, false, 0x400000};        // 4Mb
+static constexpr FileSystemProperties linuxFileSystemNoRenameProperties{false, true, true, true, 0x10000};         // 64K
+
+static std::atomic<int> avoidRename{-1};
+static CriticalSection avoidRenameCS;
+static bool isAvoidRenameEnabled()
+{
+    if (-1 == avoidRename)
+    {
+        CriticalBlock b(avoidRenameCS);
+        if (-1 == avoidRename)
+        {
+            avoidRename = getComponentConfigSP()->getPropBool("expert/@avoidRename");
+            DBGLOG("FileSystemProperties.canRename = %s", boolToStr(!avoidRename)); // NB: canRename if !avoidRename
+        }
+    }
+    return avoidRename;
+}
 
 //This implementation should eventually make use of the file hook.
 const FileSystemProperties & queryFileSystemProperties(const char * filename)
 {
     if (isUrl(filename))
         return defaultUrlFileSystemProperties;
+    else if (isAvoidRenameEnabled())
+        return linuxFileSystemNoRenameProperties;
     else
         return linuxFileSystemProperties;
 }

diff --git a/thorlcr/activities/thactivityutil.cpp b/thorlcr/activities/thactivityutil.cpp
@@ -733,7 +733,7 @@ IFileIO *createMultipleWrite(CActivityBase *activity, IPartDescriptor &partDesc,
     partDesc.getFilename(0, rfn);
     StringBuffer primaryName;
     rfn.getPath(primaryName);
-    if (isUrl(primaryName))
+    if (isUrl(primaryName) || activity->getOptBool(THOROPT_AVOID_RENAME)) // THOROPT_AVOID_RENAME see HPCC-31559
     {
         twFlags &= ~TW_RenameToPrimary;
         twFlags |= TW_Direct;

diff --git a/thorlcr/thorutil/thormisc.hpp b/thorlcr/thorutil/thormisc.hpp
@@ -115,6 +115,7 @@
 #define THOROPT_SOAP_TRACE_LEVEL "soapTraceLevel"               // The trace SOAP level (default=1)
 #define THOROPT_SORT_ALGORITHM "sortAlgorithm"                  // The algorithm used to sort records (quicksort/mergesort)
 #define THOROPT_COMPRESS_ALLFILES "compressAllOutputs"          // Compress all output files (default: bare-metal=off, cloud=on)
+#define THOROPT_AVOID_RENAME "avoidRename"                      // Avoid rename, write directly to target physical filenames (no temp file)
 
 
 #define INITIAL_SELFJOIN_MATCH_WARNING_LEVEL 20000  // max of row matches before selfjoin emits warning