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.

Sign up for GitHub

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

Jump to bottom

Oom protection #1321

Merged
merged 7 commits into from
Nov 8, 2023
Merged

Oom protection #1321

merged 7 commits into from
Nov 8, 2023

Conversation

DiegoTavares
Copy link
Collaborator

The current logic relies on hardcoded values which are not suitable for large hosts. The new logic takes into account the size of hosts and also tries to be more aggressive with misbehaving frames.

Prevent host from entering an OOM state where oom-killer might start killing important OS processes. The kill logic will kick in one of the following conditions is met:

  • Host has less than OOM_MEMORY_LEFT_THRESHOLD_PERCENT memory available
  • A frame is taking more than OOM_FRAME_OVERBOARD_PERCENT of what it had reserved For frames that are using more than they had reserved but not above the threshold, negotiate expanding the reservations with other frames on the same host.

Additional observations:

  • Setting OOM_FRAME_OVERBOARD_PERCENT to -1 will turn off the frame overboard protection feature.
  • Frames killed by this logic will be marked to be retried and will trigger the existing memory bump logic, in which the memory requirements for the layer will be increased to try to allocate a larger portion of memory for subsequent executions.
  • In rare occasions a frame can leave child processes behind. As rqd keeps track of procs related to the original frame, it will continue to report the Frame as active and cuebot will continue to request that it gets killed. To avoid this condition, there is a new logic that prevents repeated kill requests to be sent more then FRAME_KILL_RETRY_LIMIT times in 3 minutes (configurable).

@DiegoTavares
The current logic relies on hardcoded values which are not suitable f…
4377272 
…or large hosts. The new logic takes into account the size of hosts and also tries to be more aggressive with misbehaving frames.

Prevent host from entering an OOM state where oom-killer might start killing important OS processes. The kill logic will kick in one of the following conditions is met:

Host has less than OOM_MEMORY_LEFT_THRESHOLD_PERCENT memory available
A frame is taking more than OOM_FRAME_OVERBOARD_PERCENT of what it had reserved For frames that are using more than they had reserved but not above the threshold, negotiate expanding the reservations with other frames on the same host

(cherry picked from commit e88a5295f23bd927614de6d5af6a09d496d3e6ac)
@DiegoTavares
Frames killed for OOM should be retried
a4c832d 
(cherry picked from commit b88f7bcb1ad43f83fb8357576c33483dc2bf4952)
@DiegoTavares
OOM_FRAME_OVERBOARD_ALLOWED_THRESHOLD can be deactivated with -1
ee9748c 
(cherry picked from commit 647e75e2254c7a7ff68c544e438080f412bf04c1)
@DiegoTavares
Limit the number of kill retries
75506df 
There's an error condition on rqd where a frame that cannot be killed will end up preventing the host from picking up new jobs. This logic limits the number of repeated killRequests to give host a chance to pick up new jobs. At the same time, blocked frames are logged to spcue.log to be handled manually.

(cherry picked from commit aea4864ef66aca494fb455a7c103e4a832b63d41)
@DiegoTavares
Copy link
Collaborator Author

@bcipriano Ready for review

bcipriano
bcipriano requested changes Oct 5, 2023
View reviewed changes
Copy link
Collaborator

@bcipriano bcipriano left a comment

Choose a reason for hiding this comment

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

Overall looks good, some minor comments. Pretty complex but everything's broken out really nicely.

cuebot/src/main/java/com/imageworks/spcue/dao/FrameDao.java Outdated Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/DispatchSupport.java Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java Outdated Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java Outdated Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java Outdated Show resolved Hide resolved
cuebot/src/main/java/com/imageworks/spcue/dispatcher/commands/DispatchRqdKillFrameMemory.java Outdated Show resolved Hide resolved
cuebot/src/main/resources/conf/spring/applicationContext-service.xml Show resolved Hide resolved
@DiegoTavares
Copy link
Collaborator Author

Ready for another round

@DiegoTavares
Minor improvements to the logic
d11c968 
Signed-off-by: Diego Tavares <[email protected]>
@DiegoTavares DiegoTavares merged commit e3136f4 into AcademySoftwareFoundation:master Nov 8, 2023
10 of 11 checks passed
carlosfelgarcia pushed a commit to carlosfelgarcia/OpenCue that referenced this pull request May 22, 2024
@DiegoTavares @carlosfelgarcia
Oom protection (AcademySoftwareFoundation#1321)
ab9a9a8 
* The current logic relies on hardcoded values which are not suitable for large hosts. The new logic takes into account the size of hosts and also tries to be more aggressive with misbehaving frames.

Prevent host from entering an OOM state where oom-killer might start killing important OS processes. The kill logic will kick in one of the following conditions is met:

Host has less than OOM_MEMORY_LEFT_THRESHOLD_PERCENT memory available
A frame is taking more than OOM_FRAME_OVERBOARD_PERCENT of what it had reserved For frames that are using more than they had reserved but not above the threshold, negotiate expanding the reservations with other frames on the same host

(cherry picked from commit e88a5295f23bd927614de6d5af6a09d496d3e6ac)

* Frames killed for OOM should be retried

(cherry picked from commit b88f7bcb1ad43f83fb8357576c33483dc2bf4952)

* OOM_FRAME_OVERBOARD_ALLOWED_THRESHOLD can be deactivated with -1

(cherry picked from commit 647e75e2254c7a7ff68c544e438080f412bf04c1)

* Limit the number of kill retries

There's an error condition on rqd where a frame that cannot be killed will end up preventing the host from picking up new jobs. This logic limits the number of repeated killRequests to give host a chance to pick up new jobs. At the same time, blocked frames are logged to spcue.log to be handled manually.

(cherry picked from commit aea4864ef66aca494fb455a7c103e4a832b63d41)

* Fix merge conflicts

* Handle MR comments

* Minor improvements to the logic

Signed-off-by: Diego Tavares <[email protected]>

---------

Signed-off-by: Diego Tavares <[email protected]>
carlosfelgarcia pushed a commit to carlosfelgarcia/OpenCue that referenced this pull request May 22, 2024
@DiegoTavares @carlosfelgarcia
Oom protection (AcademySoftwareFoundation#1321)
f834ee7 
* The current logic relies on hardcoded values which are not suitable for large hosts. The new logic takes into account the size of hosts and also tries to be more aggressive with misbehaving frames.

Prevent host from entering an OOM state where oom-killer might start killing important OS processes. The kill logic will kick in one of the following conditions is met:

Host has less than OOM_MEMORY_LEFT_THRESHOLD_PERCENT memory available
A frame is taking more than OOM_FRAME_OVERBOARD_PERCENT of what it had reserved For frames that are using more than they had reserved but not above the threshold, negotiate expanding the reservations with other frames on the same host

(cherry picked from commit e88a5295f23bd927614de6d5af6a09d496d3e6ac)

* Frames killed for OOM should be retried

(cherry picked from commit b88f7bcb1ad43f83fb8357576c33483dc2bf4952)

* OOM_FRAME_OVERBOARD_ALLOWED_THRESHOLD can be deactivated with -1

(cherry picked from commit 647e75e2254c7a7ff68c544e438080f412bf04c1)

* Limit the number of kill retries

There's an error condition on rqd where a frame that cannot be killed will end up preventing the host from picking up new jobs. This logic limits the number of repeated killRequests to give host a chance to pick up new jobs. At the same time, blocked frames are logged to spcue.log to be handled manually.

(cherry picked from commit aea4864ef66aca494fb455a7c103e4a832b63d41)

* Fix merge conflicts

* Handle MR comments

* Minor improvements to the logic

Signed-off-by: Diego Tavares <[email protected]>

---------

Signed-off-by: Diego Tavares <[email protected]>
@ramonfigueiredo ramonfigueiredo mentioned this pull request Dec 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcipriano bcipriano bcipriano approved these changes

@gregdenton gregdenton Awaiting requested review from gregdenton

@jrray jrray Awaiting requested review from jrray

@smith1511 smith1511 Awaiting requested review from smith1511

@larsbijl larsbijl Awaiting requested review from larsbijl

@IdrisMiles IdrisMiles Awaiting requested review from IdrisMiles

@splhack splhack Awaiting requested review from splhack

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DiegoTavares @bcipriano