Threads in spring boot service

Question

Hello there are similar questions around but couldnt find answer that would help me. In spring boot application i have Service class to transfer money between accounts, where im using fine-grained locking. Threads gonna lock if they belong to the same account.

@Service
public class AccountServiceImpl implements AccountService {

static final HashMap<Long, ReentrantLock> locks = new HashMap<Long, ReentrantLock>();

    private ReentrantLock getLock(Long id) {
        synchronized (locks) {
            ReentrantLock lock = locks.get(id);
            if (lock == null) {
                lock = new ReentrantLock();
                locks.put(id, lock);
            }
            return lock;
        }
      }
    }

    @Override
    @Transactional(propagation = Propagation.REQUIRES_NEW)
    public List<Transaction> transferMoney(Long sourceAccountId, Long targetAccountId, Currency currency, BigDecimal amount) {

        Lock lock = getLock(sourceAccountId);
        try {
            lock.lock();
       Balance balance = getBalance(sourceAccountId, currency);
                System.out.println("BALANCE BEFORE TRANSFER " + balance.getBalance()); 
        //createTransactions here using transactionService.create(transactions)
        accountRepository.refresh(accountRepository.findOne(sourceAccountId))  
        balance = getBalance(sourceAccountId, currency);
        System.out.println("BALANCE AFTER TRANSFER " + balance.getBalance()); 
        return transactions;
       }finally{
           lock.unlock()
        }


}

It mostly works as expected, except when i send multiple parallel requests using apache jmeter. if i send multiple request to transfer 100 USD from account with 1000 balance console outputs something like this:

BALANCE BEFORE TRANSFER 1000
BALANCE AFTER TRANSFER 900
BALANCE BEFORE TRANSFER 1000
BALANCE AFTER TRANSFER 900
BALANCE BEFORE TRANSFER 900
BALANCE AFTER TRANSFER 800
BALANCE BEFORE TRANSFER 800
BALANCE AFTER TRANSFER 700
BALANCE BEFORE TRANSFER 700
BALANCE AFTER TRANSFER 600
BALANCE BEFORE TRANSFER 700
BALANCE AFTER TRANSFER 600

so it mostly works right but at points it doesnt get updated balances. So far i experimented with everything, propagations and isolations. Creating transactions manually and comitting them before thread removed the lock. Nothing seems to work. Before i used

Propagation.REQUIRES_NEW

console output was always

BALANCE BEFORE TRANSFER 1000
BALANCE AFTER TRANSFER 900

Now it sometimes work sometimes not. Its not consistent.

Get Balance method refreshes account using:

accountRepository.refresh()

and transactionService.createTransactions is also annotated with Propagation.REQUIRES_NEW So can anyone tell me why this is not working? At least guide me in the right way.

thank you

---

EDIT: if its not clear enough data is read from DB. using spring jpa.

Answer 1

As answered by GPI and commented by others , get rid of Java locking as benefits will weigh out losses in the long run. This problem is already solved for spring-data-jpa by using - org.springframework.data.jpa.repository.Lock annotation.

Just annotate your repository method used to select account data ( account entity ) for money transfer purposes with - @Lock(LockModeType.PESSIMISTIC_WRITE) . This will lock the selected data till the transaction is complete. I guess, same repository will be used to retrieve from as well as to accounts.

Apply the transfer amounts to both accounts and call save on repository in your @Transactional service method.

Also, it would be advisable to add a non - locking select method to repository too so you can use that when locking is not required i.e. for non - money transfer purposes.

i have Service class to transfer money between accounts, where im using fine-grained locking. Threads gonna lock if they belong to the same account.

With this approach, locking will automatically happen if accounts are same.

Answer 2

The issue is most probably that you have a race between your DB transaction, and the java lock.

Another issue is that you lock only on one of the two accounts, but both sides need to be protected from concurrent access. Which will introduce the possibility of deadlocks.

The scenario for the DB/java lock race would be :

1. The HTTP request hits your controller,
2. @Transaction starts a DB transaction
3. You get a Java lock
4. You perform the operation, nothing is flushed to DB yet
5. You release the java lock, but your controler method has not yet returned, so the JPA transaction is not flushed to the DB
6. Another request comes in, opens a transaction, and "sees the world" as it is, that is, with nothing flushed (e.g. step "0")
7. Whatever happens now, you have two transactions, one of which has a "wrong" view of the world, according to your needs.

Now imagine if on top of that, your program has multiple instances (e.g. failover, load sharing), your java locks would not even work, what a nightmare :-).

The "simple" way to do this (at this level of complexity) would be to "SELECT FOR UPDATE" your entities, which would prevent the interwinding of SELECTS. You would not even need java locks, the SQL engine would provide the locking natively (the second select would not return before the first transaction is commited).

But you would still have a risk of deadlock (if two requests come, one for account A to B, and one from B to C, which opens the possibility of B being locked in the two transactions, you'd have to catch and replay the case, hoping for the conflict to be resolved at that point in time).

You could have a read at : https://www.baeldung.com/jpa-pessimistic-locking to see how you can perform a SELECT FOR UPDATE, which basically entails loading your entity like so :

entityManager.find(Account.class, accountId, LockModeType.PESSIMISTIC_WRITE);

Another possibility would be to reverse the java lock and the @transactionnal. That way you would never hit the DB before being in exclusive mode. But that would leave the issue of having multiple instances of the program in multiple JVMs being unable to share locks. If that’s not your case then this is probably simpler.

On either case you still have to lock on both sides (DB or Java lock) and account for deadlocks.