2

For concurrency/multithreading learning purposes, I am developing a small money transfer API which will be concurrently invoked by several users. My "database" is a ConcurrentHashMap<String, Double>, which key/value pair represents an account id and its current balance.

I know that single operations of a ConcurrentHashMap (get(), put() etc.) are thread-safe, but a withdraw/deposit method would have several method calls that would ultimately make it not thread-safe.

My problem: how to design my withdraw/deposit methods to be thread-safe? At first, I thought about making them synchronized, but that doesn't make any sense as I would be throwing away the fine grained built-in mechanism of synchronization of the ConcurrentHashMap.

These are both my withdraw and deposit methods (don't worry about Double for money here, that's irrelevant in this context):

private void deposit(ConcurrentHashMap<String, Double> myDatabase, String fromAccountId, String toAccountId, double amount) {
    if(myDatabase.get(fromAccountId) < amount) {
        throw new MonetaryOperationViolation("Insufficient funds to perform this operation");
    }

    //Add the amount to the receiver's account
    myDatabase.replace(toAccountId, myDatabase.get(toAccountId), c.get(toAccountId) + amount); //key, oldValue, newValue

    //Withdraw it from the sender's account
    withdraw(myDatabase, fromAccountId, amount);
}

private void withdraw(ConcurrentHashMap<String, Double> myDatabase, String accountId, double amount) {
    if(myDatabase.get(accountId) < amount) {
        throw new MonetaryOperationViolation("Insufficient funds to perform this operation");
    }

    myDatabase.replace(accountId, myDatabase.get(accountId), myDatabase.get(accountId) - amount);
}

I hope I've made myself clear regarding my issue. Any help would be truly appreciated.

StatelessDev
  • 294
  • 4
  • 15
  • 1
    I think if you want to go with the solution you have now you have to be very careful with what methods you use and what they return. Someone asked a similar question before and it had some pretty good answers : https://stackoverflow.com/questions/14947723/is-concurrenthashmap-totally-safe (For example: In a multi-thread environment, this is a race condition. You have to use the ConcurrentHashMap.putIfAbsent(K key, V value) and pay attention to the return value, which tells you if the put operation was successful or not. Read the docs for more details.). Hope it helps – GamingFelix Feb 04 '19 at 15:45
  • 1
    As a note, there is nothing too fancy in the implementation of a `ConcurrentHashMap`. We just wrap everything in a `synchronized` block... – Sofo Gial Feb 04 '19 at 15:46
  • 2
    Re, "...I would be throwing away the fine grained built-in mechanism of synchronization of the `ConcurrentHashMap`..." Sounds like your application _must_ have explicit synchronization if it's going to work. It's up to you to figure out whether `ConcurrentHashMap` adds any additional benefit over just using a regular `HashMap` under the circumstances. I have seen `ConcurrentHashMap` used in programs that weren't even multi-threaded, but I no longer remember the developer's explanation of why he made that choice. – Solomon Slow Feb 04 '19 at 16:25

2 Answers2

2

I don't think that it is possible to solve such task just using ConcurrentHashMap with some atomic type.

Imagine a case when money from one account has been transferred to another one. In this case you need to synchronize not on one Map element but on two account simultaneously. This is called transaction. So what you need to do is implement transactions. Transaction should lock all affected acounts and release them after finishing.

As another option you can just create thread-safe queue with transactions and do all transactions sequently and you will not need nor ConcurrentHashMap nither synchronization, however probably this is not about a part which you are trying to study.

mger1979
  • 109
  • 6
1

Java internals have many solutions for concurrency, in order to use the right one you need answer a simple question: What my application does most of the time? Read or Write operations?

In case it performs writes (withdraw/deposit) I would recommend to use java.util.concurrent.atomic.DoubleAdder instance instead of Double this will ensure the thread safety and increase your application throughput in aspect of writes.

In general, such kind of applications suits to Actors model. Each account can be represented by an actor. The actor will support few message types such: withdraw/deposit/total. AKKA framework is an excellent implementation of actor model.

Maxim Kirilov
  • 2,639
  • 24
  • 49
  • Supposing a real-life API, could LongAdder replace BigDecimal for monetary values? Is it a standard practice? – StatelessDev Feb 04 '19 at 16:16
  • How would the actor model (with each account having their own actor) help with the atomicity of a transaction that withdraws from one account and deposits in another? How would a DoubleAdder (presumably also one for each account?) help? – Thilo Feb 05 '19 at 06:49
  • Just revert the order first withdraw then deposit... In the actor model, you can create an actor that conducts any transaction, first performs a withdraw from one account (sending message to some actor and waiting for response) then performs the deposit. In case the deposit fails it can always refund the amount for example. – Maxim Kirilov Feb 05 '19 at 07:06