• Post Reply Bookmark Topic Watch Topic
  • New Topic

Java Concurrency Question  RSS feed

 
Ian Norris
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,

I need some help on the a Java interview question on currency.
A friend of mine recently interviewed for a Java position and during the interview, he was presented some code for a fictitious car rental system.
He was asked to identify and fix, among other errors, the concurrency issues with the code. He couldn't quite do it at the interview.
Here is my proposed solution to the problem, but I am not sure if this works and will appreciate some advice.

The code skeleton is given below (some details are omitted for brevity):

Given classes:



The method rentCar() is not thread safe as one thread could see a car is availabe for hire but before the booking finishes,
another thread could see the same car being availabe and booked it before the original thread. Thus the first thread could override the 2nd thread's booking.
He suggested that he could make synchronized both the methods: rentCar() and returnCar(). This would make it thread safe.

The problem then is that the rentCar() method would be called by many requests. Making it serial will hit the server performance hard,
and he knew he would have to make it multi-threaded, but struggled on how to do it.

Here is my solution, the idea is:
1) Use multiple threads in a thread pool to increase performance. Create hiring and releasing tasks as Callable.
2) To avoid the situation where a car may be booked by multiple threads, guard the booking code with a lock so booking on a car
can only be done by one thread.
3) Use a ConcurrencyHashMap to stores a lock for each Car identified by its registration key. Any thread wants to book a car will
have to acquire the lock for the car before a booking can be made. Given the ConcurrentHashMap is thread safe and locks on its segments,
it will enable many requests being run concurrently, thus improve performance.

The code is listed below and I'll appreciate comments on whether it works; how to safely remove the individual locks? Alternative solutions?



 
Stephan van Hulst
Saloon Keeper
Posts: 7962
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There's no point in using an executor service. Your requests are already handled concurrently by the application server, and adding more threads to the mix just increases overhead.

Honestly, the initial setup was doomed from the start. All objects should be immutable, and the service should let locking be handled by the database, through the use of transactions.
 
Stephan van Hulst
Saloon Keeper
Posts: 7962
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to CodeRanch!
 
Ian Norris
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Stephan,
Thank you for the reply.
I agree with you that in a real system, using database transaction would have been the more straight forward solution.
But given this was an interview question and the interviewers said that the DataService was from an external jar and that's the only thing one could use for interacting with the database and the DataService has only those 3 methods given, which apparently has no transaction facility.
Presumably, they want test a candidate's ability of developing concurrency  code? With all those constraints, will this solution work?

Ian
 
Stephan van Hulst
Saloon Keeper
Posts: 7962
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, I would probably first find out if I can abuse the loadFromDb() method to create transactions anyway, and then write a facade around that.

Assuming they want to test your concurrency chops though, having a map of locks seems like a decent first approach. However, as I've said before there is absolutely no point to having the bookings performed by a separate executor service (as a matter of fact, it's very bad practice to spawn threads in a web application).

You also shouldn't be using the synchronized statement, or Object for locks. Java introduced a high level concurrency library in SE5, I believe. You might want to take a look at java.util.concurrent.locks.ReadWriteLock for this one.

I'll look at this some more tomorrow, I'm on my phone right now.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!