4

If the workload of a minion takes longer time than it's WakeupInterval, in order to avoid getting a second copy of the same minion processing the same list, is there a way to know if there is a previous process (from last time the minion got invoked) is still running so the first one can skip this wakeup?

  • I think this issue has been solved in Sitecore Commerce 9.0 update 3. – Erwin Apr 16 at 7:35
4

Update 2019-04 - Commerce 9.1 resolves the reentrancy problems, just override Minion.Execute instead of Minion.Run. For a deep dive into the changes, I've written up an article Reentrancy improvements to Minions in Sitecore Commerce 9.1

Original answer (Commerce 9.0):

First up, I consider the current behavior a bug since it directly conflicts with minions' primary usecase which is to process items from a queue. As such, I expect to it be fixed in a future version of Commerce so you might want to go with option 2 below.

Having said that, there are two ways you can go about solving this:

(note: all code is theoretical and untested - let me know if you run into any issues)

Option 1: Skip reentrant calls

This isn't difficult to implement naively, and since minions will typically wait a certain amount of time before restarting you shouldn't have to worry about multithreaded access. Having said that, minions without a WakeupInterval run the risk of complicating this situation if their execution is delayed by the OS (or TPL).

If you want to deal with potential threading issues, the typical avenues are mostly off limits since locks are per thread and minions are started using Task.Run with ConfigureAwait(false) so awaits will almost certainly complete on a different thread than they started on. There are libraries that can handle this, but that seems overkill for this one-off usage.

With all that in mind, here's a base class that uses Interlocked.CompareExchange to safely skip reentrant calls:

public abstract class NonReentrantMinion
{
    private int isRunning = 0;

    public abstract Task<MinionRunResultsModel> SafeRun();

    public override async Task<MinionRunResultsModel> Run()
    {
        if (Interlocked.CompareExchange(ref isRunning, 1, 0) != 0)
        {
            // Log skipped invocation
            return;
        }

        try
        {
            return await SafeRun();
        }
        finally
        {
            isRunning = 0;
        }
    }
}

NOTE: isRunning is intentionally non-static since Minions are singletons per Policy, which is likely inline with how you'd want the reentrancy to work.

Option 2: Queue the WakeupInterval delay

Since most minions continue to drain their managed list until it's empty, making it essentially pointless to invoke it again immediately afterwards, I consider this option to be the "fix Minion.Start" approach.

public abstract class SerialMinion
{
    public override void Start()
    {
        Task.Run(async () => 
        {
            while (true)
            {
                try
                {
                    await this.Run();
                }
                catch
                {
                    // Prevent exceptions from breaking the loop
                    // (not logged since the base implementation doesn't either)
                }

                if (this.Policy.WakeupInterval.HasValue)
                {
                    await Task.Delay(this.Policy.WakeupInterval.Value);
                }
            }
        }).ConfigureAwait(false);
    }
}
  • Thank you very much for your input. I will check with my colleagues which of your soulutions we should try first. I'm sure either one of them will solve my problem. – Thomas Wegebrand Mar 26 '18 at 6:23
  • Np. If it does resolve your issue, consider marking it as "the answer" so other users don't try to follow up. – Richard Szalay Mar 26 '18 at 6:27
  • 1
    I will do that as soon as I have had a chance to try to implement and test it. I can't promice I get a chance at it today though. – Thomas Wegebrand Mar 26 '18 at 6:46
  • 1
    I have now successfully implemented and tested the first of your solutions and it do solve my issue. Thank you for your assistance. – Thomas Wegebrand Mar 28 '18 at 13:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.