fix: ensure namespace parallelism and parallelism work together. Fixes #10985 by isubasinghe · Pull Request #14039 · argoproj/argo-workflows

isubasinghe · 2024-12-30T22:28:36Z

Motivation

The current throttler code does not work when used in unison with namespace parallelism and parallelism. This is because the concept of the chain throttler unfortunately doesn't work.

Modifications

Corrected a bug where workflows weren't added into the running list/map when the parallelism was set to 0.
Added tracking for namespace counts into a new throttler.

Verification

Relied on tests to pass

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe · 2025-01-02T00:28:20Z

/retest

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe · 2025-01-02T01:00:58Z

/retest

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe · 2025-01-03T03:05:46Z

/retest

isubasinghe · 2025-01-03T03:23:09Z

/retest

isubasinghe · 2025-01-03T03:33:35Z

/retest

isubasinghe · 2025-01-05T23:48:29Z

/retest

MasonM · 2025-01-06T02:05:06Z

@isubasinghe The E2E tests are broken due to a bug in Kit. I have a fix for it in #14048. Could you please review that PR?

isubasinghe · 2025-01-06T02:26:23Z

/retest

isubasinghe · 2025-01-06T02:31:17Z

@isubasinghe The E2E tests are broken due to a bug in Kit. I have a fix for it in #14048. Could you please review that PR?

Thanks, I will do

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe · 2025-01-06T05:09:49Z

/retest

Joibel

Also needs some documentation in docs/parallelism.md to document that when using namespaceParallelism a lower priority workflow in a namespace that has capacity will run before a high priority workflow in a namespace with no capacity.

I assume this is the intentional behaviour - it feels the sane option.

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe · 2025-01-07T06:36:19Z

/retest

isubasinghe · 2025-01-07T06:42:16Z

/retest

isubasinghe · 2025-01-07T07:29:51Z

/retest

isubasinghe · 2025-01-07T07:30:31Z

The test have gotten flakier, unsure if this contributed or something else did.

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe · 2025-01-09T02:49:44Z

/retest

Joibel

LGTM, thank you

Joibel · 2025-01-27T17:04:33Z

This PR seems to be the cause of the SignalsSuite test failures - they almost always fail with this change, and reverting makes them much better.

…argoproj#10985 (argoproj#14039) Signed-off-by: isubasinghe <isitha@pipekit.io>

…#10985 (#14039) Signed-off-by: isubasinghe <isitha@pipekit.io>

…#10985 (#14039) Signed-off-by: isubasinghe <isitha@pipekit.io> (cherry picked from commit 09d5ee7)

isubasinghe added 2 commits December 30, 2024 14:26

fix: make throttler understand both namespace and global parallelism

76b839c

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: ensure tests pass

9f6bb24

Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe changed the title ~~Fix namespace parallelism. Fixes #10985~~ fix: ensure namespace parallelism and parallelism work together. Fixes #10985 Dec 30, 2024

isubasinghe added 9 commits December 31, 2024 09:35

fix: removed redundant return

7e4e245

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: test

14599e0

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: linted file

343caf3

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: increase namespace parallelism limits

f82477e

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: remove from pending when called

4ef7f07

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: derive namespace counts

ceed4d5

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: linted file

2d438c0

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: removed unused logging

6edcfb4

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: removed logging

c749ad0

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: removed unnecessary struct

9204838

Signed-off-by: isubasinghe <isitha@pipekit.io>

Joibel self-assigned this Jan 2, 2025

Joibel added area/controller Controller issues, panics area/parallelism `parallelism` for the Controller, Workflows, or templates labels Jan 2, 2025

isubasinghe added 2 commits January 3, 2025 13:28

fix: restore namespace parallelism limits to 10

9e613df

Signed-off-by: isubasinghe <isitha@pipekit.io>

Merge branch 'main' into fix-namespace-parallelism

8be3e3c

isubasinghe marked this pull request as ready for review January 3, 2025 06:32

isubasinghe force-pushed the fix-namespace-parallelism branch from 183177a to 8be3e3c Compare January 6, 2025 02:06

Merge branch 'main' into fix-namespace-parallelism

e9601fb

isubasinghe added 2 commits January 6, 2025 13:46

fix: added printing for debug purposes

8d21515

Signed-off-by: isubasinghe <isitha@pipekit.io>

fix: simplified queueThrottled

ef494d6

Signed-off-by: isubasinghe <isitha@pipekit.io>

Joibel requested changes Jan 6, 2025

View reviewed changes

Comment thread workflow/sync/multi_throttler_test.go Outdated

Comment thread workflow/sync/multi_throttler.go Outdated

Comment thread workflow/sync/multi_throttler.go

Comment thread workflow/sync/multi_throttler.go

Comment thread workflow/sync/multi_throttler_test.go

Joibel reviewed Jan 6, 2025

View reviewed changes

Comment thread workflow/controller/controller_test.go

isubasinghe added 3 commits January 7, 2025 16:39

fix: address PR feedback

576cd1f

Signed-off-by: isubasinghe <isitha@pipekit.io>

docs: added information about namespace parallelism limits

5baa224

Signed-off-by: isubasinghe <isitha@pipekit.io>

docs: linted file

9330773

Signed-off-by: isubasinghe <isitha@pipekit.io>

Merge branch 'main' into fix-namespace-parallelism

9e6d2bc

fix: added alans feedback

aaa4c26

Signed-off-by: isubasinghe <isitha@pipekit.io>

Joibel approved these changes Jan 9, 2025

View reviewed changes

isubasinghe merged commit 09d5ee7 into argoproj:main Jan 9, 2025

Joibel deleted the fix-namespace-parallelism branch January 10, 2025 13:38

isubasinghe added a commit to pipekit/argo-workflows that referenced this pull request Jan 30, 2025

fix: ensure namespace parallelism and parallelism work together. Fixes …

378601f

…argoproj#10985 (argoproj#14039) Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe added a commit to pipekit/argo-workflows that referenced this pull request Jan 30, 2025

fix: ensure namespace parallelism and parallelism work together. Fixes …

442362b

…argoproj#10985 (argoproj#14039) Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe added a commit to pipekit/argo-workflows that referenced this pull request Jan 31, 2025

fix: ensure namespace parallelism and parallelism work together. Fixes …

64c52ff

…argoproj#10985 (argoproj#14039) Signed-off-by: isubasinghe <isitha@pipekit.io>

isubasinghe added a commit that referenced this pull request Jan 31, 2025

fix: ensure namespace parallelism and parallelism work together. Fixes …

7021548

…#10985 (#14039) Signed-off-by: isubasinghe <isitha@pipekit.io>

Joibel mentioned this pull request Feb 3, 2025

GitHub Action CI test-executor E2E Test Often Fails - TestStopBehavior #14120

Open

4 tasks

Joibel pushed a commit that referenced this pull request Feb 7, 2025

fix: ensure namespace parallelism and parallelism work together. Fixes …

293b4a5

…#10985 (#14039) Signed-off-by: isubasinghe <isitha@pipekit.io> (cherry picked from commit 09d5ee7)

Joibel mentioned this pull request Aug 18, 2025

Total parallelism reports limit reached but is not actually reached #14750

Open

4 tasks

Conversation

isubasinghe commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verification

Uh oh!

isubasinghe commented Jan 2, 2025

Uh oh!

isubasinghe commented Jan 2, 2025

Uh oh!

isubasinghe commented Jan 3, 2025

Uh oh!

isubasinghe commented Jan 3, 2025

Uh oh!

isubasinghe commented Jan 3, 2025

Uh oh!

isubasinghe commented Jan 5, 2025

Uh oh!

MasonM commented Jan 6, 2025

Uh oh!

isubasinghe commented Jan 6, 2025

Uh oh!

isubasinghe commented Jan 6, 2025

Uh oh!

isubasinghe commented Jan 6, 2025

Uh oh!

Joibel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

isubasinghe commented Jan 7, 2025

Uh oh!

isubasinghe commented Jan 7, 2025

Uh oh!

isubasinghe commented Jan 7, 2025

Uh oh!

isubasinghe commented Jan 7, 2025

Uh oh!

isubasinghe commented Jan 9, 2025

Uh oh!

Joibel left a comment

Choose a reason for hiding this comment

Uh oh!

Joibel commented Jan 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

isubasinghe commented Dec 30, 2024 •

edited

Loading