microsoft / durabletask-go

The Durable Task Framework is a lightweight, embeddable engine for writing durable, fault-tolerant business logic (orchestrations) as ordinary code.
Apache License 2.0
198 stars 25 forks source link

Memory Leak: Let's not create ticker each time #30

Closed DeepanshuA closed 1 year ago

DeepanshuA commented 1 year ago

Currently, we create ticker each time.

I ran 500 workflow instances concurrently. All workflows of same type. Each workflow having 4 activities and each activity just doing only an addition of 1 to input.

After all these workflows had run, I collected heap dump. It resulted in following:

image image

So, in all around 8.8+2.2 = 11% of current inuse_space. In some other runs, it reached a high of around 14% as well.

image image

In this change, we don't create Ticker each time. Rather, we just create it once and when ProcessNext is called, we DON'T Stop current Ticker and start New. RATHER, we just Reset the backoff.

After the change, I don't see it in inuse_space:

image
ItalyPaleAle commented 1 year ago

@DeepanshuA i've opened a PR against yours to avoid the use of tickers. PTAL: https://github.com/DeepanshuA/durabletask-go/pull/1

cgillum commented 1 year ago

@DeepanshuA i've opened a PR against yours to avoid the use of tickers. PTAL: DeepanshuA#1

I like the suggestion in @ItalyPaleAle's PR to remove the ticker. Seems to simplify the code quite a bit. I added a comment to that PR.

DeepanshuA commented 1 year ago

@DeepanshuA i've opened a PR against yours to avoid the use of tickers. PTAL: DeepanshuA#1

I like the suggestion in @ItalyPaleAle's PR to remove the ticker. Seems to simplify the code quite a bit. I added a comment to that PR.

I have merged that PR. Please review it now. @cgillum