project-codeflare / multi-cluster-app-dispatcher

Holistic job manager on Kubernetes
Apache License 2.0
107 stars 63 forks source link

fix for some AWs that do not complete #658

Closed asm582 closed 12 months ago

asm582 commented 12 months ago

Issue link

657

What changes have been made

Changes to addqueue method which exists only after informer and etcd has status.

Verification steps

I have manually run scale tests and confirmed all AWs complete

(base) abhishekmalvankar@Abhisheks-MBP perf-test % kubectl get appwrappers --all-namespaces -o json | jq '.items[] | select(.status.Succeeded != 1) | {namespace: .metadata.namespace, name: .metadata.name, status: .status}'
(base) abhishekmalvankar@Abhisheks-MBP perf-test % 

Checks

jbusche commented 12 months ago

I'm hoping I did my PR onto your PR right, but I made a few small adjustments to the perf.sh script that seems to be working well. I rebuilt the mcad image based on PR 658 and it's running very well, not seeing any hanging appwrappers - I've run up to 300 so far.

As a control, I also ran the same perf.sh against the codeflare-operator:v1.0.0-rc.1 release and that one does well with 5 and 20 appwrappers, but is consistently hanging with 50 appwrappers. The jobs all show complete, but the appwrappers are fighting to write to etcd.

I think this PR is a big improvement, if we can just get the small perf.sh tweaks in there.

jbusche commented 12 months ago

It's looking really good @asm582, I've built the image and then done:

helm install mcad-controller --namespace kube-system --generate-name --set  image.repository=quay.io/jbusche/mcad-controller --set image.tag=comp-stat-try2

and then running your newly modified perf.sh and it's looking really good:

AppWrappers     Total Time (seconds)    MCAD Image
50      181 seconds     Abishek PR 658
50      182 seconds     Abishek PR 658
100     343 seconds     Abishek PR 658
200     670 seconds     Abishek PR 658
300     1011 seconds    Abishek PR 658

I'm also watching the appwrappers and it's cleanly finishing even the long runs, which was NOT the case in the regular rc.1 image.

watch "kubectl get appwrappers --all-namespaces -o json | jq '.items[] | select(.status.Succeeded != 1) | {namespace: .metadata.namespace, name: .metadata.name, status: .status}' |wc -l"
0
openshift-ci[bot] commented 12 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbusche, metalcycling

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/project-codeflare/multi-cluster-app-dispatcher/blob/main/OWNERS)~~ [metalcycling] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment