Skip to content

Added support for the dynamic rate limiting using StartWorkflowRequest #569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hchepey-clari
Copy link
Contributor

@hchepey-clari hchepey-clari commented Aug 8, 2025

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

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

Changes in this PR

Implemented a runtime-configurable rate limiting feature that allows per-workflow-instance rate limit customization while maintaining full backward compatibility.

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

We have a requirement where we want to rate limit the tasks as per the different client rate limit applicable. That is dynamic based on the Workflow Inputs supplied to the Workflow.

When starting a workflow, users can provide optional taskRateLimitOverrides in the request
These overrides are keyed by reference task name (most specific) or task definition name
Task mappers check for overrides before falling back to static TaskDef values
The existing Redis rate limiting infrastructure works unchanged with the dynamic values

Alternatives considered

Describe alternative implementation you have considered

@v1r3n
Copy link
Collaborator

v1r3n commented Aug 9, 2025

Have you considered adding TaskDef with rate limits in the workflow definition when starting a workflow?

@hchepey-clari
Copy link
Contributor Author

Have you considered adding TaskDef with rate limits in the workflow definition when starting a workflow?

I just checked. Thanks for pointing that out. Let me think about it.

One thing for sure, that would mean I have to pass the entire workflowdef json in every StartWorkflow Request. And we have to operate on a scale where we have to invoke thousands of workflows every hour at a regular cadence.

@hchepey-clari
Copy link
Contributor Author

Have you considered adding TaskDef with rate limits in the workflow definition when starting a workflow?

I just checked. Thanks for pointing that out. Let me think about it.

One thing for sure, that would mean I have to pass the entire workflowdef json in every StartWorkflow Request. And we have to operate on a scale where we have to invoke thousands of workflows every hour at a regular cadence.

This change is backward compatible and would not need to pass the WorkflowDef everytime in the StartWorkflowRequest payload. We will be triggering millions of workflows everyday. Do you foresee any other concern with this change, @v1r3n ?

@hchepey-clari
Copy link
Contributor Author

@v1r3n @manan164 - Bumping up in case if this slipped off your radar.

@v1r3n
Copy link
Collaborator

v1r3n commented Aug 22, 2025

Passing WorkflowDef is going to be OK -- I do not see any performance issues with that, actually it might overall be a little quicker because it completely avoids DB read to get the workflow metadata.

The purpose makes sense, but I think the change itself is a bit too complex. We are internally discussing to see if there is a simpler way. The easiest thing to do here might to be be able to set the parameterized values for task definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants