-
Notifications
You must be signed in to change notification settings - Fork 303
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-32958 Roxie dynamic priority #19300
base: candidate-9.8.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -319,6 +319,7 @@ class CSharedOnceContext : public CInterfaceOf<ISharedOnceContext> | |
QueryOptions::QueryOptions() | ||
{ | ||
priority = 0; | ||
dynPriority = 0; | ||
timeLimit = defaultTimeLimit[0]; | ||
warnTimeLimit = defaultWarnTimeLimit[0]; | ||
|
||
|
@@ -358,6 +359,7 @@ QueryOptions::QueryOptions() | |
QueryOptions::QueryOptions(const QueryOptions &other) | ||
{ | ||
priority = other.priority; | ||
dynPriority = other.dynPriority; | ||
timeLimit = other.timeLimit; | ||
warnTimeLimit = other.warnTimeLimit; | ||
|
||
|
@@ -400,8 +402,18 @@ void QueryOptions::setFromWorkUnit(IConstWorkUnit &wu, const IPropertyTree *stat | |
updateFromWorkUnit(priority, wu, "priority"); | ||
if (stateInfo) | ||
updateFromContext(priority, stateInfo, "@priority"); | ||
timeLimit = defaultTimeLimit[priority]; | ||
warnTimeLimit = defaultWarnTimeLimit[priority]; | ||
dynPriority = priority; | ||
if ((int)priority < 0) | ||
{ | ||
// use LOW queue time limits ... | ||
timeLimit = defaultTimeLimit[0]; | ||
warnTimeLimit = defaultWarnTimeLimit[0]; | ||
} | ||
else | ||
{ | ||
timeLimit = defaultTimeLimit[priority]; | ||
warnTimeLimit = defaultWarnTimeLimit[priority]; | ||
} | ||
updateFromWorkUnit(timeLimit, wu, "timeLimit"); | ||
updateFromWorkUnit(warnTimeLimit, wu, "warnTimeLimit"); | ||
updateFromWorkUnitM(memoryLimit, wu, "memoryLimit"); | ||
|
@@ -486,6 +498,31 @@ void QueryOptions::setFromContext(const IPropertyTree *ctx) | |
if (ctx) | ||
{ | ||
// Note: priority cannot be set at context level | ||
// b/c this is after activities have been created, but we could | ||
// dynamically adj priority in the header activityId before sending | ||
int tmpPriority; | ||
updateFromContext(tmpPriority, ctx, "@priority", "_Priority"); | ||
if (tmpPriority > 1) | ||
tmpPriority = 1; | ||
if (tmpPriority < -1) | ||
tmpPriority = -1; | ||
|
||
if (tmpPriority < (int)priority) | ||
{ | ||
dynPriority = tmpPriority; | ||
if (dynPriority < 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably not worth it, but this could be commoned up with the code in setFromWorkunit e.g. a setDynamicPriority() function. |
||
{ | ||
// use LOW queue time limits ... | ||
timeLimit = defaultTimeLimit[0]; | ||
warnTimeLimit = defaultWarnTimeLimit[0]; | ||
} | ||
else | ||
{ | ||
timeLimit = defaultTimeLimit[dynPriority]; | ||
warnTimeLimit = defaultWarnTimeLimit[dynPriority]; | ||
} | ||
} | ||
|
||
updateFromContext(timeLimit, ctx, "@timeLimit", "_TimeLimit"); | ||
updateFromContext(warnTimeLimit, ctx, "@warnTimeLimit", "_WarnTimeLimit"); | ||
updateFromContextM(memoryLimit, ctx, "@memoryLimit", "_MemoryLimit"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,8 +80,8 @@ class QueryOptions | |
void setFromContext(const IPropertyTree *ctx); | ||
void setFromAgentLoggingFlags(unsigned loggingFlags); | ||
|
||
|
||
unsigned priority; | ||
mutable int dynPriority; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. technically should be an atomic, but unlikely to cause any grief. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be better as an unsigned so consistent with priority. |
||
unsigned timeLimit; | ||
unsigned warnTimeLimit; | ||
unsigned traceLimit; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be clearer to have some constants for MinQueryPriority and MaxQueryPriority and compare against those. (Comparing > 1 would be clearer to compare with > 2, since that is leaking the next test into the bounds test).