Skip to content

Add support for Eager Activity Dispatch#1439

Merged
mjameswh merged 11 commits intomasterfrom
eager-activity-dispatch
Sep 22, 2022
Merged

Add support for Eager Activity Dispatch#1439
mjameswh merged 11 commits intomasterfrom
eager-activity-dispatch

Conversation

@mjameswh
Copy link
Copy Markdown
Contributor

@mjameswh mjameswh commented Sep 15, 2022 •
edited
Loading

What was changed

  • Add support for Eager Activity Support

Why?

  • Improve performance
  • Feature parity with core and go SDKs

Checklist

  1. Add metrics
  2. Closes Eager activity dispatch #1339
  3. Note: Eager Activity Dispatch support in tes server will be pushed in a distinct PR

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 15, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 15, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Spikhalskiy
Copy link
Copy Markdown
Contributor

Spikhalskiy commented Sep 16, 2022

I think this PR should have a unit test where we spawn 2 Workers on the same queue, run a workflow with several sync activities, and make sure that the activities always end up scheduled and executed on the worker-originator. This may be checked from the history.

Spikhalskiy
Spikhalskiy reviewed Sep 16, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowContext.java Outdated
Spikhalskiy
Spikhalskiy reviewed Sep 16, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/worker/ActivityWorker.java Outdated
Spikhalskiy
Spikhalskiy reviewed Sep 16, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java Outdated
mjameswh
mjameswh commented Sep 20, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/activity/ActivityOptions.java
mjameswh
mjameswh commented Sep 20, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowContextImpl.java Outdated
@Spikhalskiy Spikhalskiy marked this pull request as ready for review September 21, 2022 17:02
Spikhalskiy
Spikhalskiy reviewed Sep 21, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/worker/EagerActivityInjector.java Outdated
Spikhalskiy
Spikhalskiy reviewed Sep 21, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/worker/EagerActivityInjector.java Outdated
@mjameswh mjameswh force-pushed the eager-activity-dispatch branch from f9f54ed to 8e55bef Compare September 22, 2022 15:49
Spikhalskiy
Spikhalskiy approved these changes Sep 22, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@Spikhalskiy Spikhalskiy left a comment

Choose a reason for hiding this comment

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

LGTM!

cretz
cretz reviewed Sep 22, 2022
View reviewed changes
Comment thread temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowContext.java
Comment thread temporal-sdk/src/main/java/io/temporal/internal/worker/EagerActivitySlotsReservation.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/internal/worker/EagerActivitySlotsReservation.java Outdated
@mjameswh mjameswh merged commit 447c4ef into master Sep 22, 2022
@mjameswh mjameswh deleted the eager-activity-dispatch branch September 22, 2022 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bergundy bergundy bergundy left review comments

@mfateev mfateev Awaiting requested review from mfateev

@Sushisource Sushisource Awaiting requested review from Sushisource

@mmcshane mmcshane Awaiting requested review from mmcshane

+2 more reviewers

@cretz cretz cretz left review comments

@Spikhalskiy Spikhalskiy Spikhalskiy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Eager activity dispatch

5 participants

@mjameswh @CLAassistant @Spikhalskiy @bergundy @cretz