Concurrency Handling : Synchronization vs Multiple DB Calls

Question

Imagine a MNC bank who wants to implement the account transfer API using core Java only and API would be used in multiple threaded environment, and maintain the consistency of account's amount all the time with no deadlock of-course

I have two approach in mind to implement this and know their pros and cons, as described below, but not able to measure which pros is more good here in case of concurrency or when we have more load. Or, which cons should effect more in case of concurrency or when we have more load.

Kindly advice and provide your suggestion.

Full code Github repository link https://github.com/sharmama07/money-transfer

API prototype: public boolean transferAmount(Integer fromAccountId, Integer toAccountId, Integer amount);

Approach 1: Handling the concurrency via while loop and SQL statement to check for previous balance in where clause. If previous balance is not same, update amount call on account will fail and it will fetch the latest account's amount from DB and try for update again till it updated it successfully. Here is no thread will be locked, that means no chance of deadlock, no thread suspension overhead and no thread latency, But it might have few more DB calls

public boolean transferAmount(Integer fromAccountId, Integer toAccountId, Double amount) {

        boolean updated = false;

        try {
            while(!updated) {

                Account fromAccount = accountDAO.getAccount(fromAccountId);

                if(fromAccount.getAmount()-amount < 0) {throw new OperationCannotBePerformedException("Insufficient balance!");}

                int recordsUpdated = accountDAO.updateAccountAmount(fromAccount.getId(), fromAccount.getAmount(), 
                        fromAccount.getAmount()-amount);
                updated = (recordsUpdated==1);

            }
        }catch (OperationCannotBePerformedException e) {
            LOG.log(Level.SEVERE, "Debit Operation cannot be performed, because " + e.getMessage());
        }

        if(updated) {
            updated = false;
            try {
                while(!updated) {
                    Account toAccount = accountDAO.getAccount(toAccountId);
                    int recordsUpdated = accountDAO.updateAccountAmount(toAccount.getId(), toAccount.getAmount(), toAccount.getAmount()+amount);
                    updated = (recordsUpdated==1);
                }
            }catch (OperationCannotBePerformedException e) {
                LOG.log(Level.SEVERE, "Credit Operation cannot be performed, because " + e.getMessage());
                revertDebittransaction(fromAccountId, amount);
            }
        }


        return updated;
    }

// Account DAO call
@Override
    public Account getAccount(Integer accountId) throws OperationCannotBePerformedException {
        String SQL = "select id, amount from ACCOUNT where id="+accountId+"";
        ResultSet rs;
        try {
            rs = statement.executeQuery(SQL);
            if (rs.next()) {
                return new Account(rs.getInt(1), rs.getDouble(2));
            }
            return null;
        } catch (SQLException e) {
            LOG.error("Cannot retrieve account from DB, reason: "+ e.getMessage());
            throw new OperationCannotBePerformedException("Cannot retrieve account from DB, reason: "+ e.getMessage(), e);
        }

    }

    @Override
    public int updateAccountAmount(Integer accountId, Double currentAmount, Double newAmount) throws OperationCannotBePerformedException {
        String SQL = "update ACCOUNT set amount=" + newAmount +" where id="+accountId+" and amount="+currentAmount+"";
        int rs;
        try {
            rs = statement.executeUpdate(SQL);
            return rs;
        } catch (SQLException e) {
            LOG.error("Cannot update account amount, reason: "+ e.getMessage());
            throw new OperationCannotBePerformedException("Cannot update account amount, reason: "+ e.getMessage(), e);
        }
    }

Approach 2: Here other threads will be locked if same account is in two transaction under different thread,
But it would have lesser DB calls

    public boolean transferAmount1(Integer fromAccountId, Integer toAccountId, Double amount) {

            boolean updated = false;
            Integer smallerAccountId = (fromAccountId<toAccountId)? fromAccountId: toAccountId;
            Integer largerAccountId =  (fromAccountId<toAccountId)? toAccountId:fromAccountId;

            synchronized(smallerAccountId) {
                synchronized(largerAccountId) {
                    try {
                        Account fromAccount = accountDAO.getAccount(fromAccountId);
                        if(fromAccount.getAmount()-amount < 0) {
                            throw new OperationCannotBePerformedException("Insufficient balance!");
                        }
                        int recordsUpdated = accountDAO.updateAccountAmount(fromAccount.getId(),
                            fromAccount.getAmount(), fromAccount.getAmount()-amount);
                        updated = (recordsUpdated==1);
                    }catch (OperationCannotBePerformedException e) {
                        LOG.log(Level.SEVERE, "Debit Operation cannot be performed, because " + e.getMessage());
                    }
                    // credit operation
                    if(updated) {
                        try {
                            updated = false;
                            Account toAccount = accountDAO.getAccount(toAccountId);
                            int recordsUpdated = accountDAO.updateAccountAmount(toAccount.getId(),
                                toAccount.getAmount(), toAccount.getAmount()+amount);
                            updated = (recordsUpdated==1);
                        }catch (OperationCannotBePerformedException e) {
                            LOG.log(Level.SEVERE, "Credit Operation cannot be performed, because " + e.getMessage());
                            revertDebittransaction(fromAccountId, amount);
                        }
                    }
                }
            }

            return updated;
    }

Answer 1

I suppose we're assuming for some reason DB transactions are not usable for this scheme. Because with DB transactions none of this is necessary.

The first scheme should work but it is unsafe. If the first update loop works bu second fails, the first account loses money without depositing it to the second one. There are better ways to handle it though. read-operate-update loops are inherently racy, so you can change the updateAccountAmount function to get a delta instead of the final value, and update the amount if final amount is >=0. That is set amount=amount+delta where amount+delta>0. This would both check and update. You won't need the while loops, because if the first operation fails then there is no point in retrying.

The second implementation, the way it is written, is wrong. You would need to synchronize on a shared object corresponding to the from-account number and to-account number to prevent simultaneous updates. You can use a HashMap to get the locking object for the ID, and then lock that object. You have to synchronize the access to the hashmap, and once you get the two objects, you have to lock the Ids in the same order for all threads to prevent deadlocks (which you seem to have done already, but your synchronization is on a stack variable, hence won't prevent any concurrent access). However, once you fix that, it should work as long as you only have one instance of this application running. Then you'll have the problem of dealing with an ever-growing hashmap.

A simpler scheme is to use locking. You can use a lock set shared among threads with Integer keys corresponding to account numbers. Since the locking method is synchronized, you don't need to worry about locking orher. Then:

public boolean lock(Integer id1, Integer id2) {
  synchronized(lockSet) {
    if(!lockSet.contains(id1) && !lockSet.contains(id2)) {
       lockSet.put(id1)
       lockSet.put(id2)
       return true
    }
    return false
  }
}

public void unlock(Integer id1, Integer id2) {
  synchronized(lockSet) {
     lockSet.remove(id1)
     lockSet.remove(id2)
  }
}

Then lock before database operations, and unlock after them. This prevents entry of any other thread working with those account ids. This scheme suffers from the same problems others do, that if the first update succeeds and second fails, you'll lose money.