Cuebot reserve all cores #1313

KernAttila · 2023-08-26T01:07:20Z

Link the Issue(s) this Pull Request is related to.
Fixes #1297

Summarize your change.
As in many render engines, we should be able to set a negative core requirement.
minCores=8 > reserve 8 cores
minCores=0 > reserve all cores
minCores=-2 > reserve all cores minus 2

This PR addresses this feature by handling negative core requests.
Cuebot will try to match this number against the number of cores on each host.
The frame will be booked only if all cores are available in this scenario.
If the host is busy (even slightly), the frame is not booked, to avoid filling the remaining cores.

Testing
I would need some guidance to create proper tests for cuebot.

Screenshot

Update:
There is now a "ALL" text for zero cores, or "ALL (-2)" for negative cores reservation.

(cuesubmit feature in another PR #1284)

…t. Let it pass when they match.

…nt. Let it pass when it matches.

…o be used during dispatch

…o be used during dispatch (LocalHost)

…. To be removed

…lues.

…as not threadable.

…res()

…uest is <= 0

doc: added some comments doc: Added some documentation doc: fix docstrings and parameters doc: added debug message doc: explain why we allow negative value doc: update debug message for rqd

… inserting proc

…alculation

…o cuebot-reserve-all-cores

bcipriano · 2023-09-27T21:48:51Z

Very interesting addition, thanks for sending this!

I haven't dug into this much yet but I brought it up during the TSC meeting, we're going to discuss some more.

One thing to note -- we'll definitely want to wrap this in a flag so it can be turned on and off. Scheduling changes are always tricky and we don't want to disrupt other folks too much.

bcipriano · 2023-09-27T21:49:21Z

@DiegoTavares Let me know if y'all have thoughts on this.

KernAttila · 2023-11-09T09:45:48Z

@bcipriano Would you recommand it to be on or off by default ?

DiegoTavares

IMO we should default to keeping this feature disabled. Very well implemented and an interesting feature to have.

Would you mind writing a blog post explaining the concept and how to use it?

KernAttila · 2024-05-21T17:31:50Z

IMO we should default to keeping this feature disabled. Very well implemented and an interesting feature to have.

Would you mind writing a blog post explaining the concept and how to use it?

Thanks for the review.

I don't have access to a Linux farm at the moment, but we are doing a Windows set-up very soon.
I'll need a bit more time for this one, and I will try to have that blog post ;)

DiegoTavares · 2024-05-21T17:52:07Z

Thanks Kern

…

_____________________________________ Diego Tavares da Silva ***@***.*** ***@***.***

On Tue, May 21, 2024 at 10:32 AM Kern Attila GERMAIN < ***@***.***> wrote: IMO we should default to keeping this feature disabled. Very well implemented and an interesting feature to have. Would you mind writing a blog post explaining the concept and how to use it? Thanks for the review. I don't have access to a Linux farm at the moment, but we are doing a Windows set-up very soon. I'll need a bit more time for this one, and I will try to have that blog post ;) — Reply to this email directly, view it on GitHub <#1313 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGXQ22BGDF3GFM45ZK7CT3ZDOAJ3AVCNFSM6AAAAAA37FUL5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRTGEYTCMBVGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

…o cuebot-reserve-all-cores # Conflicts: # cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java

Signed-off-by: Kern Attila GERMAIN <[email protected]>

…ting "ALL" or "ALL (-x)".

… the new negative cores feature.

…ll-cores

KernAttila · 2024-09-21T02:03:41Z

@bcipriano
As discussed with the team in the last meeting, the logic doesn't change the behavior of current submissions.
Launching with zero or negative cores before this feature seems unlikely.
We figured that disabling this feature with a flag would mean introducing a lot of code, with no added value or safety for current setups.
Instead, we settled on changing the layer's "cores" column display in cuegui to "ALL" for 0 and "ALL (-2.00)" for negative values (see the last screenshot in the description).

Would that work for you ?

KernAttila and others added 30 commits May 31, 2023 12:49

fix: only skip cores when the number is strictly under the requiremen…

48758fb

…t. Let it pass when they match.

fix: only skip memory when the amount is strictly under the requireme…

8f80240

…nt. Let it pass when it matches.

feat: handle negative cores request

7a3917d

fix: missing semicolon

da57b2e

fix: error unclosed string literal

78fb71d

feat: handle negative frame cores requirement in a public function, t…

fd4a040

…o be used during dispatch

fix: handle negative cores requirement during dispatch

263b248

fix: handle negative cores requirement during reservation

00f9111

fix: handle negative cores requirement during report

5c9dcf4

feat: handle negative frame cores requirement in a public function, t…

fd17206

…o be used during dispatch (LocalHost)

dev: add debug to find out how cores are reserved with negative value…

627274a

…. To be removed

fix: cores are counted by 100

7e8c48e

fix: do not set minimum if below threshold. we now accept negative va…

e5a77fe

…lues.

fix: threadable only if core != 100

ef2b627

fix: test if minimumCores is between 0 and 100 (excluded) to mark it …

902d30a

…as not threadable.

dbg: do not override minimumCores

f82485e

fix: syntax error, missing semicolon

c3d2c24

fix: handle negative cores during db insertion, in determineMinimumCo…

3a793b7

…res()

fix: syntax error, missing semicolon

7e10f64

log: add debug logs to determine threadable during db insertion.

145d14e

fix: syntax error, did not pay attention

7ca2918

fix: missing logger

7c3f61c

doc: fix typo

3b94240

feat: handle negative core requests (+ remove debug logs)

9baba0b

feat: handle negative core requests (+ remove debug logs)

19521d9

fix: do not default to 100 cores, return false when the processed req…

2cd6945

…uest is <= 0

fix: same for local dispatch

ca89e05

doc: remove all logs

b442fe7

fis: better management of negative cores

33c2952

feat: handle zero/negative cores request when launching frame

866d738

KernAttila added 5 commits August 26, 2023 04:11

fix: no namespace

8ff11e3

config: lower thresholds, jobs are not distributed to all hosts

79b6d11

pref: set frame_query_max=2

247327e

feat: accept dash character in tags (-)

a97b4c0

feat: Added function and property canHandleNegativeCoresRequest

cc0f3a9

doc: added some comments doc: Added some documentation doc: fix docstrings and parameters doc: added debug message doc: explain why we allow negative value doc: update debug message for rqd

KernAttila force-pushed the cuebot-reserve-all-cores branch from 669be2d to cc0f3a9 Compare August 26, 2023 02:12

KernAttila added 4 commits August 26, 2023 16:18

fix: line too long

9eddb03

revert: no need to do this anymore, cuebot handles calculation before…

1a93ca1

… inserting proc

revert: no need to handle negative cores in rqd, cuebot handles the c…

962ce55

…alculation

Merge branch 'master' of https://github.com/Wolf-Pipeline/OpenCue int…

014b91b

…o cuebot-reserve-all-cores

DiegoTavares approved these changes May 21, 2024

View reviewed changes

Merge branch 'master' of https://github.com/Wolf-Pipeline/OpenCue int…

748b12d

…o cuebot-reserve-all-cores # Conflicts: # cuebot/src/main/java/com/imageworks/spcue/dispatcher/HostReportHandler.java

KernAttila requested a review from ramonfigueiredo as a code owner August 24, 2024 17:26

KernAttila added 2 commits August 24, 2024 19:34

Merge branch 'master' into cuebot-reserve-all-cores

4efe601

Signed-off-by: Kern Attila GERMAIN <[email protected]>

chores: fix variable name, add comment

302a67f

KernAttila marked this pull request as draft August 27, 2024 10:19

KernAttila added 5 commits September 11, 2024 11:43

Merge branch 'master' into cuebot-reserve-all-cores

9a7a9cf

feat: change the layer "cores" label for zero or negative values, sta…

1f7235f

…ting "ALL" or "ALL (-x)".

doc: update the layer "cores" column tooltip to inform the user about…

c1cfbdc

… the new negative cores feature.

fix: missing space in sql statement.

318021c

Merge branch 'AcademySoftwareFoundation:master' into cuebot-reserve-a…

6796bac

…ll-cores

KernAttila mentioned this pull request Sep 21, 2024

Cuesubmit jobs from config file #1284

Open

KernAttila marked this pull request as ready for review September 21, 2024 02:03

DiegoTavares merged commit da2fe01 into AcademySoftwareFoundation:master Sep 25, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cuebot reserve all cores #1313

Cuebot reserve all cores #1313

KernAttila commented Aug 26, 2023 •

edited

Loading

bcipriano commented Sep 27, 2023

bcipriano commented Sep 27, 2023

KernAttila commented Nov 9, 2023

DiegoTavares left a comment

KernAttila commented May 21, 2024

DiegoTavares commented May 21, 2024 via email

KernAttila commented Sep 21, 2024

Cuebot reserve all cores #1313

Cuebot reserve all cores #1313

Conversation

KernAttila commented Aug 26, 2023 • edited Loading

bcipriano commented Sep 27, 2023

bcipriano commented Sep 27, 2023

KernAttila commented Nov 9, 2023

DiegoTavares left a comment

Choose a reason for hiding this comment

KernAttila commented May 21, 2024

DiegoTavares commented May 21, 2024 via email

KernAttila commented Sep 21, 2024

KernAttila commented Aug 26, 2023 •

edited

Loading