0

I have a REST application written using Spring Boot, in which I have a wallet. The wallet has methods like addAmount, deductAmount and so on. Here's the code:

WalletController.java

public class WalletController {
    public final LoadDatabase loadDatabase;
    private final WalletRepository repository;

    @Autowired
    public WalletController(LoadDatabase loadDatabase, WalletRepository repository) {
        this.loadDatabase = loadDatabase;
        this.repository = repository;
    }

    @GetMapping("/addAmount")
    @ResponseBody
    public void addAmount(@RequestParam Long custId, @RequestParam Long amount){
        try{
            Wallet wallet = repository.findWalletsByCustId(custId).get(0);
            wallet.balance = wallet.balance+amount;
            repository.save(wallet);
        }catch(IndexOutOfBoundsException e){
            //handle exception
        }
    }

    @GetMapping("/deductAmount")
    @ResponseBody
    public boolean deductAmount(@RequestParam Long custId, @RequestParam Long amount){
        try{
            Wallet wallet = repository.findWalletsByCustId(custId).get(0);
            if(wallet.balance < amount)
                return false;
            wallet.balance = wallet.balance-amount;
            repository.save(wallet);
            return true;
        }catch(IndexOutOfBoundsException e){
            return false;
        }
    }

    // some other methods.
 

This is going to be concurrently accessed and hence, I want to make both addAmount and deductAmount atomic in nature.

To check this, I wrote a shell script which adds and deducts some amount concurrently.

wallet_test.sh

#! /bin/sh

# Get the balance of Customer 201 before.
balanceBefore=$(curl -s "http://localhost:8082/getBalance?custId=201")

echo "Balance Before:" $balanceBefore

sh wa1 & sh wa2
wait    

# Get the balance of Customer 201 afterwards.
balanceAfter=$(curl -s "http://localhost:8082/getBalance?custId=201")

echo "Balance After" $balanceAfter

where wa1 and wa2 are as follows:

for i in {0..10};
do
#   echo "Shell 1:" $i
    resp=$(curl -s "http://localhost:8082/addAmount?custId=201&amount=100")
done
for i in {0..10};
do
#   echo "Shell 2:" $i
    resp=$(curl -s "http://localhost:8082/deductAmount?custId=201&amount=100")
done

The output, as expected due to concurrent access, is something of the form:

Balance Before: 10000
Balance After 9900

Balance Before: 10000
Balance After 10300

Balance Before: 10000
Balance After 9600

The expected output for me is that the balance before and after should remain the same i.e 10000.

Now, I've read that to make it atomic, we can make use of the @Transactional annotation and that we can add it to both the methods, or to the entire class. I tried doing both, and yet, I am not getting the results I desire.

I added it at the method level i.e

    @GetMapping("/deductAmount")
    @ResponseBody
    @Transactional(isolation = Isolation.SERIALIZABLE)
    public boolean deductAmount(@RequestParam Long custId, @RequestParam Long amount){

and same for deductAmount, which didn't work.

I tried adding it at the class level i.e

@Controller
@Transactional(isolation = Isolation.SERIALIZABLE)
public class WalletController {
    public final LoadDatabase loadDatabase;
    private final WalletRepository repository;

and this didn't work either.

Is @Transactional not meant to be used this way? Should I use some other locking mechanism in order to accomplish what I want?

EDIT:

As mentioned, I tried adding Pessimistic locks as well.

    import static javax.persistence.LockModeType.PESSIMISTIC_WRITE;

    @PersistenceContext
    private EntityManager em;


    @GetMapping("/addAmount")
    @ResponseBody
    @Transactional
    synchronized public void addAmount(@RequestParam Long custId, @RequestParam Long amount){
        try{
            Wallet wallet = repository.findWalletsByCustId(custId).get(0);
            em.lock(wallet, PESSIMISTIC_WRITE);
            wallet.balance = wallet.balance+amount;
            repository.save(wallet);
        }catch(IndexOutOfBoundsException e){
            //handle exception
        }
    }

    @GetMapping("/deductAmount")
    @ResponseBody
    @Transactional
    synchronized public boolean deductAmount(@RequestParam Long custId, @RequestParam Long amount){
        try{
            Wallet wallet = repository.findWalletsByCustId(custId).get(0);
            em.lock(wallet, PESSIMISTIC_WRITE);
            if(wallet.balance < amount)
                return false;
            wallet.balance = wallet.balance-amount;
            repository.save(wallet);
            return true;
        }catch(IndexOutOfBoundsException e){
            return false;
        }
    }
Gokul
  • 227
  • 2
  • 12
  • I remember using it on methods, with default value , (I mean : `@Transactional` only, not `@Transactional(isolation=...)`). Also, I used it in the service layer, not in controllers. but it seams that you call repositories directly in the controllers... that should work anyway. If you want to test it more easily, you can make a request then throw an exception and see if the result was committed. – Arnaud Denoyelle Apr 09 '21 at 12:47
  • @ArnaudDenoyelle I tried doing that and I get the same results. – Gokul Apr 09 '21 at 12:55

1 Answers1

1

It's not enough to set the isolation level. You should use optimistic or pessimistic locking to achieve the behaviour you want. Nice and short description of this strategies can be found in this answer.

Sergey Vasnev
  • 783
  • 5
  • 18
  • I did that, and my results are still incorrect. I've added my edited code in the original question. – Gokul Apr 09 '21 at 16:57
  • I think you should modify your locking approach in respect using @Lock annotation in your Wallet repository class, for example. In your code example read and lock are separate operations which can allow concurrency issues like: Thread T1 reads wallet's value 1000 and at the same time thread T2 reads wallet's value 1000 when T1 locks and updates the wallet with 1000-100, when T2 unlocks and updates it's version of the wallet with 1000+100. TO avoid this row should be locked during select operation. In a db level it can be achieved using SELECT *... FOR UPDATE – Sergey Vasnev Apr 09 '21 at 17:48