-
Notifications
You must be signed in to change notification settings - Fork 107
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
Support for setting maxPSS in specified tasks (Merge tasks) #12086
Comments
@LinaresToine before you dive too much with this development, I wanted to say that this is a delicate and intrusive change in Request Manager. Having the ability to update a Merge task resource requirement would ideally require knowledge of the task name (e.g. DataProcessingMergeNANOEDMAODoutput, which means knowledge of the parent task name and the output module), which is not kept at the request JSON description. So it's a complex issue to solve and easy to make mistakes and inject new bugs. If we decide to follow this route, then we need to be well covered with unit tests as well. About the |
Thank you @amaltaro for your illustration. I agree that this development is quite intrusive. Perhaps its better to include new functions, rather than modifying existing ones for the time being. For example, the task types that are being ignored could have their own setMaxPSS function. In any case, Ill think of a way to make it as less intrusive as possible. One idea that comes to mind is finding a way to define the Memory parameter as a dictionary with taskNames from the T0 side, as this is already supported by the agent. Ill follow up on this. Thanks again |
After discussing this issue with @amaltaro and @germanfgv , we have agreed to keep the memory value for merge tasks hardcoded in addRuntimeMonitors. The only change to make would be to change the For further record, this decision was made making this value configurable requires a lot of effort for something that rarely changes. |
Impact of the new feature
WMAgent
Is your feature request related to a problem? Please describe.
Express Merge jobs require more than the default 2.3 GiB defined in addRuntimeMonitors. The current memory setting functions ignore "Merge" tasks, which prevents them to ever have more than 2.3 GiB maxPSS.
Describe the solution you'd like
We want to be able to set an arbitrary value of maxPSS only for Express Merge tasks, or for that matter, any specified task. Currently, a workload goes through all its tasks and if its not "Merge", "Cleanup" or "LogCollect", it will set it with the same maxPSS value. We want to discriminate among tasks so that we can have "Merge" tasks with a maxPSS value of our choice without affecting the maxPSS value of other tasks within the workload.
Describe alternatives you've considered
I have considered giving flexibility to the current memory setting functions so that if a task is specified then there is no need to ignore it if it happens to be type "Merge". Also, adding a "TaskMemory" parameter support in company of the current "Memory" parameter in updateArguments. This is explicitly visible in modified updateArguments
Such changes can be seen in the patch: #12085
Additional context
None
The text was updated successfully, but these errors were encountered: