Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-31559 Add option to avoid renames (spray and thor for now) #18485

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 40 additions & 14 deletions helm/hpcc/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -342,7 +341,7 @@
}
},
"expert": {
"description": "Settings for developers, debugging and testing"
"$ref": "#/definitions/expert"
},
"misc": {
"description": "Miscellaneous settings",
Expand Down Expand Up @@ -1341,6 +1340,9 @@
"terminationGracePeriodSeconds": {
"$ref": "#/definitions/terminationGracePeriodSeconds",
"default": 3600
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -1429,6 +1431,9 @@
},
"hpa": {
"$ref": "#/definitions/hpa"
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -1466,6 +1471,9 @@
},
"terminationGracePeriodSeconds": {
"$ref": "#/definitions/terminationGracePeriodSeconds"
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -1543,8 +1551,7 @@
"$ref": "#/definitions/hpa"
},
"expert": {
"description": "Custom internal options usually reserved for internal testing",
"type": "object"
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -1634,6 +1641,9 @@
"type": "boolean",
"default": false,
"description": "Require SOAPCALL and HTTPCALL URLs are secrets or mapped to secrets"
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -2388,6 +2397,9 @@
},
"hpa": {
"$ref": "#/definitions/hpa"
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -2675,6 +2686,9 @@
},
"egress": {
"$ref" : "#/definitions/egress"
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -2714,7 +2728,8 @@
"env": {},
"annotations": {},
"labels": {},
"egress": {}
"egress": {},
"expert": {}
},
"additionalProperties": false
}
Expand Down Expand Up @@ -2762,7 +2777,8 @@
"labels": {},
"limit": {},
"cutoff": {},
"egress": {}
"egress": {},
"expert": {}
},
"additionalProperties": false
}
Expand Down Expand Up @@ -2790,7 +2806,8 @@
"labels": {},
"limit": {},
"cutoff": {},
"egress": {}
"egress": {},
"expert": {}
},
"additionalProperties": false
}
Expand All @@ -2816,7 +2833,8 @@
"labels": {},
"limit": {},
"cutoff": {},
"egress": {}
"egress": {},
"expert": {}
},
"additionalProperties": false
}
Expand Down Expand Up @@ -2850,7 +2868,8 @@
"resources": {},
"annotations": {},
"labels": {},
"egress": {}
"egress": {},
"expert": {}
},
"additionalProperties": false
},
Expand Down Expand Up @@ -2935,6 +2954,9 @@
},
"hpa": {
"$ref": "#/definitions/hpa"
},
"expert": {
"$ref": "#/definitions/expert"
}
}
},
Expand Down Expand Up @@ -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"
}
}
}
19 changes: 19 additions & 0 deletions system/jlib/jfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very minor, could check atomic value again after getting lock.

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;
}
Expand Down
2 changes: 1 addition & 1 deletion thorlcr/activities/thactivityutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions thorlcr/thorutil/thormisc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading