HangfireIO / Hangfire

An easy way to perform background job processing in .NET and .NET Core applications. No Windows Service or separate process required
https://www.hangfire.io
Other
9.42k stars 1.7k forks source link

Pending local transaction is not assigned to SQL commands #1008

Open dweggemans opened 7 years ago

dweggemans commented 7 years ago

I am trying to use Hangfire with an existing connection and transaction. Ultimate goal is to save changes together with one or more background jobs in a single transaction.

This fails because the transaction is not set on the SQL commands executed by Hangfire. I assume this would work on the full framework because one can use a TransactionScope and the SQL commands would auto enlist in the ambient transaction? This is not an option on .NET Core though.

Exception details

Hangfire.BackgroundJobClientException: Background job creation failed. See inner exception for details. ---> System.InvalidOperationException: ExecuteScalar requires the command to have a transaction when the connection assigned to the command is in a pending local transaction.  The Transaction property of the command has not been initialized.
   at System.Data.SqlClient.SqlCommand.ValidateCommand(Boolean async, String method)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean asyncWrite, String method)
   at System.Data.SqlClient.SqlCommand.ExecuteScalar()
   at Dapper.SqlMapper.ExecuteScalarImpl[T](IDbConnection cnn, CommandDefinition& command)
   at Dapper.SqlMapper.ExecuteScalar[T](IDbConnection cnn, String sql, Object param, IDbTransaction transaction, Nullable`1 commandTimeout, Nullable`1 commandType)
   at Hangfire.SqlServer.SqlServerConnection.<>c__DisplayClass7_0.<CreateExpiredJob>b__0(DbConnection connection)
   at Hangfire.SqlServer.SqlServerStorage.UseConnection[T](DbConnection dedicatedConnection, Func`2 func)
   at Hangfire.SqlServer.SqlServerConnection.CreateExpiredJob(Job job, IDictionary`2 parameters, DateTime createdAt, TimeSpan expireIn)
   at Hangfire.Client.CoreBackgroundJobFactory.Create(CreateContext context)
   at Hangfire.Client.BackgroundJobFactory.<>c__DisplayClass7_0.<CreateWithFilters>b__0()
   at Hangfire.Client.BackgroundJobFactory.InvokeClientFilter(IClientFilter filter, CreatingContext preContext, Func`1 continuation)
   at Hangfire.Client.BackgroundJobFactory.Create(CreateContext context)
   at Hangfire.BackgroundJobClient.Create(Job job, IState state)
   --- End of inner exception stack trace ---
   at Hangfire.BackgroundJobClient.Create(Job job, IState state)

Steps to reproduce

using (var context = new TestDbContext())
{
    context.Database.Migrate();
    context.InitializeHangfireIfNecessary();

    context.SaveChanges();
}
public class TestDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer("Server=.;Database=HangfireTest;Integrated Security=SSPI");
        //optionsBuilder.ConfigureWarnings(x => x.Ignore(RelationalEventId.AmbientTransactionWarning));
        base.OnConfiguring(optionsBuilder);
    }

    public override int SaveChanges()
    {
        //TransactionScope is not supported on .NET Core. 
        //Exceptions can be suppressed, but hangfire commands are not enlisted in the transaction
        //using (var tx = new TransactionScope())
        using (var tx = Database.BeginTransaction())
        {
            //Reuse existing connection because we want a single transaction
            var storage = new SqlServerStorage(Database.GetDbConnection(), 
                new SqlServerStorageOptions { PrepareSchemaIfNecessary = false });

            var client = new BackgroundJobClient(storage);

            client.Enqueue(() => Console.WriteLine("Fire-and-forget!"));
            var result = base.SaveChanges();
            tx.Commit();

            return result;
        }
    }

    public void InitializeHangfireIfNecessary()
    {
        // Initialize Hangfire storage        
        var storage = new SqlServerStorage(Database.GetDbConnection());
    }
}
dweggemans commented 7 years ago

I have found no easy solution since commands are not automatically enlisted in local pending transactions. See https://github.com/dotnet/corefx/issues/13927 This means you have to pass the existing transaction around to assign it to the command.

odinserj commented 7 years ago

Ultimate goal is to save changes together with one or more background jobs in a single transaction.

Have you considered to start a database transaction, and create a background job just before committing it?

using (var transaction = postgres.BeginTransaction())
{
    var entityId = transaction.CreateEntity();
    BackgroundJob.Enqueue(() => ProcessEntity(entityId));

    transaction.Commit();
}

When executing actions like this, you could expect the following behaviors:

  1. Neither entity, nor background job is created. This is fine, usually error is send back to a user asking for a retry.
  2. Both entity and background job is created. Everything is fine too.
  3. Background job is created, but transaction fails. In this case error should be reported to a user, and background job should deal with non-existing entities:
  4. ~Transaction committed, but background job isn't created~: impossible, because in this case you'll get an exception from the BackgroundJob.Enqueue method, and your transaction will not be committed.
public static void ProcessEntity(int entityId)
{
    using (var transaction = Database.BeginTransaction(IsolationLevel.ReadCommitted))
    {
        var entity = transaction.GetEntity(entityId);
        if (entity == null) return;

        // Process entity
    }
}

That shouldn't be a big problem, because background jobs sometimes are delayed to minutes, hours and days, and they should be robust enough to deal with non-existing records.

dweggemans commented 7 years ago

I've considered it, but rejected the idea because it is error prone (this is coded in a base library which is used by many components) to have all handlers perform these checks. Besides that it has performance impact.

What I did as a workaround is that I've subclassed 'SqlServerStorage' to return a custom connection that wraps the SqlServerConnection and override the CreateExpiredJob to use the existing transaction.

Additionally I had to override CreateWriteTransaction to return a custom JobStorageTransaction which also uses the existing transaction on the few methods that are used to create a job. The only reason to do this is to prevent it from actually starting a transaction (which I couldn't, so I had to override the methods that create and update jobs not to start another transaction).

Not the most elegant, but it works. The downside is that I now have a direct dependency on the Hangfire database schema.