Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Fix a subworkflow performance issue without selecting tasks #3152

Merged
merged 10 commits into from
Aug 9, 2022
Merged

Fix a subworkflow performance issue without selecting tasks #3152

merged 10 commits into from
Aug 9, 2022

Conversation

james-deee
Copy link
Contributor

@james-deee james-deee commented Aug 6, 2022

Pull Request type

  • [ X] Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew generateLock saveLock to refresh dependencies)
  • WHOSUSING.md
  • Other (please describe):

Changes in this PR

Describe the new behavior from this PR, and why it's needed
This plays it extremely safe for now, but other SystemTasks might be able to take advantage of this as well. The only SystemTask that I think needs the tasks pulled in is the DO WHILE. But for now, the subworkflow benefits greatly by not selecting all the tasks (it doesnt need them at this point).

jxu-nflx
jxu-nflx previously approved these changes Aug 8, 2022
@jxu-nflx jxu-nflx dismissed their stale review August 8, 2022 20:58

un-approval it

@@ -52,6 +52,8 @@ class AsyncSystemTaskExecutorTest extends Specification {
workflowExecutor = Mock(WorkflowExecutor.class)

workflowSystemTask = Mock(WorkflowSystemTask.class)
then:
workflowSystemTask.isTaskRetrievalRequired() >> true
Copy link
Contributor

Choose a reason for hiding this comment

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

workflowSystemTask = Mock(WorkflowSystemTask.class) { isTaskRetrievalRequired() >> true }

@apanicker-nflx apanicker-nflx merged commit e20a193 into Netflix:main Aug 9, 2022
pmchung pushed a commit to routific/conductor that referenced this pull request Sep 6, 2023
…3152)

* Try fixing subworkflow performance issue

* Running spotless apply on conductor core

* Revert "Running spotless apply on conductor core"

This reverts commit b5ebd43.

* apply spotless java 11

* fix unit tests

* Try to log some info

* Revert "Try to log some info"

This reverts commit 3411f5d.

* See if this change fixes the broken test

* Try again
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants