-
Notifications
You must be signed in to change notification settings - Fork 184
[9.0] more on RAM matching #8414
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
[9.0] more on RAM matching #8414
Conversation
7801c52 to
76938f4
Compare
24d38b8 to
7e9bf4a
Compare
| except IndexError: # we are in Jenkins | ||
| J.setInputSandbox([find_all("exe-script-with-input.py", "/home/dirac", "DIRAC/tests/Workflow")[0]]) | ||
| J.setExecutable("exe-script-with-input.py", "", "helloWorld.log") | ||
| J.setInputData(["/dteam/user/f/fstagni/test/testInputFile.txt"]) |
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.
Out of curiosity, where do this file and testInputFileSingleLocation.txt come from?
Do we have access to them?
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.
I simply uploaded myself...
What do you mean with "access"? If another user can access it? I believe yes.
| resourceDict[name] = resourceDescription[name] | ||
|
|
||
| for name in multiValueMatchFields: | ||
| for name in singleValueDefFields + multiValueMatchFields + ("MaxRAM",): |
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.
Any reason for not having a rangeValueMatchFields in TaskQueueDB that would contain MaxRAM?
So that we avoid having hardcoded value like MaxRAM spread throughout the code.
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.
rangeValueMatchFields also includes MinRAM, which here is not needed.
| nProcessors = int(nProcessors) | ||
| except ValueError: | ||
| nProcessors = None | ||
| for param, key, limit, increment in [(maxRAM, "MB", 1024 * 1024, 256), (nProcessors, "Processors", 1024, 1)]: |
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.
I'm curious, don't you need that mechanism anymore?
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.
Update: I think I get it, it's because you now define range values, right?
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.
The mechanism is still there, just it's only needed for Processors (since RAM requirements are not stored anymore as tags).
| if not retVal["OK"]: | ||
| # If table doesn't exist (e.g., old installation), log a warning but continue | ||
| # This provides backward compatibility | ||
| if "doesn't exist" in retVal["Message"] or "Table" in retVal["Message"]: |
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.
That sounds a bit fragile but I guess you don't have much choice.
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.
It's indeed a bit fragile, but already the chances of hitting this case are close to 0, as the table SHOULD be existing here (I am tempted to actually remove this check...).
7e9bf4a to
d3c8663
Compare
BEGINRELEASENOTES
*WMS
NEW: added a new table to TaskQueueDB for RAM requirements and matching
ENDRELEASENOTES
The new table should be automatically created when the DB class is instantiated (at service restart)