temporal icon indicating copy to clipboard operation
temporal copied to clipboard

Do not consume rate limit token on invalid tasks

Open yiminc opened this issue 1 year ago • 2 comments

When setting maxTaskQueueActivitiesPerSecond to enable rate limit on task queue, the token is consumed when task is attempted to be delivered. However, the task could be invalid (expired or workflow already closed), which lead to waste of rate limit token.

We should change this to only consume token when task is delivered.

yiminc avatar Jul 24 '24 19:07 yiminc

I think if you do this in the obvious way, it'll create more problems. Like if you block on the rate limiter after a task is assigned to a poller, and then the poller times out, you have to put the task on the end of the queue, so you'd get a lot more cycling and maybe fail to deliver entirely for low rate limits.

Two other approaches could be:

  1. If you fail to deliver the task for any reason after it comes from the matcher, return the token that was taken (so another task can go immediately). This would probably look like storing a reservation in the task and canceling it in finish if there was an error.
  2. Make the task validator more aggressive so these tasks are more likely to be thrown away before they get matched.

dnr avatar Jul 24 '24 21:07 dnr

For 1 (returning the token after delivery failure), it turns out that the rate limiter doesn't support canceling a reservation after its time has passed, so the logic would have to be more complicated.

dnr avatar Aug 27 '24 18:08 dnr