marcoCasamento / Hangfire.Redis.StackExchange

HangFire Redis storage based on original (and now unsupported) Hangfire.Redis but using lovely StackExchange.Redis client
Other
452 stars 108 forks source link

DisableConcurrentExecution Attribute does not seems to work #65

Closed JustinAren closed 4 years ago

JustinAren commented 6 years ago

Hi,

I want to use the DisableConcurrentExecution attribute to prevent that some jobs are running multiple times at the same time, but it does not seems to work.

As you can see in the following screendumps, both jobs started at the same time, and bot jobs ended at almost the same time. Also, in Redis I don't see any evidence of a lock object being created

First execution: disableconcurrentexecution job 1

Second execution: disableconcurrentexecution job 2

I am using the following code:

[Queue(HangfireConstants.HighPriorityQueue)]
[DisableConcurrentExecution(10)]
public void PaymentEngineProcessAsync(string specificAcquirer = null, string commandlineParameters = null, PerformContext context = null)
{
    string output;
    Business.PaymentEngine.PaymentEngine.ProcessAsynchronous(out output, specificAcquirer, commandlineParameters);
    context.WriteLine(output);
    Thread.Sleep(60000);
    context.WriteLine("Awake");
}

And I am using the following packages and versions:

<packages>
  <package id="Hangfire.Console" version="1.3.4" targetFramework="net451" />
  <package id="Hangfire.Core" version="1.6.16" targetFramework="net451" />
  <package id="Hangfire.Redis.StackExchange" version="1.7.0" targetFramework="net451" />
  <package id="Newtonsoft.Json" version="9.0.1" targetFramework="net451" />
  <package id="StackExchange.Redis" version="1.2.6" targetFramework="net451" />
  <package id="StackExchange.Redis.Extensions.Core" version="2.3.0" targetFramework="net451" />
</packages>

Am I doing something wrong?

Many thanks in advance.

Justin

xumix commented 5 years ago

I don't know if it's Hangfire or RedistStorage bug, but it seems that recurring jobs marked with DisableConcurrentExecution are still trying to launch every period and they wait for timeout set in the attribute. I suppose that the logic should be not to launch a job if timeout is not expired

marcoCasamento commented 4 years ago

I believe that is already solved. I've tried the following code in a linqpad window and it does work as expected.

void Main()
{
    var storage = new RedisStorage();
    var options = new BackgroundJobServerOptions() { Queues = new[] { "myqueue" }};
    var js = new BackgroundJobServer(options, storage);
    var client = new BackgroundJobClient(storage);
    client.Enqueue<DummyClass>(x=> x.PaymentEngineProcessAsync("", "", null));
    client.Enqueue<DummyClass>(x=> x.PaymentEngineProcessAsync("", "", null));
    LINQPad.Util.KeepRunning();
}

public class DummyClass
{
    // Define other methods, classes and namespaces here
    [Hangfire.Queue("myqueue")]
    [DisableConcurrentExecution(10)]
    public void PaymentEngineProcessAsync(string specificAcquirer = null, string commandlineParameters = null, PerformContext context = null)
    {
        $"Starting {DateTime.Now}".Dump();
        Thread.Sleep(60000);
        $"Done {DateTime.Now}".Dump();
    }
}

Output is:

Starting 09/01/2020 12:01:03
Done 09/01/2020 12:02:03
Starting 09/01/2020 12:02:03
Done 09/01/2020 12:03:03