perrich / Hangfire.MemoryStorage

A memory storage for Hangfire.
Apache License 2.0
131 stars 43 forks source link

Refactor distributed lock, such that they work with async code #41

Closed Spacefish closed 1 year ago

Spacefish commented 1 year ago
perrich commented 1 year ago

Thank you, I'll try to check and merge your PR tomorrow.

perrich commented 1 year ago

Thanks, seems to work well (not reproduced the sync issue from #31)

EvheniyHlushko commented 1 year ago
`Hangfire.Storage.DistributedLockTimeoutException
Timeout expired. The timeout elapsed prior to obtaining a distributed lock on the 'lock:recurring-job:Test' resource.
   at Hangfire.MemoryStorage.MemoryStorageConnection.AcquireDistributedLock(String resource, TimeSpan timeout)
   at Hangfire.RecurringJobExtensions.AcquireDistributedRecurringJobLock(IStorageConnection connection, String recurringJobId, TimeSpan timeout)
   at Hangfire.RecurringJobManager.AddOrUpdate(String recurringJobId, Job job, String cronExpression, RecurringJobOptions options)
   at Hangfire.RecurringJobManagerExtensions.AddOrUpdate[T](IRecurringJobManager manager, String recurringJobId, Expression`1 methodCall, String cronExpression, RecurringJobOptions options)
   at Hangfire.RecurringJobManagerExtensions.AddOrUpdate[T](IRecurringJobManager manager, String recurringJobId, Expression`1 methodCall, Func`1 cronExpression, RecurringJobOptions options)`

Looks like after these changes on version 1.8 encountering test failures when run in parallel. Everything works in the previous version, 1.7.0.

perrich commented 1 year ago

Seems that you try to execute same named job in the same time. First is not finished and second request is in timeout. I don't think it's a bug.

EvheniyHlushko commented 1 year ago

Right. I'm using WebApplicationFactory for tests. In Strartup each instance adds a background job. Probably it's because Data are stored in a static way

Spacefish commented 1 year ago

Right. I'm using WebApplicationFactory for tests. In Strartup each instance adds a background job. Probably it's because Data are stored in a static way

yes it uses static variables to store a lot of internal state.. So no separation between different instances.. I tried to make them independent while fixing this synchronization issue, but it would have introduced too many changes / i wasn´t sure if this breaks something else.. The MemoryStorageConnection class is public after all..

Furthemore Hangfire itself uses static classes / advertises the use of static class methods to submit jobs..

I did it now and created a PR: https://github.com/perrich/Hangfire.MemoryStorage/pull/43

EvheniyHlushko commented 1 year ago

Built a package based on your PR, still encountering the same issue.

Furthemore Hangfire itself uses static classes / advertises the use of static class methods to submit jobs..

Most likely it's the case static JobStorage.Current passed to RecurringJobManager constructor