aspose file tools*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes Data class thread safe using a RecordLockingManager and a DataAccessFile Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "Data class thread safe using a RecordLockingManager and a DataAccessFile" Watch "Data class thread safe using a RecordLockingManager and a DataAccessFile" New topic
Author

Data class thread safe using a RecordLockingManager and a DataAccessFile

Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Hi all,

I'm working on the Bodgitt and Scarper assignment and I have to say that dealing with the cookies is bugging me a bit (I had to change my thin client for a think client) .

I thought that everything in my assignment was almost completed but I found a flaw in my approach.

This is the lock method given by Oracle.


and this is the update method.


I opted to follow the facade/adapter approach and implement my DB interface to have a Data class. Now this class only has to delegate each one of the methods to the more specific classes RecordLockingManager and DataAccessFile. This two classes are thread safe by themselves, ... are not singletons but they use the concurrent package as an alternative.

I already used Roberto's tool to check that there are not deadlocks or something wrong in the application performance. However when I was refining my decision choices I checked my code again:



The read method is thread safe because it goes directly to the getRecord method of the DataAccessFile class that is thread safe internally.

But I think we can not say the same in the update method (the same goes for the delete method). Even though the RecordLockingManager and the DataAccessFile are thread safe this operation is not atomic (first reads and then updates). Of course the chances that two threads want to lock the same record and update the record with the same cookie are almost zero.

In my services layer there is no problem at all because I use the almighty solution: lock, update, unlock; or lock, delete, unlock. But I'm concerned about losing points because of this defect in the Data class.
I would like to have it thread safe by itself.

I was thinking to pass a reference of the RecordLockingManager to the DataAccessFile and then in the update method ask for the cookie. This way I can perform the cookie retrieving inside the DataAccessFile and then ensure that these two actions (get cookie and record update) are atomic. But I don't know if it would be a little messy because my original idea was that these two classes were independent from each other, one gives cookies and the other one receives, but if I pass a reference to the other it will be coarse grained.

Another alternative is to synchronize all the Data methods but then it would be no use having my other two classes thread safe (And change my design ). Hope you can give me your opinions.

Regards!!



Himai Minh
Ranch Hand

Joined: Jul 29, 2012
Posts: 757
Do you lock or synchronize the methods that perform reading the db file or updating the db file?
To my understanding, your code pass Roberto's test without deadlock and without updating the db file or reading the file at the same time.
And you want to make sure it is still thread safe?


For your update / delete method, do you seek the right location for your record and update / delete it? You don't need to read the whole db file in order to find the record to update/delete. You can seek the location of the record and update it. But make sure you synchronize this update/delete method.

When I work on my assignment, my concern was this scenario:
1. thread 1 locks record 1
2. thread 2 locks record 2
3. thread 1 updates record 1 while thread 2 updates record 2.
4. thread 1 unlocks record 1
5. thread 2 unlocks record 2
My concern was in step 3. Two threads are updating the same file at the same time. To avoid this, I synchronize the update/delete methods that access the db file.

I hope this helps.
Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Thanks for your reply. The Data class itself is not thread safe so to speak, but are the classes that actually do the real work: access the database file (DataAccessFile) and lock the records(LockingRecordManager).

I'm not locking the whole database each time a client access to it, instead, the methods in the DataAccessFile and LockingRecordManager are synchronized using a ReentrantReadWriteLock at the beginning and at the end of the methods (with the try, finally statement).

My concern was in step 3. Two threads are updating the same file at the same time. To avoid this, I synchronize the update/delete methods that access the db file.


I changed a the test a bit and also added the part you mentioned, when 2 threads are accessing two different records at the same time (reading, updating and deleting) and passed without problem. Thinking carefully, if the code passed Roberto's test I could guarrantee that the Data class is thread safe .

My only concern would be that Oracle doesn't like the fact that the update method (get cookie and update the record) is not atomic.



Regards!!

Himai Minh
Ranch Hand

Joined: Jul 29, 2012
Posts: 757


You can use synchronize keyword or an ReentrantLock to lock the code inside this update method.
As suggested by KB's SCJP 6.0 study guide, it is a good practice to lock a block of code to make it thread safe even though each individual step is thread safe.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5265
    
  13

Can you think of a scenario in which your non-atomic update-method could potentially cause a problem? At 1st sight (and thought) I can't come up with such a scenario...


SCJA, SCJP (1.4 | 5.0 | 6.0), SCJD
http://www.javaroe.be/
Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Thanks both Himai and Roel

Indeed, I was thinking in one potential scenario in which two threads enters the update methods and cause problems because the operation is not atomic:
1.- When two threads want to update the same record. It can't happen because one of them had to lock the record previously then the other thread has to wait for the record to be unlocked. However, I'm relying on the programmer to use the lock, update, unlock structure.
2.- When two threads want to update different records (to put the things more interesting, suppose that they are going to get the same cookie number):

Thread 1 has locked record 1 and got a cookie = 123456
Thread 2 has locked record 2 and got a cookie = 123456 (In the real application I'm using a random number in the order of 1x10^6)
Thread 1 enters the update method
Thread 2 enters the update method
Thread 1 gets its cookie
Thread 2 gets its cookie
Trhead 1 enters the DataAccessFile update method. Passes the cookie check test and books the record 1. (map.get(1) = 123456)
Thread 2 enters the DataAccessFile update method. Passes the cookie check test and books the record 2. (map.get(2) = 123456)
Thread 1 exits the update method.
Thread 2 exits the update method.
Thread 1 unlocks the record 1.
Thread 2 unlocks the record 2.

Result, no corrupt data. At first I thougth that having the same cookie would cause havoc but actually no.

In the end, I think that we can say that the method is thread safe but as Himai says, it is a good practice to have the methods synchronized. Now if I do that and have my Data methods synchonized, then they would say "What's the use to have your underlying classes thread safe then? ... because in the end the Data class is the only one that matters and is also synchronized. You are synchrozing all the classes (the Data, DataAccessFile, RecordLockingManager) and actually your design seems somewhat cluttered".
So I could symply have my data class synchronized and leave my other two classes normal, but if I do so the facade pattern would be weird because I would be adding the synchonizing part (and I wanted my two working classes to be synchronized).

Also I was trying to come up with another solution. To inject the LockingManagerClass in the DataAccessFile and say that they have two different taks, but one compose the other. They might say that they are tigthly coupled. So I would say "That's not true because LockingManager is an interface ".

The Data class would look like this:

And the DataAccessFile would look like this.


Now my Data class follows only the facade pattern, the adapter pattern is eliminated because the both update methods have the same parameters and the only classes that are synchonized are the DataAccessFile and RecordLockingManager as was the original idea. What do you think about this approach? (Sorry that I used the word "synchronized" 1000 times).
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5265
    
  13

My initial approach was exactly the same: I also wanted to use the Data class as a facade for 2 worker classes. But because of the issues you described here (and the quite simple logic) I opted to remove the worker classes from my design and just let the Data class perform all necessary logic.

But the solution you described using the interfaces (to prevent tight coupling) is a nice one. If I have to redo the assignment and must implement a facade pattern for the Data class my design will be similar to what you have described here.
Himai Minh
Ranch Hand

Joined: Jul 29, 2012
Posts: 757
In your database.updateRecord method, do you use synchronize keyword or ReentrantLock to make sure only one thread can update the record in the database file?
If so, I believe it is good to go.
Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Thank you both, with your permission I will implement the interface solution then. By the way, I was looking for other solutions here but I could not find if someone had posted this question before. I think is the only feature I didn't know how Roel had resolved .
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5265
    
  13

Cesar Cardozo wrote:I think is the only feature I didn't know how Roel had resolved .

And now you know
Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Hi again, well I have another question regarding this design. I was reading the book Head First Design about the correct implementation of facade pattern. As I had commented before my Data class is the facade and LockingManager and DataAccess are interfaces whose implementation will be the working classes.
Accoding to the book the facade pattern hides the complex interaction that may have this working classes. As I could see in the examples these classes are passed to the constructor of the facade.
However, they also introduce the Principle of the Least Knowledge in which the client only has to know the Data class. In the diagram it seems that the working classes are hidden to the client, but there are no implementation examples.
My question is: In my current approach the LockingManager and DataAccess implementations have to be first created by the client and then passed to the Data class costructor (Dependency Injection). But this means that the client not only knows the Data class but also the working classes. What I am concern about is if this violates the Principle of the Least Knowledge and in this particular case I should hide the existence of the working classes in the Data costructor.

Regards!!
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5265
    
  13

It's quite weird that with a Facade the client 1st has to create the worker classes. That makes no sense at all. Because that's counter intuitive when using a facade. Its purpose is just to hide complex logic or a complex class hierarchy and provide a simple, clean, easy to use API.
Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Yes, that's what I thought. Throughout the chapter "Being Adaptive: the Adapter and Facade Patterns" they pass all kind of dependencies to the constructors of the adapter and the facade classes, but I believe that at the end they also introduced the Principle of the Least Knowledge (or Low of Demeter) to emphasize that in the case of the facade pattern, the dependencies have to be hidden in the constructor (they don't mention this explicitly in one example but that is what I concluded) so that the client only knows one friend, the facade class.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5265
    
  13

If a client only knows 1 friend, the classes will also be loosely coupled. And if someone else (or you) needs to write a new implementation of Data class (using a MySQL database for example) you should only require to change Data class (and not your client).
Cesar Cardozo
Greenhorn

Joined: Jun 30, 2013
Posts: 14
Interesting!!!. I had missed that point. Thanks again Roel
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5265
    
  13

Glad I could give you another insight!
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Data class thread safe using a RecordLockingManager and a DataAccessFile