0

I'm not familiar with Postgres concurrency and lock models, so I assume there might be a solution to the problem I'm having without resorting to explicit locks in code. I'm integrating one of payment gateway service. Everytime there is a payment made, I receive a notification and add the sum to user's balance. Now here's the problem, there's a slim, but theoretical chance that I receive 2 messages with identical payload. That is, two payment notification for single transaction. The problem is to avoid adding double amount to user's balance.

Here's the code:

        var deposit = await _repository.GetDepositByGatewayDepositId(ipn.DepositId);
        var oldStatus = default(DepositStatus);
        if (deposit == null)
        {
            deposit = _converter.ToDeposit(model, status, ipn.Id, user.Id);
        }
        else
        {
            oldStatus = deposit.Status;
            deposit.IpnId = ipn.Id;
            deposit.Updated = DateTime.UtcNow;
            deposit.Status = status;
        }
        await _repository.SaveAsync(deposit);

        if (deposit.Status == DepositStatus.Completed && oldStatus != DepositStatus.Completed)
        {
            await _repository.AddBalance(user.Id, ipn.Amounti);
        }

I'm updating user's balance only if previous payment status (oldStatus variable) was not Completed (Pending), however, I acquire oldStatus from database, and if there was a duplicate request and second transaction was also in progress, I would end up with a race condition.

So my question is, is there an efficient way to solve the problem without using locks in code? Maybe tell Postgres to use row level lock that will prevent reads from other transactions until first one finishes?

Update: Thanks everyone for the replies. I think I would add few more details to the topic. I might receive multiple message for same transaction (same Id that is) when trans status changes. Good part is that the messages are digitally signed, so I already added unique constraint on signature field. But payment gateway states that any status >= 100 is a success status. So in theory, if they send me status 100 and then 101, this theoretically might cause race condition with financial consequences. Again, the chances might be very low, but I'd rather be safe then sorry.

Based on the replies, the most elegant solution seems to be optimistic locking. However, for this particular case, I think distributed locking might be a better option because it seems to be simpler. Acquire lock only for one specific customer so lock for customer A doesn't cause wait for lock for customer B.

Thanks

Davita
  • 8,928
  • 14
  • 67
  • 119
  • 1
    To avoid locks you can use "optimistic locking" (with an overflow bucket and random retry times). It's much more performant. Or... you can serialize the transactions. Or, you can use a queue in the app. Or you can use explicit locks. For high volumes I still prefer the first option, though. – The Impaler Jul 04 '21 at 03:51

1 Answers1

1

SET TRANSACTION ISOLATION LEVEL SERIALIZABLE will put your transaction into serializable mode, which basically means no transactions interfere with yours edit: provided they're also serializable mode. But that may come at a large performance cost, and you have to be prepared to retry your transaction if (and when) it fails. Alternatively Postgres has advisory locks. Both features have a little required reading. The Impaler's comment mentions that and other options. But maybe there's a way to avoid this...

These notifications really ought to have IDs, the way you're describing them. IpnId = ipn.Id; stands out. Are those unique? You could put a unique constraint on the combination of (payment_id, status) so it's impossible for two transactions to race and both (or neither) think they were first to mark a payment complete. Even the default, non-serializable isolation level guarantees this, so you avoid dealing with serializable mode.

A potentially more elegant solution is to further normalize your schema: Don't have a separate table for deposits and balances. Only track deposits, and derive a user's balance from them (and potentially other tables like "withdrawals" etc) when you need to. Then you're not worrying about ensuring those two tables are consistent with each other. Personally it's rare that I use a multi-statement write transaction in my own services' code at all, since I don't want to deal with the caveats, and I find the more normalized schema this policy forces upon me easier to work with in most ways.

sudo
  • 5,604
  • 5
  • 40
  • 78
  • Hey sudo, thanks for the reply. Does SERIALIZABLE level cause whole table lock or row level one? – Davita Jul 05 '21 at 09:29
  • @Davita If you're careful, it'll only lock the row. You have to make sure no seq scans occur because those will lock the whole table, and there are probably other situations that lock it too. And when I say "lock," these are optimistic locks. They might not block other transactions from writing. If one does, your serializable xact will fail. – sudo Jul 05 '21 at 18:44
  • Forgot to mention, if a non-serializable xact edits the row, the serializable one won't realize. All the xacts affecting the same data have to be serializable. See https://stackoverflow.com/questions/23805128/mixing-isolation-levels-in-postgresql . For detailed info, the official docs are best: https://www.postgresql.org/docs/9.5/transaction-iso.html – sudo Jul 05 '21 at 19:59