-
Notifications
You must be signed in to change notification settings - Fork 183
[8.0] feat: PoolCE takes care of RAM requirements #8232
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
base: rel-v8r0
Are you sure you want to change the base?
Changes from all commits
184ff2d
305f91a
6a135e3
1d275db
66b0165
608d57f
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 |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
LocalCEType = Pool | ||
|
||
The Pool Computing Element is specific: it embeds an additional "inner" CE | ||
(`InProcess` by default, `Sudo`, `Singularity`). The "inner" CE can be specified such as:: | ||
(`InProcess` by default, or `Singularity`). The "inner" CE can be specified such as:: | ||
|
||
LocalCEType = Pool/Singularity | ||
|
||
|
@@ -19,24 +19,18 @@ | |
|
||
**Code Documentation** | ||
""" | ||
import functools | ||
import os | ||
import concurrent.futures | ||
import functools | ||
|
||
from DIRAC import S_OK, S_ERROR | ||
from DIRAC import S_ERROR, S_OK | ||
from DIRAC.ConfigurationSystem.private.ConfigurationData import ConfigurationData | ||
|
||
from DIRAC.Resources.Computing.ComputingElement import ComputingElement | ||
|
||
from DIRAC.Resources.Computing.InProcessComputingElement import InProcessComputingElement | ||
from DIRAC.Resources.Computing.SingularityComputingElement import SingularityComputingElement | ||
|
||
# Number of unix users to run job payloads with sudo | ||
MAX_NUMBER_OF_SUDO_UNIX_USERS = 32 | ||
|
||
|
||
def executeJob(executableFile, proxy, taskID, inputs, **kwargs): | ||
"""wrapper around ce.submitJob: decides which CE to use (Sudo or InProcess or Singularity) | ||
"""wrapper around ce.submitJob: decides which CE to use (InProcess or Singularity) | ||
|
||
:param str executableFile: location of the executable file | ||
:param str proxy: proxy file location to be used for job submission | ||
|
@@ -67,6 +61,8 @@ def __init__(self, ceUniqueID): | |
self.taskID = 0 | ||
self.processorsPerTask = {} | ||
self.userNumberPerTask = {} | ||
self.ram = 1024 # Default RAM in GB (this is an arbitrary large value in case of no limit) | ||
self.ramPerTask = {} | ||
|
||
# This CE will effectively submit to another "Inner"CE | ||
# (by default to the InProcess CE) | ||
|
@@ -80,22 +76,16 @@ def _reset(self): | |
|
||
self.processors = int(self.ceParameters.get("NumberOfProcessors", self.processors)) | ||
self.ceParameters["MaxTotalJobs"] = self.processors | ||
max_ram = int(self.ceParameters.get("MaxRAM", 0)) | ||
if max_ram > 0: | ||
self.ram = max_ram // 1024 # Convert from MB to GB | ||
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. Why don't we specify 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. The specification comes from BDII2CSAgent. I guess just "historical". Maybe MB should be used everywhere...? 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. I see thanks! 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. May be better GB everywhere. BDII2CSAgent can be updated if necessary. |
||
self.ceParameters["MaxRAM"] = self.ram | ||
# Indicates that the submission is done asynchronously | ||
# The result is not immediately available | ||
self.ceParameters["AsyncSubmission"] = True | ||
self.innerCESubmissionType = self.ceParameters.get("InnerCESubmissionType", self.innerCESubmissionType) | ||
return S_OK() | ||
|
||
def getProcessorsInUse(self): | ||
"""Get the number of currently allocated processor cores | ||
|
||
:return: number of processors in use | ||
""" | ||
processorsInUse = 0 | ||
for future in self.processorsPerTask: | ||
processorsInUse += self.processorsPerTask[future] | ||
return processorsInUse | ||
|
||
############################################################################# | ||
def submitJob(self, executableFile, proxy=None, inputs=None, **kwargs): | ||
"""Method to submit job. | ||
|
@@ -118,33 +108,34 @@ def submitJob(self, executableFile, proxy=None, inputs=None, **kwargs): | |
self.taskID += 1 | ||
return S_OK(taskID) | ||
|
||
# Now persisting the job limits for later use in pilot.cfg file (pilot 3 default) | ||
memoryForJob = self._getMemoryForJobs(kwargs) | ||
if memoryForJob is None: | ||
self.taskResults[self.taskID] = S_ERROR("Not enough memory for the job") | ||
taskID = self.taskID | ||
self.taskID += 1 | ||
return S_OK(taskID) | ||
|
||
# Now persisting the job limits for later use in pilot.cfg file | ||
cd = ConfigurationData(loadDefaultCFG=False) | ||
res = cd.loadFile("pilot.cfg") | ||
if not res["OK"]: | ||
self.log.error("Could not load pilot.cfg", res["Message"]) | ||
else: | ||
# only NumberOfProcessors for now, but RAM (or other stuff) can also be added | ||
jobID = int(kwargs.get("jobDesc", {}).get("jobID", 0)) | ||
cd.setOptionInCFG("/Resources/Computing/JobLimits/%d/NumberOfProcessors" % jobID, processorsForJob) | ||
cd.setOptionInCFG("/Resources/Computing/JobLimits/%d/MaxRAM" % jobID, memoryForJob) | ||
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. Is the intention to read this value in the innerCE to pass it to the CG2Manager? (It should just be a case of setting MemoryLimitMB in (a copy of) the ceParameters dictionary just before it's given to systemCall). 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.
yes. |
||
res = cd.dumpLocalCFGToFile("pilot.cfg") | ||
if not res["OK"]: | ||
self.log.error("Could not dump cfg to pilot.cfg", res["Message"]) | ||
|
||
# Here we define task kwargs: adding complex objects like thread.Lock can trigger errors in the task | ||
taskKwargs = {"InnerCESubmissionType": self.innerCESubmissionType} | ||
taskKwargs["jobDesc"] = kwargs.get("jobDesc", {}) | ||
if self.innerCESubmissionType == "Sudo": | ||
for nUser in range(MAX_NUMBER_OF_SUDO_UNIX_USERS): | ||
if nUser not in self.userNumberPerTask.values(): | ||
break | ||
taskKwargs["NUser"] = nUser | ||
if "USER" in os.environ: | ||
taskKwargs["PayloadUser"] = os.environ["USER"] + f"p{str(nUser).zfill(2)}" | ||
|
||
# Submission | ||
future = self.pPool.submit(executeJob, executableFile, proxy, self.taskID, inputs, **taskKwargs) | ||
self.processorsPerTask[future] = processorsForJob | ||
self.ramPerTask[future] = memoryForJob | ||
future.add_done_callback(functools.partial(self.finalizeJob, self.taskID)) | ||
|
||
taskID = self.taskID | ||
|
@@ -154,7 +145,7 @@ def submitJob(self, executableFile, proxy=None, inputs=None, **kwargs): | |
|
||
def _getProcessorsForJobs(self, kwargs): | ||
"""helper function""" | ||
processorsInUse = self.getProcessorsInUse() | ||
processorsInUse = sum(self.processorsPerTask.values()) | ||
availableProcessors = self.processors - processorsInUse | ||
|
||
self.log.verbose( | ||
|
@@ -191,6 +182,24 @@ def _getProcessorsForJobs(self, kwargs): | |
|
||
return requestedProcessors | ||
|
||
def _getMemoryForJobs(self, kwargs): | ||
"""helper function to get the memory that will be allocated for the job | ||
|
||
:param kwargs: job parameters | ||
:return: memory in GB or None if not enough memory | ||
""" | ||
|
||
# # job requirements | ||
requestedMemory = kwargs.get("MaxRAM", 0) | ||
|
||
# # now check what the slot can provide | ||
# Do we have enough memory? | ||
availableMemory = self.ram - sum(self.ramPerTask.values()) | ||
if availableMemory < requestedMemory: | ||
return None | ||
|
||
return requestedMemory | ||
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. I might have overlooked something, do we specify a default RAM value for the jobs? Because if I understand correctly, 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. There's no obvious default value for RAM (for processors we specify 1 which is a slightly more obvious default value). So, yes, as it stands the default value for RAM is 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. RAM and NumberOfProcessors should be treated in exactly the same way. These are resources with integer values, having default values and summing up for jobs on the same node. So, may be common methods (or class) can be introduced for those, may be other tags can be added eventually, e.g. disk space. 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. Having RAM in GBs isn't fine enough for an integer value, having less than 1GB per core is sensible on some systems and jobs. 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. Could this be made MB instead? |
||
|
||
def finalizeJob(self, taskID, future): | ||
"""Finalize the job by updating the process utilisation counters | ||
|
||
|
@@ -222,7 +231,7 @@ def getCEStatus(self): | |
result["WaitingJobs"] = 0 | ||
|
||
# dealing with processors | ||
processorsInUse = self.getProcessorsInUse() | ||
processorsInUse = sum(self.processorsPerTask.values()) | ||
result["UsedProcessors"] = processorsInUse | ||
result["AvailableProcessors"] = self.processors - processorsInUse | ||
return result | ||
|
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.
Defining this at the CE level isn't going to work very well (e.g. the HLTFarm has several different hardware configurations with wildly different RAM per core).
Can we inspect the host to have a sensible default?
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.
This is already at the "inner CE" level (this is a very common confusion). So, basically it is at the level of the Worker Node.