Plan execution with transaction

[Risord]

Hi

We are currently using plans everywhere and I have somehow (miss) understood that plan also wraps with transactions. However we have some code like this: if exists() then update() else insert(). This constantly generates duplicate primary key violations since data is tried to update while it's actually exists during rapid repetition of function.

Commands have clear way of connection and therefore transaction management but how transactions should be used with plans?

[rspeele]

Hi Risord,

This is a big answer. Please read carefully. I should probably add something to the Rezoom docs about this, but might end up just linking to this issue.

Transactions

When you execute a plan, the whole thing is wrapped in a transaction. The isolation level defaults to whatever you get from DbConnection.BeginTransaction() which will depend on the database backend you're using. But you can implement your own ConnectionProvider by inheriting from DefaultConnectionProvider and overriding BeginTransaction. Then use it like so:

let myConnectionProvider = new MyConnectionProvider() // whatever class you wrote
let serviceConfig = new ServiceConfig()
// make sure to explicitly pass the type parameter here so it configures
// the base ConnectionProvider type to use this instance
serviceConfig.SetConfiguration<Rezoom.SQL.Mapping.ConnectionProvider>(myConnectionProvider)
let executionConfig =
    {   Execution.ExecutionConfig.Default with
            ServiceConfig = upcast serviceConfig
    }
let runPlan (plan : 'a Plan) : 'a Task = Execution.execute executionConfig plan

Batching and pre-condition checks

It sounds like your problem might not be due to the transactions at all, but happening within the execution of a single plan. Batching can make things that would otherwise be occasional concurrency conflicts happen every time. If you write code like this, it will always fail (if starting with no such user in the DB, and a unique constraint on email):

let addUserIfNotExisting (email : string) =
    plan {
          let! alreadyExists = checkIfUserAlreadyExists email
          if not alreadyExists then
              do! addUser email
    }

let exampleFailure() =
    plan {
        for email in batch ["a@example.com"; "a@example.com"] do
            do! addUserIfNotExisting email
    }

This is because all the plans in the batch step move in lockstep. So they start by both checking if the user exists, and both get the same answer "nope, no user by that name". Then they both conclude that they can move forward. so the 2nd one to execute blows up when it violates the unique constraint.

It's actually not so bad here since you at least see there's a problem. What you really want to watch out for is attempting to write code like:

let addUser (organization : OrganizationId) (email : string) =
    plan {
          let! organizationLimits = getOrganizationLimits organization
          let! numberOfUsers = getNumberOfUsers organization
          if numberOfUsers >= organizationLimits.MaximumNumberOfUsers then
              failwith "Hey, you have to pay more for a subscription to have that many users!"
          else
              do! addUserInternal organization email
    }
let addMultipleUsers (organization : OrganizationId) (emails : string list) =
    plan {
          for email in batch emails do
                do! addUser organization email
    }

Here, anyone could add as many users as they wanted to their organization, as long as they currently have room for at least one and they add them via a single call to addMultipleUsers. So the code that tries to enforce the limits is ineffective.

To be clear, this would have been bad code anyway, except that normally we only have concurrency between multiple requests to our web API, not within the handling of a single request, so slapping a transaction around the whole API call solves the concurrency issue. With plan batching you have to think about concurrency at a finer-grained level.

Avoiding this problem

Option 1: Atomic SQL commands

When possible, the best way to avoid the problem is to unify the check and the action into a single command so it executes atomically. For example, you could write an insert-or-update as a two-command sequence where part 1 inserts the record if it does not exist then spits out its ID, and part 2 unconditionally updates the record.

The SQL for part 1 could look like:

-- add the user if they don't already exist
insert into Users(Email)
select @email
where not exists (select null x from users where Email = @email);

-- return the row id to the program for use in the update part
select Id from Users where Email = @email;

Option 2: Convert pre-conditions to post-conditions

If you have a more complex scenario, violation of the business rule should be an error, and you don't want to have to think about what exactly the implications of batching will be, a bulletproof approach is to change your preconditions into postconditions.

This would probably be the approach I'd use in the case of the user limit for an organization. I'd write the code to add the user and then check if the limit was exceeded, instead of the other way around. Here I'd be leaning on the fact that every plan implicitly runs in the transaction, and if the plan execution fails due to an uncaught exception, the whole thing won't be committed. Of course you do still have to make sure you don't have a catch-all somewhere higher up within the plan that will eat and ignore this. If you really want to be certain, you can raise (PlanAbortException("message")) which cannot be caught within a plan block.

Option 3: Contribute a new feature to Rezoom

This feature does not exist, but it could, and it would be great for situations like this. If there was a way to convert an 'a Plan to an 'a Errand, you could use this to wrap your plan { precondition; conditional action } so it runs atomically (i.e. no other errands can run in between the precondition and conditional action).

This is semi-possible already since you can convert the plan to a task, and then wrap the task in an errand, but doing that loses the caching information and also would probably have issues due to the wrapped plan trying to use a different connection and transaction.

Implementing this feature would not be terribly hard (Rezoom is a small codebase, much smaller than Rezoom.SQL) but requires some thought as to what order errands would run in and whether that order would be guaranteed or arbitrary.

[Risord]

Hi

Thanks for fast and wide answer. This is useful information many ways for the future.

Actually in this case it was really multi request race condition. SQL Server just seems to use read commited isolation level by default so raising that to repeatable read this problem vanished. Although transaction repeat would be needed next.

At the end I made my SQL query to atomic. I was convinced that conditional insert is not possible but thanks for you sample it's now done and seems to work perfectly!

Edit: Btw I did use SQL profiler to watch under hood but since transactions aren't done with explicit begin transaction I was concluded wrongly that there are none.