URLyBIRD: My locking ideas

[Steve Smith]

Hi all,

I have spent quite a long time today thinking about how I will implement locking. But there are still some things that are confusing me a bit.

Here is my idea:
Have a static Vector that stores record numbers that are locked. Synchronize on this Vector before adding or removing a record number from it. This is meant to be similar to Max's book.

But there a few things I am not sure how to do. Do I need to use locking for read operations as well as write operations? For example, I am not sure how to use locking when I want to retrieve all records from the database. My current code to do this works something like this:

get number of records in file
get each individual record using a counter up to the above figure.

In order to do this, what do I need to lock? If I did not use locking (as this is a read operation), I could calculate the number of records, only for this to change whilst I am reading them (for example one is deleted and then RecordNotFound is thrown). If I did use locking, then surely I would need to lock the entire database, as the number of records needs to be the same during the algorithm?

Sorry if this is a little confusing, its probably because I am quite confused myself. Any advice would be much appreciated, or even any questions that can help me further explain my algorithm to you.

And lastly do I need to implement locking in Data.java or can I use an adapter class (like Max's book does?).

[Andrew Monkhouse]

Hi Steve

Have a static Vector that stores record numbers that are locked. Synchronize on this Vector before adding or removing a record number from it.

Vectors are synchronized classes. Are you sure you want to use a synchronized class within a synchronized method? What benefit will it give you?

Do I need to use locking for read operations as well as write operations? [...] If I did not use locking (as this is a read operation), I could calculate the number of records, only for this to change whilst I am reading them (for example one is deleted and then RecordNotFound is thrown). If I did use locking, then surely I would need to lock the entire database, as the number of records needs to be the same during the algorithm?

Playing devil's advocate here: if you did lock the entire database, you could be certain that the records are correct at the moment you read them, however milleseconds after you return the records to the client, they could be out of date due to deletions / bookings / creations. So what does it really gain you?

Also: locking the entire database means that no other client would be able to work on the database. If you are doing this for reading the entire database and/or find methods then you could be potentially blocking multiple clients for long periods of time. Does this fit with the requirement that "Your server must be capable of handling multiple concurrent requests"? (my highlighting)

And lastly do I need to implement locking in Data.java or can I use an adapter class (like Max's book does?).

You might want to check your instructions. If your instructions tell you that your Data class must implement locking, at that that locking must provide certain functionality, then that is what your Data class (or a class it calls) must do.

Note that the Data class can defer to another class if you want it to (so your Data class itself could be an Adapter or a Facade).

Max's book has its own project, with its own requirements - these are not always a perfect match for the Sun assignment (and this was actually a design decision in developing the book: having the book's project and the Sun assignment too similar is "a bad thing®". In addition, the book was written at a time when the "Fly By Night Services" (FBNS) assignment was in vogue, and it had different requirements than URLyBird (and FBNS allowed an adapter class for locking purposes).

Regards, Andrew

[Steve Smith]

Thanks for the reply Andrew,

Vectors are synchronized classes. Are you sure you want to use a synchronized class within a synchronized method? What benefit will it give you?

As I understand it, the fact that Vector is synchronized just means that the methods are atomic and cant be interrupted. There is still the possibility for it being interrupted between methods. For example if I read the vector, then write to it, it can still be interrupted between these two methods.

So I think I do need to synchronize on the collection, but perhaps I can use an ArrayList instead of a Vector and it would still be thread safe? The disadvantage I think would then be that, for example, the vector could be read whilst in the middle of being written to (assuming I don't lock reads). I am not sure what effect this would have.

Playing devil's advocate here: if you did lock the entire database, you could be certain that the records are correct at the moment you read them, however milleseconds after you return the records to the client, they could be out of date due to deletions / bookings / creations. So what does it really gain you?

Also: locking the entire database means that no other client would be able to work on the database. If you are doing this for reading the entire database and/or find methods then you could be potentially blocking multiple clients for long periods of time. Does this fit with the requirement that "Your server must be capable of handling multiple concurrent requests"? (my highlighting)

Some very good points there which I think I agree with on the whole. One concern is how to handle a RecordNotFoundException when I read all records. For example in my algorithm: for getting all the records:

get a list of all record numbers in the file
get each record using the above list (throws RecordNotFound if it has been deleted, as specified in the interface from Sun).

If a record is deleted in between the two steps, then a RecordNotFoundException is thrown. But this would be fairly normal and something I would expect to happen now and then. But I do not want to swallow the exception. I suppose really this is a different topic to locking though.

You might want to check your instructions. If your instructions tell you that your Data class must implement locking, at that that locking must provide certain functionality, then that is what your Data class (or a class it calls) must do.

Note that the Data class can defer to another class if you want it to (so your Data class itself could be an Adapter or a Facade).

I don't have the assignment handy at the moment, so I will check what it says later. My main concern is that automated testing of Data.java will be performed and therefore I can't use a "higher level" class to manage locks.

Max's book has its own project, with its own requirements - these are not always a perfect match for the Sun assignment (and this was actually a design decision in developing the book: having the book's project and the Sun assignment too similar is "a bad thing�". In addition, the book was written at a time when the "Fly By Night Services" (FBNS) assignment was in vogue, and it had different requirements than URLyBird (and FBNS allowed an adapter class for locking purposes).

That makes sense thinking about it. I've pre-ordered the next edition of the book, perhaps it will be more helpful for me, although I'm sure again it will not be too similar to the assignment. I'm using Java5 anyhow so that part will come in handy for me at least
[ September 14, 2005: Message edited by: Steve Smith ]

[Andrew Monkhouse]

Hi Steve,

Originally posted by Andrew Monkhouse:
Vectors are synchronized classes. Are you sure you want to use a synchronized class within a synchronized method? What benefit will it give you?

Originally posted by Steve Smith:
As I understand it, the fact that Vector is synchronized just means that the methods are atomic and cant be interrupted. There is still the possibility for it being interrupted between methods. For example if I read the vector, then write to it, it can still be interrupted between these two methods.

So I think I do need to synchronize on the collection, but perhaps I can use an ArrayList instead of a Vector and it would still be thread safe? The disadvantage I think would then be that, for example, the vector could be read whilst in the middle of being written to (assuming I don't lock reads). I am not sure what effect this would have.

Assuming you don't lock reads, the effects are undefined. Most of the time you will probably be lucky and get the right value back, simply because it is unlikely that the thread swapping will happen at a critical point. The problem is that it is "unlikely" not "impossible" - if the thread reading from the collection swapped out during the read and the collection was added to or deleted from then you might get the wrong value back. Or you might get a NullPointerException. Or, depending on the collection and how it is implemented, you might get a ConcurrentModificationException. No way to tell - this would be a mistake.

So if you are not going to explicitly synchronize for your reads, then using a synchronized collection will give you thread safety for your reads. However using a synchronized collection to get around the non synchronized read means that you will effectively have two synchronized blocks around your code that modifies the collection:

private Vector v = new Vector();
 
public void addAValue(String value) {
    synchronized (v) {
        // do any of [B]your[/B] work that
        // needs to be synchronized
                                            /- public synchronized boolean add(E o) {
                                            |      // some array sizing code goes here
        myCollection.add(value);         --<       elementData[elementCount++] = o;
                                            |      return true;
                                            \- }
        // do any of [B]your[/B] work that
        // needs to be synchronized
    }
}

Doing this can still be a valid design decision. Your code will be easier to read, and you can guarantee that your reads will always be synchronized.

However if you do go down this path, then make it explicit that this was your design choice. Make sure you put it in your design documentation, and in implementation comments. Preferably don't use a Vector: use the Collections.synchronizedXxxx (e.g. Collections.synchronizedList) methods to get a synchronized collection so that it is obvious that you want the collection to be synchronized (rather than somebody thinking that you just picked a Vector as it was 'a collection').

Originally posted by Steve Smith:
One concern is how to handle a RecordNotFoundException when I read all records. [...] I suppose really this is a different topic to locking though.

Agreed

I will say, however, that I think that this is not such a problem when you sit down and think it through. That is - when you start thinking about what conditions could cause this from a specific client's perspective, then you will start thinking about how to handle each of those unique conditions, and you will probably find that blocking all other users from accessing the database might be a bit over the top .

Originally posted by Steve Smith:
And lastly do I need to implement locking in Data.java or can I use an adapter class (like Max's book does?).

Originally posted by Andrew Monkhouse:
You might want to check your instructions. If your instructions tell you that your Data class must implement locking, at that that locking must provide certain functionality, then that is what your Data class (or a class it calls) must do.

Note that the Data class can defer to another class if you want it to (so your Data class itself could be an Adapter or a Facade).

Originally posted by Steve Smith:
My main concern is that automated testing of Data.java will be performed and therefore I can't use a "higher level" class to manage locks.

Automated testing (or the lack thereof ) is a separate subject as well. But whether it is automated testing, or an assessor just reading your code is pretty much irrelevant in my opinion. What matters is what your instructions require from you. If your instructions say that another class should be able to call Data's lock method and gain a lock so that no other client can lock or modify the record, then in my opinion that is what must happen. Data's lock method can still call other lower level classes (becoming an adapter or a facade), but in my opinion having a higher level class do the work instead of the Data class is not allowed.

But just for the record: There has been at least one candidate who passed using a "higher level" class to manage locks with the instructions that have come out after Max's book. From memory he spent considerable time explaining why he did this. It is not a risk I would want to take with my money.

Regards, Andrew

[Steve Smith]

Thanks again Andrew,

So I think thats cleared up most of my questions. I'm not sure I expressed very well what I meant by using an adapter class. I want to implement lock() and unlock() in Data.java which is what my instructions say, but then I do not want to call these within the other methods in Data.java. I want a higher up class that manages calls to lock and unlock in Data.java before whilst calling the other methods in this class.

Please ignore the last paragraph if this is what you thought I wanted to do anyhow

[Steve Smith]

After coming back to this I have a few more questions.

1. How can locking work with the create() method, as the record does not exist to lock when you call create?

2. With regards to the raf, should I be creating a new one in each method in Data, or should I use an instance variable and synchornize all methods that use it? Bearing in mind I intend to have multiple instances of the Data class.

What I was currently thinking is the following:
Have both a static raf and an instance variable raf. The create method synchronizes on the static raf, which means that the create method can only be running once for all Data instances at any one time. This provides thread safety for the create method. The rest of the methods use the instance variable raf and are all synchronised to prevent corruption of the raf. Thread safety is provided via lock() and unlock() for these methods. So basically I am treating create() as a different case with seperate locking strategy. Is this a good way to do it, or is there a better way?

Another option is to just synchronise on my lockedRecords Vector within the create method. However this will stop the user from modifying any records whilst the create operation takes place. This seems to be a simpler solution to code but worse for peformance.

Many thanks for any help. If my questions are unclear please say and I will rephrase them.
[ October 28, 2005: Message edited by: Steve Smith ]

[Steve Smith]

Sorry for talking to myself a bit here, It's just I have done a fair bit on the assignment today. I decided to have just an instance variable raf. In create, I synchornise on my Vector of locked records, but only until I have marked the new record as active and then locked it. The rest of the method can then carry on safely.

I am now starting to get a bit worried about Deadlock though. If I have an instance variable RAF then I think Im going to need to synchronize on it to prevent it getting corrupted, but at the same time I'm synchronizing on my Vector of locked records. I haven't seen any deadlock but surely it's possible that this could lead to deadlock? I could ensure a certain order of synchronization, but is this bad practice? Also, if there is one instance of Data.java per client, do I actually need to synchronise the raf?

Any thoughts appreciated
[ October 28, 2005: Message edited by: Steve Smith ]