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

do not synchronously eval workflow during update task for select criteria #3146

Merged
merged 2 commits into from
Aug 5, 2022

Conversation

apanicker-nflx
Copy link
Collaborator

@apanicker-nflx apanicker-nflx commented Aug 3, 2022

Pull Request type

  • 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):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Describe the new behavior from this PR, and why it's needed
Issue #

Alternatives considered

Describe alternative implementation you have considered

@apanicker-nflx apanicker-nflx changed the title do not synchronously eval workflow when task in dynamic fork branch i… do not synchronously eval workflow when task in dynamic fork branch is updated Aug 3, 2022
@@ -168,6 +168,10 @@ public static void recordGauge(String name, long count) {
gauge(classQualifier, name, count);
}

public static void recordCounter(String name, long count, String... additionalTags) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan to use this method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, i noticed that we have a public method for gauge but not for a counter. Further, I plan to use the counter.

@@ -183,6 +183,9 @@ class DecisionTaskSpec extends AbstractSpecification {
when: "the task 'integration_task_20' is polled and completed"
def polledAndCompletedTask20Try1 = workflowTestUtil.pollAndCompleteTask('integration_task_20', 'task1.integration.worker')

and: "the workflow is evaluated"
sweep(workflowInstanceId)
Copy link
Contributor

Choose a reason for hiding this comment

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

why this changes in tests? same comment for the others?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As part of the changes introduced, the workflow will be evaluated lazily for a join task, so we simulate the lazy evaluation by calling a sweep here.

@apanicker-nflx apanicker-nflx force-pushed the do_not_sync_eval_fork_update branch from 0269543 to 758115a Compare August 5, 2022 18:42
@apanicker-nflx apanicker-nflx changed the title do not synchronously eval workflow when task in dynamic fork branch is updated do not synchronously eval workflow during update task for select criteria Aug 5, 2022
@apanicker-nflx apanicker-nflx force-pushed the do_not_sync_eval_fork_update branch from 758115a to 03a6251 Compare August 5, 2022 18:44
@apanicker-nflx apanicker-nflx merged commit 0b85b70 into main Aug 5, 2022
@apanicker-nflx apanicker-nflx deleted the do_not_sync_eval_fork_update branch August 5, 2022 19:52
pmchung pushed a commit to routific/conductor that referenced this pull request Sep 6, 2023
…eria (Netflix#3146)

* do not synchronously eval workflow under select criteria

* expedite lazy evaluation with higher priority
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.

2 participants