Synchronization of public methods in DVDDatabase

[John Canavan]

Apologies if this question has been answered before…..In Max’s book he makes all the public methods in DVDDatabase synchronized. I was wondering why he did this as he is also giving each client its own DVDDatabase object and is using the vector reservedDVDs to keep track of the records that are locked.
Is it because he wants to implement the method as a single atomic operation, or is there more to it?
Thanks in advance,
John

[Andrew Monkhouse]

Hi John
Does this answer your question?
Regards, Andrew

[Arun Kumar]

Hi John,
I had a similar problem. I was following max's example thinking that his example was creating individual instance of DVDDatabase for each client accessing the server. But after writing the code, i was priting the instance of the DVDAdaptor, and was found that all the clients were printing out the same instance. I asked this question under the topic RMI confusion. Bharat explained how RMI works and how to get multiple instances of DVDDatabase.
Arun

[Bharat Ruparel]

Hello John,
If I read correctly what Andrew is saying here:
1. You DO have to implement LOGICAL record locking - this means implementing the lock, unlock, and islocked method and use them in collaboration with OTHER public methods to achieve the desired functionality.
2. Regarding the synchronization of other public methods such as read, update, find, delete, create - it seems to me that Andrew is recommending NOT to synchronize them. I am not so sure, I have synchronized them to be on the safe side. However, it will have a performance impact that can be substantial.
Perhaps Andrew can explain why ?
Andrew, I am going to try to articulate why the method level synchronization for the aforementioned methods may not be necessary. Here we go: Since we use multiple Data instance design where one Data instance is assigned to one client, we know that "different" clients will not clash with each other. Even though RMI does not guarantee "one" thread to execute the methods of an instance of the Data class, since each instance is isolated from the "other" executing instances and since the LOGICAL record level locking is implemented, we should not synchronize read, upate, delete, find, and create methods?
Regards.
Bharat

[Arun Kumar]

Hi Bharat,
Saw your reasons for not synchronizing the public methods (other than lock,unlock,islocked). Since each client has its own instance of data, it makes sense.
I remember reading in some other thread that you are using the same RAF handle for all your clients. if you dont synchronize your public methods or synchronize on some object, wont your RAF seek/position be changed by another clients update/read.... I am actually using a static instance of RAF in the data class and the read/write methods are synchronized on this RAF instance. Does this make sense?
Arun

[Arun Kumar]

Bharat,
saw that you are using datahelper class and synchronizing the methods there.
Can ignore my Q above.
Arun

[John Canavan]

Thanks for your responses. I took yesterday off, so sorry for not replying….
Andrew,
The link to the thread did not really supply the answer to the question. Vlad in this thread seemed to think that:

There is only one instance of DVDDatabase used.

However in the book this does not seem to be the case (page 127):

..several instances of the DVDDatabase object can exist…

Hi Arun,
I don’t think the thread you pointed me to answers my question either.
Hi Bharat,
From reading your comments it looks like you are saying that there is no need to make these public methods synchronized. Is this the route you will take?
John

[Andrew Monkhouse]

Hi Bharat,

Andrew, I am going to try to articulate why the method level synchronization for the aforementioned methods may not be necessary. Here we go: Since we use multiple Data instance design where one Data instance is assigned to one client, we know that "different" clients will not clash with each other. Even though RMI does not guarantee "one" thread to execute the methods of an instance of the Data class, since each instance is isolated from the "other" executing instances and since the LOGICAL record level locking is implemented, we should not synchronize read, upate, delete, find, and create methods?

Exactly right.
Regards, Andrew

[Andrew Monkhouse]

Hi John
The section you quoted from Max's book was in the threading section, where the assumption was that "several instances of the DVDDatabase object can exist".
However when you get to the RMI section (or look at the real code) you will see that this is not the case - there can be only one.
Since there is only one instance of the DVDDatabase object, then the synchronization is a good thing.
But in Bharat's case where there will be more than one instance of the Data class, having the methods within the Data class synchronized will have a (minor) performance hit with no benefits.
Regards, Andrew

[Bill Robertson]

Since there is only one instance of the DVDDatabase object, then the synchronization is a good thing.
But in Bharat's case where there will be more than one instance of the Data class, having the methods within the Data class synchronized will have a (minor) performance hit with no benefits.

Thought I had this all done. Andrew could you try to elaborate. In my opinion it seemed like polar opposite. What am I missing?

[Bharat Ruparel]

Hello John,
You asked:

From reading your comments it looks like you are saying that there is no need to make these public methods synchronized. Is this the route you will take?

Please see Andrew's comments. Understand that my own thoughts are evolving as I "learn" from from all of us: yourself, Ulrich, Andrew, Arun, Vlad, Phil, Mark for example. Our basic instinct is to liberally sprinkle "synchronized" word throughout our code and pray to God that we don't deadlock. However, in large scale development, one needs to know precisely why something is being synchronized and should seek opportunities NOT to synchronize to increase throughput and not cause bottlenecks or worse deadlocks.
1. I am going to remove synchronization from these methods: create, delete, read, find, update.
2. I will keep the ReadFixedString and WriteFixedString methods of the DataHelper class synchronized.
Regards.
Bharat
[ September 29, 2003: Message edited by: Bharat Ruparel ]

[Andrew Monkhouse]

Hi Bill,
What part would you like me to expand on?
Regards, Andrew

[John Canavan]

Hello there,
I have purposely stayed away from this issue and moved on to the GUI side of things as I am more comfortable with it - however the approach of burying my head in the sand did not solve any of the problems I have pertaining to threading!!!
Apologies, but I am still having a little difficulty with this area and would appreciate any comments etc. It seems, from above, that the recommendation is not to synchronize the public methods. So, for example, I have the following update method:

<font size="2">
public void update(int recNo, String [] data) throws
                     RecordNotFoundException {
    if (inValidRecord(recNo))
        throw new RecordNotFoundException("Not a valid record!");
    long position = getRecordStartPos(recNo);
    try {
        randomAccessFile.seek(position);
        for (int iPos = 0; iPos < fieldLengths.length; iPos++) {
            // write the string to the database but only if it is not part
            // of the key (i.e. name or location)
            if ((iPos != NAME_FIELD) && (iPos != LOCATION_FIELD))
                DataHelper.writeFixedString(randomAccessFile,
                            fieldLengths[iPos], data[iPos]);
            else //just move the pointer on
                randomAccessFile.skipBytes(fieldLengths[iPos]);
        }
    } catch (IOException ioe) {
        throw new RecordNotFoundException("Database write error");
    }
}

with the following method in DataHelper…………:

  
public static synchronized void writeFixedString(RandomAccessFile raf,
        short size, String str) throws IOException {
    byte [] byteArray = new byte[size];
    for( int i = 0; i < size; i++) {
        if (i < str.length()) {
            byteArray[i] = (byte) str.charAt(i);
        } else {
            byteArray[i] = (byte) ' ';
        }
    }
    String tempStr = new String(byteArray, "US-ASCII");
    raf.writeBytes(str);
}
</font>

My concerns are demonstrated in the following example:
On the server….
Thread A serving client 1 begins to update record 10. iPos in the loop “for (int iPos = 0; iPos < fieldLengths.length; iPos++)” has just been incremented to 2 therefore it is in the middle of a writing a full record i.e. two fields in the record have been written and it is about to write the third field. Thread B serving client 5 slices in and begins updating record 5, it updates a few fields in the record and Thread A starts again with iPos == 2, however the pointer in the randomAccessFile has been moved (when Thread B was updating record 5) therefore, thread A is actually writing to the wrong location in the file/database! Is this not correct?
(NOTE: I intend using RMI with one instance of Data.java per client. Each Data object has a handle on the same randomAccessFile which was created in the Singleton DataSchema class. The locking of the records is handled by using a static WeakHashMap in Data.java)
Should I not therefore make these public methods synchronized so they are atomic (or at least wrap the relevant part of each of the public methods in a synchronized block)? Therefore there would be no need to have the writeFixedString synchronized as it would be called from a synchronized context? Are these two comments correct? Maybe my design is wrong….

Still confused.….
John
[ October 02, 2003: Message edited by: John Canavan ]

[Andrew Monkhouse]

Hi John,
I think part of the confusion is because we are often talking about two different ways of writing the Data class dependant on the instructions from Sun:

Some people have received an interface (for either hotel or contractors) with a lock() method that returns a cookie.
For these instructions, many people are planing on having only a single instance of the Data class (either via a Singleton or a Multiton).
In this case, it makes sense to synchronize the methods within Data as this will alleviate the concerns you have

Some people have received an interface (for either hotel or contractors) with a lock() method that does not return any identifier (return type = void).
For these instructions, many people are planing on having only one instance of Data class per connected client.
In this case, it makes no sense to synchronize the methods within Data.
However some common options from there are:
* Have a single instance of a helper class which does the disk operations, in which case it's methods could be synchronized, alleviating your concerns.
* Have a single instance of the file I/O class (RAF, or NIO, or ...), and have a synchronized block based on that single instance, in which the seek and read or update methods are treated as an atomic operation.
*Have a synchronized block based on some other common object (e.g. the object holding the locks), in which the seek and read or update methods are treated as an atomic operation.

Without synchronization somewhere you are going to get into the problem that you mentioned.
Hopefully I have given you some ideas of how you might solve this.
I have deliberately given lots of options without going into the good or bad points on any of them, so you can have a thing about them.
Regards, Andrew
[ October 03, 2003: Message edited by: Andrew Monkhouse ]

[Ulrich Heeger]

Hi,
Concerning the thread safety for one Data-instance per client there is also an interesting threadthread.
Hi Andrew,
I'm now a little bit confused because my thread safety is based on having one Data-Instance per client through Connection Factory, but I have this signature of lock() which returns a cookie.
But you wrote:

Some people have received an interface (for either hotel or contractors) with a lock() method that returns a cookie.
For these instructions, many people are planing on having only a single instance of the Data class (either via a Singleton or a Multiton).

Would you recommend me to change my design?
I'm afraid to change my concept accordingly to the slogan:
"the devil you know is better than the devil you don't know"
But of course if you guys are suggesting this, I would have to buy some Fosters and after waking up with big headaches redesign my thread safety concepts
Regards
Ulrich
[ October 03, 2003: Message edited by: Ulrich Heeger ]

[John Canavan]

Hello Andrew,

Some people have received an interface (for either hotel or contractors) with a lock() method that does not return any identifier (return type = void).
For these instructions, many people are planing on having only one instance of Data class per connected client.

I would fall into this category.

In this case, it makes no sense to synchronize the methods within Data.

I guess I did not fully understand the use of the synchronized keyword, but it makes sense now what you are saying as I see from Max’s book (page 111) he states:

By wrapping your method in a synchronized block, you are effectively treating the entire method as a single atomic operation, as far as other threads’ clients of that object are concerned.

Together with this along with your comments and Bharat’s above (….we know that "different" clients will not clash with each other….) I understand now.
What I will do is the following:
1. I will synchronize on the WeakHashMap whenever I want to do a read or a write (I have seen others doing this but was not fully sure why they were doing it but now I know). So the update method will now look like the following:

  
public void update(int recNo, String [] data) throws
                     RecordNotFoundException {
    if (inValidRecord(recNo))
        throw new RecordNotFoundException("Not a valid record!");
    long position = getRecordStartPos(recNo);
    synchronized (lockedRecords) {
        try {
             //same code as above
        } catch (IOException ioe) {
        throw new RecordNotFoundException("Database write error");
        }
    }
}

I realise this may not be the most efficient way of doing things but I would like to keep it as simple as possible as there are plenty of places to get “bogged down” in yet. It should be okay, shouldn’t it? So basically before a read or a write is done you must own the lock on this object (lockedRecords) and this in turn will make any code in the synchronized block is atomic, therefore the problem I discussed in my previous thread would never occur as a slice out in the middle of the for loop cannot take place. Is this correct?
2. Also, there would be no need to have the writeFixedString and readFixedString methods synchronized if they are called from a synchronized context. Is this correct?
Thanks very much for your help Andrew,
John

[Bharat Ruparel]

Hello John,
I have been busy and it is time for Andrew to take charge. I am also watching this thread carefully and am mulling things over as Andrew explains further. I seem to have reached my current limits here.
Regards.
Bharat
Hello Andrew,
We are awaiting your response, please give some exampes with code snippets.
Regards.
Bharat

[Andrew Monkhouse]

Hi Ulrich,

Would you recommend me to change my design?

Not at all.
As I said, many people are having single instances of Data class. But "many" != "all". I also deliberatly avoided any comments about what I thought was good / bad about any of those concepts I mentioned. I was simply trying to point out where some of the confusion might have occured and some suggestions for someone who did not have a cookie.
There are many good reasons for having multiple instances of the Data class no matter whether you have a cookie or not.

I would have to buy some Fosters and after waking up with big headaches redesign my thread safety concepts

Bleagh. Well if you are going to drink that Fosters crap then it is no wonder you will wake up with a big headache. There's a reason it is so plentiful in Europe - we don't want it in Australia, so we export it every chance we can. With so many excellent wheat beers in Gemany and so many other good beers from Holland and Belgium I don't know why you would even look twice at Fosters. I certainly didn't have a single Fosters while I was in Europe. Now if you ever get a chance to try some Cascade (from Tasmania) you will know what a really good Australian beer is like. And I have never heard of anyone getting a headache after drinking it - it is made from very pure water and pure ingredients.
In another thread you commented:

Andrew, you australians are really amazing, I think that's really popular in Down Under to travel around the world, isn't it? But you have so good beers in Australia too

Yes, many Australians try to travel around Europe while they are young, typically just after high school and before starting University.
I was lucky to join a big multi national company with an office in Sydney which they closed just after I joined it. Fortunately I had convinced them that I was half decent, so they shipped me off to Holland for 2 years and London for a further year.
Regards, Andrew

[Andrew Monkhouse]

Hi John,
I think I have confused you, and for that I appologise.
Your earlier code had static synchronized methods in DataHelper. Therefore these methods would have synchronized on DataHelper.class - of which only one instance can ever occur, so you already worked with the suggestion I had made: "Have a single instance of a helper class which does the disk operations, in which case it's methods could be synchronized, alleviating your concerns".
So you should have already been fine. Your writeFixedString() should already have been treated as an atomic operation.

I will synchronize on the WeakHashMap whenever I want to do a read or a write ... It should be okay, shouldn?t it?

Looks pretty good to me.
My only (slight) issue is that the object you are synchronizing on (the WeakHashMap) is not really the right one to use. I know I did suggest it earlier, but I really shouldn't have. It will work, but it probably also blocks other threads from obtaining locks while you are doing reads / writes which is an undesirable side effect. Admittedly a small side effect, but you need to be aware of it and decide whether you really want to do this.

Also, there would be no need to have the writeFixedString and readFixedString methods synchronized if they are called from a synchronized context. Is this correct?

Correct.
Regards, Andrew

[Andrew Monkhouse]

Hi Bharat,

I have been busy and it is time for Andrew to take charge.

I have been busy on SCBCD, but completed that on Friday. It is good to see that I am not really needed most of the time.
I don't know about taking charge, but I will at least add a few comments for people to think about.

I seem to have reached my current limits here.

I doubt that.

We are awaiting your response, please give some exampes with code snippets.

What sort of code snippets are you after?
Regards, Andrew

[John Canavan]

Hi Andrew,
Thanks for the feedback . Hope the exam went well!
I just have a few comments/questions on a couple of your posts in this thread.

Your earlier code had static synchronized methods in DataHelper. Therefore these methods would have synchronized on DataHelper.class - of which only one instance can ever occur, so you already worked with the suggestion I had made: "Have a single instance of a helper class which does the disk operations, in which case it's methods could be synchronized, alleviating your concerns".

I do have a class DataHelper which had the following two methods.

    public static synchronized String readFixedString(RandomAccessFile raf,
            int size) throws IOException 
    public static synchronized void writeFixedString(RandomAccessFile raf,
            short size, String str) throws IOException {

You also stated:

So you should have already been fine. Your writeFixedString() should already have been treated as an atomic operation.

This is simply a class with two static methods and I call whenever I need to do a read or write be it in Data.java or DataSchema.java (which reads and stores the magicCookie, schema information etc). There is never an instance of this object created, therefore (I think) it would be incorrect to state that “only one instance may occur” (sorry, I did not make this clear). This method infact writes and reads fields, not whole records at a time, for example if I am writing out a complete record 6 calls will be made to this (see my update method in post made on 02/10/03 above). Synchronizing the methods, while it does lock DataHelper.class and ensures that the methods are synchronized, would not have solved my problem (I think!) in the middle of such a write/read e.g. if two fields were written, another thread (Thread B) could have gained the lock for DataHelper.class when the loop in the current thread (Thread A) was incrementing iPos (see my update method) and when this thread (Thread A) gained the lock back the randomAccessFile pointer could/would be positioned at the wrong place when writing the third field of the record. This is correct, isn’t it?
The scond method you advocated was:

Have a single instance of the file I/O class (RAF, or NIO, or ...), and have a synchronized block based on that single instance, in which the seek and read or update methods are treated as an atomic operation.

Thinking about this one, I do have a single instance of randomAccessFile object which is created in the singleton DataSchema file. A handle on this file is passed to all the Data objects that are created. This would, in my opinion, be a more logical object (i.e. the randomAccessFile object) to synchronize on when doing the reading and writing, rather than the lockedRecords object. I am not sure if this method is any better (efficient etc) than synchronizing on the lockedRecords object, maybe you could give me your opinion on it.
If I choose this mechanism it would in one way solve the problem you mentioned above:

it probably also blocks other threads from obtaining locks while you are doing reads / writes which is an undesirable side effect

…and it a way it wouldn’t as the reason you want to lock a record is because you wish to do a read or a write, therefore you have to wait until the randomAccessFile is nolonger locked anyways.
If the method of locking the randomAccessFile sounds ok, I may go with this as, stated above, it sounds more logical to lock on the file rather than the lockedRecords object (even though it may not make a lot of difference).
Thanks again Andrew,
John

[Ulrich Heeger]

Hi Andrew,

Bleagh. Well if you are going to drink that Fosters crap then it is no wonder you will wake up with a big headache. There's a reason it is so plentiful in Europe - we don't want it in Australia, so we export it every chance we can. With so many excellent wheat beers in Gemany and so many other good beers from Holland and Belgium I don't know why you would even look twice at Fosters. I certainly didn't have a single Fosters while I was in Europe. Now if you ever get a chance to try some Cascade (from Tasmania) you will know what a really good Australian beer is like. And I have never heard of anyone getting a headache after drinking it - it is made from very pure water and pure ingredients.

Ooh, good to know, I will try to get some Cascade, I'm now really curious
Thanks a lot for your help
Ulrich

[Andrew Monkhouse]

Hi John,

Hope the exam went well!

Thanks. It was OK
You are right - since your static synchronized methods did not contain the seek() themselves, then you could have ended up writing or reading in the wrong location.

I do have a single instance of randomAccessFile object which is created in the singleton DataSchema file. A handle on this file is passed to all the Data objects that are created. This would, in my opinion, be a more logical object (i.e. the randomAccessFile object) to synchronize on when doing the reading and writing, rather than the lockedRecords object.

Yep - this does sound more logical.

I am not sure if this method is any better (efficient etc) than synchronizing on the lockedRecords object, maybe you could give me your opinion on it.

It will be more performant, simply because locks can be granted without affecting reading / writing, and reading / writing can be done without affecting the locking. However the performance increase for our use will be so small that performance alone does not justify it.
But the more logical usage does justify it (especially for junior programmers reading your code).
And the fact that other operations within locking or within database read/writes can now be added with side affecting each other also justifies this.

the reason you want to lock a record is because you wish to do a read or a write, therefore you have to wait until the randomAccessFile is nolonger locked anyways.

You only need to lock the record for writing not for reading.
Consider if we had separate CPUs handling the read operations and the locking operations. In that case, having separate objects that the locking and the disk operations rely on makes more sense:

    common mutex                        separate mutexes
    ---------------------               ------------------------
    lock(3)                             lock(3)    read(7)
    read(7)                                        read(3)
    read(3)                                        update(3)
    update(3)                           unlock(3)  read(4)
    unlock(3)
    read(4)

As you can see (hopefully) having separate mutex objexts allowed for unrelated methods to operate simultaneously. Having a single mutex made them all operate serially, so it would take longer.
This is a bit theoretical, since unless you have an OS that will allow a single process to use multiple CPUs and a JVM that knows how to use the multiple CPUS then the scenario is unlikely to eventuate. But if you were running on 64 bit Solaris then it could make a difference.

If the method of locking the randomAccessFile sounds ok, I may go with this as, stated above, it sounds more logical to lock on the file rather than the lockedRecords object (even though it may not make a lot of difference).

Regards, Andrew

[Andrew Monkhouse]

Hi Ulrich,

Ooh, good to know, I will try to get some Cascade, I'm now really curious

If we ever do end up meeting up, I will have to bring some Aussie beers for you, Vlad, and Phil (and maybe Jim) to try.
Regards, Andrew

[Bharat Ruparel]

Hello Andrew/John/Ulrich,
Thanks for keeping this nice discussion rolling. I didn't have much to add about the time I wrote my last post, hence my comments. Based on the discussion that has occured after my last post, I think that I do have the answer, but more likely, am wrong. I would love to have you all validate my thought process. I am talking about the design in which there is a unique instance of the Data object per client:
1. You do need to synchronize on the static data structure (HashMap, WeakHashMap, HashSet) used for logical record locking. This is a good idea since in a method such as booking a room, you do want to maintain lock-read-update-unlock sequence.
2. If we have a singleton instance of the DataScema class that creates the RAF, passes its handle to the requesting Data instances, and synchronize on the ReadFixedString and WriteFixedString methods of the DataHelper classes which are also static, we should be OK.
The reasons I think we are OK are:
1. Logical record locking ensures that the clients are working on one record of the RAF file at a time.
2. The record storage scheme being used for the assignment actually helps out here, since:
2.1 The record numbers do not change. Deleted records maintain their position and are reused.
2.2 These are fixed length records.
3. Dirty reads are OK. Therefore only logically record lock before reading a record?
It is not a good idea to synchronize on the RAF since that needlessly throttles the I/O.
I think I don't need the code snippets for the time being since this might clear up the doubts.
Regards.
Bharat

[Ulrich Heeger]

Hi Bharat,
you wrote:

If we have a singleton instance of the DataScema class that creates the RAF, passes its handle to the requesting Data instances, and synchronize on the ReadFixedString and WriteFixedString methods of the DataHelper classes which are also static, we should be OK.

I think you should be right.
So you write the Strings individually?
Because my write method looks like:

private void writeRecord(long recNo, String[] data) throws IOException{
synchronized(reservedRecords)
byte[] buffer;
int numFields = dataSchema.getNumOfFieldsinRec();
 byte[] fieldLength = dataSchema.getFieldLength();
 byte[][] bytesInFields = new byte[fieldLength.length][];

for(int j = 0; j < numFields; j++){
bytesInFields[j] = new byte[fieldLength[j]];
buffer = data[j].getBytes();
for(int i = 0; i < buffer.length; i++){
bytesInFields[j][i] = buffer[i];
}

 }
 //increasing the getRecposition with 1 to 
                 //skip the delete-flag
 raf.seek(dataSchema.getRecPosition(recNo)+1);
 for(int i = 0; i< fieldLength.length; i++){
  raf.write(bytesInFields[i]);
 } 
 } 
 
}

I'm thinking to write all the Strings synchronly, because I think to get all the values of the fields back from the client, so these fields which haven't changed will be rewritten with the original value. But I have to admit, that your approach is very interesting.
So you will check the position of the concerning field and call WriteFixedString?
How do you get the right position of the field, have you hardcoded it in DataSchema-Class?
I use a mutex, but your way to ensure threadsafety by synchronizing static methods of DataHelper is similar. Perhaps I would declare RAF a static member in DataSchema, that's namely tautological, but like the slogan says: Double is better

3. Dirty reads are OK. Therefore only logically record lock before reading a record?

Yes, I think your approach should go this way. Understanding you correctly it can be that for example Thread A is Reading String[2] of Record 1. Before reading String[3], Thread B will write String[3]. So I think to lock the Record should be the right approach. My reading method is also using a mutex and doing the read synchronly:

public String[] readRecord(long recNo) throws IOException, 
                                     RecordNotFoundException{
                                                
int numFields = dataSchema.getNumOfFieldsinRec();
byte[] fieldLength = dataSchema.getFieldLength();
byte[][] bytesInFields = new byte[fieldLength.length][];
String[] fields = new String[numFields];
    
validate(recNo);
     
synchronized(reservedRecords){
    raf.seek(dataSchema.getRecPosition(recNo));
    
    if(raf.readByte() == deleted)
         throw new RecordNotFoundException
                          ("the Record has been deleted");
              
    for(int j=0 ; j < numFields; j++){
     
         bytesInFields[j] = new byte[fieldLength[j]];
                 raf.read(bytesInFields[j],0, bytesInFields[j].length);
                 fields[j] = new String(bytesInFields[j]);
    }
   
       
}
      
    return fields;     
}

Regards
Ulrich
[ October 04, 2003: Message edited by: Ulrich Heeger ]

[Bharat Ruparel]

Hello Ulrich,
I wish I could withdraw my earlier post. I think I may have caused more confusion for all of us than warranted. Let me see if I can undo the damage (or cause more of it unwittingly).
1. I standby my comments on logical record locking made in the post above.
Note that as Andrew has pointed out in one of the earlier threads, a logical record is merely a logical structure that informs the clients (or threads) that a particular record is being worked on and should be considered off-limits until further notification.
Additionally,
2. We need to ensure that the physical reads and writes are also done at a record level atomic unit since another thread slicing in and moving the "single(ton)" RAF file pointer will cause problems to say the least. A clean way to do so will be to:
Synchronize the methods that are being used to read and write records:
2.1 In my program, I have a private writeRecordToFile method that is used by both update and create public methods. I will synchronize this method. Granted that we really want to synchronize on the RAF to prevent the singleton file pointer from being moved in the middle of a physical record write, since this method does precisely that, it is OK to synchronize this at the method level. Note that this method is calling WriteFixedString method "n" number of times where "n" is equal to the number of fields in the record. Since WriteFixed
String is being called from within a synchronized context, i.e., the writeRecordToFile method, it doesn't need to be synchronized further. Therefore, remove the synchronization from the WriteFixedString method in the DataHelper class.
Similary,
2.2 We all have a public read method defined for readling A record off the file. This method uses the services of the DataHelper.readFixedString method. Synchronize this method as well and remove synchronization from the readFixedString method in DataHelper class.
Therefore, this is a long-winded way of saying:
1. Synchronize the private method that you use to write one record to the file, preferably this method should be used by both create and udpate pubilic methods of the Data class. And, synchronize the public read method of the Data class.
2. Remove the synchronization from the readFixedString and writeFixedString methods of the DataHelper class.
The key to understanding why this synchronization of the writeRecordToFile private method and read public method is necessary even though we are using a unique instance of the Data object per client is that we are really trying to single-thread the underlying write and read operations using the singleton RAF file-pointer. Another thing to keep in mind is that only one synchronized method can execute for the Data instance at a time and no other synchronized method can execute until the currently executing synchronized method is finished execution, i.e., either writeRecordToFile or the read operation will happen but they cannot slice into each other's way for a given Data instance. Therefore, they will not mis-position the singleton file-pointer when it is being used to either write or read a record.
This is my current state of understanding.
Regards.
Bharat

[John Canavan]

Hello Andrew,
The example you gave was particularly helpful and I can see the benefit of using the two mutexes rather than the one (The phrase mutex is relatively new to me so let me know if the previous sentence is not properly phrased).
I would just like to verify a few things with you in relation to my code based on our discussion here – hopefully you will just have to reply “Yes” to all of these!!!

1. Just to double check, when you said the following:

<font size="2"><font size="2">  
    public String [] read(int recNo) throws RecordNotFoundException {
        if (inValidRecord(recNo))
            throw new RecordNotFoundException("Not a valid record!");
        long position = getRecordStartPos(recNo);
        String [] record = new String[fieldLengths.length];
        try {
            synchronized(randomAccessFile) {
                randomAccessFile.seek(position);
                for (int iPos = 0; iPos < fieldLengths.length; iPos++)
                    record[iPos] = DataHelper.readFixedString(
                                randomAccessFile, fieldLengths[iPos]);
            }
        } catch (IOException ioe) {
            throw new RecordNotFoundException("Database read error");
        }
        return record;
    }
</font></font>

2. In fact anywhere I do a randomAccessFile.seek I would need to synchronize the block following it – e.g. my private inValidRecord method does a seek() followed by a readByte(), so this would need such a synchronized block as well?

<font size="2"><font size="2">
    private boolean inValidRecord(int recNo){
       //some code initialising variables…..
        try {
            //some more code……. 
           
            synchronized(randomAccessFile) {
                randomAccessFile.seek(position);
                byte deletedFlag = randomAccessFile.readByte();
            }
            // some more code…….     
        } catch (IOException ioe) {
            //handle exception            
        }
       //return result
    }
</font></font>

3. The isValidRecord is also called by the three functions which are involved in record locking (lock, unlock and isLocked). I presume also there is no problem here i.e. two synchronizations take place when lock, for example, is called – one in inValidRecord and one further down, they are not nested however. Is this ok?

<font size="2"><font size="2">
public void lock(int recNo) throws RecordNotFoundException  {
        if (inValidRecord(recNo))
            throw new RecordNotFoundException("Not a valid record!");
        //some code….
        synchronized(lockedRecords) {
            //some code to lock the record etc
        }
}
</font></font>

4. In the find method I do a seek() on the first record in the file and iterate through each record in the database. As I iterate through each record, I read it and check to see if it matches the criteria, add it into the match list and continue doing this until the end of file. When I say I read the record I mean I call my public read() method which returns a record in the form of a string array. So each time I do a read(), if point 1 above is correct, the read method synchronizes on the randonAccessFile, however I have already synchronized on it in the find() method. This I suppose is sort of like a nested lock, but they are locking on the same object, so it should be all right. (Basically find() locks on the randomAccessFile once and within this block read() is called many times and each read() also tries to synchronize on the randomAccessFile). Is this mechanism is okay?
I better stop asking questions in case I’ve gone wrong in point 1…if I have then all the others are wrong too!
Regards and thanks,
John
[ October 04, 2003: Message edited by: John Canavan ]
[ October 05, 2003: Message edited by: John Canavan ]

[Bharat Ruparel]

Hello Andrew/John/Ulrich,
Sometimes it is better not to post! I am going through one of those "brain-short-circuit" phases. The scenario that I described in my last post applies to a single instance of the Data object but not an instance of the Data object for each client design that I and John (and Ulrich?) are persuing. So Ulrich, if you do decide to use a single instance since you have cookies in your DBAccess method signatures, then what I have posted above should be applicable for you.
John, you were astute enough to ignore my posts and focus on Andrew's. I think I have finally come home from around the block only after having gone around the world in the process of doing it. Your code looks fine to me with the following caveat:
You need to follow the same synchronization logic for the "write" operations as well, i.e., synchronize on the randomAccessFile for the create/update method same as you are doing for the write methods. And you should remove the synchronization from your readFixedString/writeFixedString methods.
Regards and Thanks.
Bharat.
Hello Andrew,
We are eagerly awaiting your response. Besides that, would you please define the word "mutex" for me? I think of mutants as in the X-men movie and that has a rather negative effect in my efforts to understand what you are explaining above.
[ October 04, 2003: Message edited by: Bharat Ruparel ]

[John Canavan]

Hi Bharat,

John, you were astute enough to ignore my posts and focus on Andrew's.

I assure you I read all your posts and in fact I had written a reply to the one you posted on October 04, 2003 08:33 AM, but I noticed you posted another one soon afterwards which I didn’t fully understand so I decided not to bring further confusion to the matter!!!

I think I have finally come home from around the block only after having gone around the world in the process of doing it.

No worries, from reading various threads you have posted in it seems you have most of it completed now - at least you can see a bit of light at the end of the tunnel! I have done most of the data access layer and some of the GUI but there is much work (and learning) to be done yet. I am actually starting a full-time Masters (M. Sc. In Networks and Distributed Systems) in Trinity College Dublin tomorrow which lasts 12 months so I reckon it will be a while before I complete this assignment, but I’ll keep chipping away at it in the meantime. I have been on ‘holidays’ at home here in the West of Ireland for the last two weeks and only made a start on it then but unfortunately I do not have the same Java expertise as most of you guys!

You need to follow the same synchronization logic for the "write" operations as well, i.e., synchronize on the randomAccessFile for the create/update method same as you are doing for the write methods.

Yes I had planned to do that!

And you should remove the synchronization from your readFixedString / writeFixedString methods.

Yes I had planned to do that as well!

Hello Andrew,
We are eagerly awaiting your response.

Yes hopefully Andrew can give us some feedback.
Thanks for the suggestions Bharat,
John

[Andrew Monkhouse]

Hi everyone,

Ulrich: my write method looks like:

private void writeRecord(long recNo, String[] data) throws IOException{
    synchronized(reservedRecords)
        byte[] buffer;
        int numFields = dataSchema.getNumOfFieldsinRec();
        byte[] fieldLength = dataSchema.getFieldLength();
        byte[][] bytesInFields = new byte[fieldLength.length][];
        for(int j = 0; j < numFields; j++){
            bytesInFields[j] = new byte[fieldLength[j]];
            buffer = data[j].getBytes();
            for(int i = 0; i < buffer.length; i++){
                bytesInFields[j][i] = buffer[i];
            }
        }
        //increasing the getRecposition with 1 to 
        //skip the delete-flag
        raf.seek(dataSchema.getRecPosition(recNo)+1);
        for(int i = 0; i< fieldLength.length; i++){
            raf.write(bytesInFields[i]);
        } 
    } 
}

Just some more things for you to think about:

Most of that code does not need to be in a synchronized block.

If you create one byte array large enough to contain the entire record, you will only need to call the write() method once

You are converting from String into an array of bytes using the platform's default charset. Do you think this meets the US-ASCII / 7 bit requirement?

Why skip the deleted flag? If you assume that any write is only going to occur for valid records, then you can always set the deleted flag to "undeleted" (or whatever you call it). Then your writeRecord() method can be used for adding a record as well.

John: Just to double check, when you said the following:

What did I say, and what is your question?
By the way, there is a known bug in UBB that may have caused your question to be lost. If you edit a post which has both [quote] and [code] tags, UBB sometimes looses the portion of the post between the quoted text and the first code block (I think that's how the bug works). That is why some people like Jim don't ever use [quote] blocks.
(There is also another issue where editing a post with [code] blocks will result in <font size="2"> tags being added inside the code block, and you have to manually remove them.)

John: 2. In fact anywhere I do a randomAccessFile.seek I would need to synchronize the block following it ? e.g. my private inValidRecord method does a seek() followed by a readByte(), so this would need such a synchronized block as well?

Correct.

John: 3. The isValidRecord is also called by the three functions which are involved in record locking (lock, unlock and isLocked). I presume also there is no problem here i.e. two synchronizations take place when lock, for example, is called ? one in inValidRecord and one further down, they are not nested however. Is this ok?

No problems.

John: 4. In the find method I do a .... Is this mechanism is okay?

Should be OK.
Why are you synchronising the find method though? What does it gain you? Or, more importantly, what does it cost you?

Bharat: would you please define the word "mutex" for me? I think of mutants as in the X-men movie and that has a rather negative effect in my efforts to understand what you are explaining above.

Mutex is a shortening of "Mutual exclusion" - a mutual (common) device which allows one client access to something, while excluding all others. So the object you synchronize on becomes a mutex.

Bharat: We are eagerly awaiting your response.
John: Yes hopefully Andrew can give us some feedback.

I don't know why you keep asking me for comments - you guys are doing fine without me.
Regards, Andrew

[John Canavan]

Hello Andrew,

What did I say, and what is your question?

Oops – looks like some of my post did go missing – yes I did edit the post a couple of times as you can see - but here was the first point again (I won’t bother posting the code as it has not changed).
1. Just to double check, when you said the following:

You only need to lock the record for writing not for reading.

Yes, anytime I read a record I do not need to lock it (i.e. do not need to add the record number into the static WeakHashMap), but within the read() method of Data.java I need to wrap part of it (see code posted October 04, 2003 03:11 PM) in a synchronized block to make sure the pointer does not get moved to the incorrect location in the middle of reading a record. I presume what I said here is correct and that this code is all right, is it?
2.

Why are you synchronising the find method though? What does it gain you? Or, more importantly, what does it cost you?

Yes, you’re correct I should remove this synchronization from the find() method. Anyway, there was no need for me to do the seek() in the find() method as the read() method, on receiving the record number to read, does it’s own seek() to the correct record location! Hmmm…..although saying that, if I did synchronize on the randomAccessFile in the find() method I would make sure that there were no more records added to the database and that I had checked every physical record in the database (I loop all the records by calculating the number of records in the database prior to the loop). What advice would you give here? I’m not really too worried about this, and I do think it would be acceptable not to synchronise the find() anyway, I would be slightly more concerned about point 3 below.
3. Regardless of synchronizing or not in the find() method scenarios like the one below may arise.
EXAMPLE
---------------
The find() method loops through the 100 records and it finds that only record 10 and 75 match the criteria specified (e.g. user looking for records with the skill of "painting") , therefore it returns an array of size 2 with “10” and “75” as the elements – all fine so far. The read() function is called twice when the results are to be printed to the screen. In between the find() method running and before the two records were printed to the screen both records had the skill field changed from “painting” to “roofing”. So the user searched on the “painting” skill but two records with the skill “roofing” are now on the screen! Is this acceptable? I can think of ways to work around this such as storing lists of matching records in Data.java and getting some type of public access to the list but would this be necessary?
Thanks again,
John
[ October 05, 2003: Message edited by: John Canavan ]

[Ulrich Heeger]

Hi Andrew,
Thank you for pointing me to problems I haven't even noticed

You are converting from String into an array of bytes using the platform's default charset. Do you think this meets the US-ASCII / 7 bit requirement?

Oops, good to know. In my assignment it's written:

The character encoding is 8-bit US ASCII

But I think, that's the same, isn'it?
So the right call would be:

byte[] buffer = string.getBytes("US-ASCII"); ?

Why skip the deleted flag? If you assume that any write is only going to occur for valid records, then you can always set the deleted flag to "undeleted" (or whatever you call it).
Then your writeRecord() method can be used for adding a record as well.

Good point. I haven't allready thought at the createRecord-method. I thought at an overloaded write-method for the delete-flag, but I will integrate this functionality within the write-method.
Due to my re-reading of this thread I will change my reading and writing concept, which I will impertinently post in my following posting.
Greetings
Ulrich

[Bharat Ruparel]

Hello John,
Best of luck with your master's program. You will do just fine. Your post about synchronization of the RandomAcessFile operation was very helpful to me. Thanks.
Hello Andrew,
As you might have noticed, I do go through those "brain-short-circuit" phases occcasionally, your words have a wonderful way of getting me out of that state of mind.
Hello Ulrich,
I should be online for the next two hours, so post away if you feel like running something by me.
Gentlemen, I will be happy to buy all of you a beer if you are in the Boston, Massachusetts area. My recommendation? Sam Adams and Coors Light. I picked up a taste for Sam Adams in Massachusetts and Coors Light in Texas (I did my college work in Lubbock,Texas).
Regards.
Bharat

[Ulrich Heeger]

Hi Bharat,
thanks for your invitation
Coors light I remember, when I was in the States. After HighSchool I was half a year in the States, travelling across the continent as a tramper. We hadn't much money so we bought mostly this "Old Milwaukee" which made laugh each american, is that right.
But Coors light, I remember that I like it a lot.
My posting is coming soon
Greetings
Ulrich

[Ulrich Heeger]

Hi Bharat, John, Andrew,
after rereading this thread I have decided to change my concept of reading and writing.
I will displace the reading and writing from/to disk to a new Class DataHelper, like you guys have allready implemented.
I find this way more elegant and anyway my Data-Class is getting so big and I'm glad to remove some code
Further I won't synchronize anymore on reservedRecords but, like Andrew suggested, on a second mutex, either static raf (like John) or the static member dataHelper within Data.
To annoy you with my code ;, I will post here my redesign of the read-method:

public String[] readRecord(long recNo) throws IOException, 
                            RecordNotFoundException{                            
int numFields = dataSchema.getNumOfFieldsinRec();
byte[] fieldLength = dataSchema.getFieldLength();
String[] fields = new String[numFields];

validate(recNo);  
synchronized(mutex2){
     raf.seek(dataSchema.getRecPosition(recNo));
     if(dataHelper.readFlag(raf) == deleted)
            throw new RecordNotFoundException
                              ("the Record has been deleted");
      for(int j=0 ; j < numFields; j++){
   fields[j] = dataHelper.
                               readFixedString(raf, fieldLength[j]);
   }   
}
return fields;     
 }

and within the DataHelper-Class:

protected static String readFixedString(RandomAccessFile raf, byte size) 
          throws IOException,UnsupportedEncodingException{
        byte[] buffer = new byte[size];
raf.read(buffer);
return new String(buffer, "US-ASCII");

}
protected static byte readFlag(RandomAccessFile raf) 
                                          throws IOException{
 return raf.readByte();
}

Here are my questions:
1. General
Is this similar to the way you have implemented your read-method?
2.Concerning the readRecord-method in Data-Class:
a:How have you guys implemented the validate(recNo)-method. I mean have you integrated the control of existence and deletion in validate() or split like I have done it above?
And if you have both control mechanisms in validate() how have you implemented the deletion control? (Reading of delete-flag at the disk or Caching the deleted Records?)
b:Jim, your code above, the inValidRecord(recNo)-method shouldn't be included in the synchronized block for reasons of integrity of the Record?
Regards & thanks alot in advance
Ulrich
[ October 05, 2003: Message edited by: Ulrich Heeger ]
[ October 05, 2003: Message edited by: Ulrich Heeger ]
[ October 05, 2003: Message edited by: Ulrich Heeger ]

[John Canavan]

Hello Bharat and Ulrich,
Bharat,

Best of luck with your master's program.

Thanks, I’ll need plenty of luck over the next year!
Ulrich,

Is this similar to the way you have implemented your read-method?

Take a look at my read method above and it hasn’t changed.

a:How have you guys implemented the validate(recNo)-method. I mean have you integrated the control of existence and deletion in validate() or split like I have done it above?
And if you have both control mechanisms in validate() how have you implemented the deletion control? (Reading of delete-flag at the disk or Caching the deleted Records?)

Again you can see that I call this first thing in the read() method. I also call it first thing in the update(), delete(), lock(), unlock() and isLocked() methods. I do not do any caching. The actual method itself does the following two things:
1) Checks to see if the file/database is actually big enough to have a record at this position e.g. if there are only 5 records in the file/database and you pass in record number 100 then it returns false straight away (a couple of quick calculations will work this out e.g. get length of data section in file and divide it by the length of a record).
2) Next, if the first step does not fail I check that there is not a deleted flag associated with the record. Here I do a seek() for the record and then I read the byte - these two lines are in a synchronized block as I do not want a thread to slice in and move the randomAccessFile pointer between the seek() and the reading of the byte – see point 2 in the post I made on the October 04, 2003 03:11 PM above and you will see that these two lines are synchronized.

Jim, your code above, the inValidRecord(recNo)-method shouldn't be included in the synchronized block for reasons of integrity of the Record?

Here I do not wrap the inValidRecord() record inside the synchronized block of the read(), update() etc methods – I suppose you could do it. The reason I don’t is that I want to wrap as little of code as possible inside the synchronized block. Maybe Andrew or Bharat or indeed your good self will let me know if this is a good principle or not! The only part of the inValidRecord() method that really needs to be synchronized are the two lines mentioned above i.e. the seek() and readByte() – mind you if you look at the last post I made we are getting into the same problems again e.g. Thread A checks to see if record 1 is invalid, it is not, before it does the read it slices out and Thread B comes in and deletes record 1, Thread A slices in and continues to read the fields of record 1 believing it to be a valid record – I suppose this is another example of a ‘dirty read’ which are probably acceptable. Maybe you guys can let me know your opinions here – have a look at the previous post I made.
Hope that helps,
John

[Bharat Ruparel]

Hello Ulrich & John,
I just got through going through your post. Ulrich, it seems to me that you have gotten two cocepts related to locking mixed up. lock, unlock, and islocked methods perform the logical record level locking. You can use whatever concept you want to achieve this: people are using lock-manager, WeakHashMap (yours truly), HashSet coupled with a vector etc. Regardless of the data-structures being used, you have to account for a particular record being worked on using these three methods: lock, unlock, and islocked. The purpose of logical record locking is to inform other threads/clients that this particular record is locked currently and should be considered off-limits until further notification.
The purpose of synchronizing the RAF file-pointer is entirely different. Since the same file pointer is being used by different clients (remember it is defined in the DataSchema class which is designed to be a Singleton), it can be moved around unintentionally by competing threads which are working on different records. If we don't synchronize to make sure that the file-pointer moves as we wish, i.e., read or write entire physical records, we will end-up with getting weird data which is an unpredictable mixture of bytes from different records of the RandomAccessFile.
Therefore, in my opinion, you need both. That is, you need to implement logical record level locking by implementing lock, unlock, and islocked methods using whatever data-structure appropriate to your implementation scheme, e.g., WeakHashMap. Additionally, you need to synchronize on the RAF file-pointer (if you are using one instance of the Data class per client design) or synchronize on the methods writeRecordToFile and read as I mentioned in my post above (if you are using a shared instance of the Data class for all clients - appropriate when using cookies).
John, I think that you are doing what I mention above. But I wanted to clarify just to make sure that we all are on the same page.
Regards.
Bharat
[ October 05, 2003: Message edited by: Bharat Ruparel ]
[ October 05, 2003: Message edited by: Bharat Ruparel ]

[Andrew Monkhouse]

Hi everyone,

John anytime I read a record I do not need to lock it ..., but within the read() method of Data.java I need to wrap part of it ... in a synchronized block to make sure the pointer does not get moved to the incorrect location in the middle of reading a record. I presume what I said here is correct and that this code is all right, is it?

Yep - all sounds good.

John What advice would you give [regarding records being created in the middle of doing a find]?

If you synchronise the find method, you are effectively stopping any other threads from accessing the database for the entire time you are doing the find. This is quite a performance hit for the rare case where someone creates a record while you are doing the find.
Personally I wouldn't worry about it - if a record gets added while you are in the middle of a find, then ignore it. Just put a comment in your JavaDoc / design decisions stating that you are ignoring this issue for simplicity / performance.

John 3. Regardless of synchronizing or not in the find() method, scenarios [where the data returned from the read() does not match the criteria specified in the find()] may arise [, resulting in the wrong data being displayed] on the screen! Is this acceptable?

Nope - not acceptable.
The Data.find() method will do "starts with" matches, whereas the GUI must only display "exact" matches.
So you already have to have a method (in the GUI (IMHO)) that will verify whether the data returned by read() is an exact match or not. This routine can automatically discard any records that have been modified.
But this brings up another issue: when you go to book the record, if the details have changed, what do you do? What can you do if you are running fat client? What can you do if you are running thin client? Do you go back to my comment above about just documenting this as an issue?

Ulrich So the right call would be:

    byte[] buffer = string.getBytes("US-ASCII"); ?

Nope - the question mark after the semicolon will cause the compiler to issue an "illegal start of type" error message
Seriously, this is correct.

Bharat I will be happy to buy all of you a beer if you are in the Boston, Massachusetts area. My recommendation? Sam Adams and Coors Light.

Mmmmm, I like the Sam Adams. Had a few while I was working in Atlanta (my project manager there liked them, and she found somewhere in Atlanta that stocked them!!!). Haven't had any for years though. I don't know Coors Light (yet).

Ulrich public String[] readRecord(long recNo) throws IOException, RecordNotFoundException{

Where is this readRecord() method? I thought it was in Data class. But I thought Data class had a method read(). If your Data class is meant to have a readRecord() method, is this the signature given to you by Sun?

Ulrich

    synchronized(mutex2){
         raf.seek(dataSchema.getRecPosition(recNo));
         if(dataHelper.readFlag(raf) == deleted)
             throw new RecordNotFoundException("the Record has been deleted");
         for(int j=0 ; j < numFields; j++){
             fields[j] = dataHelper.readFixedString(raf, fieldLength[j]);
         }   
    }

Just to give you something to think about: if you read an entire record as one big byte array, then you could check whether the record was deleted and convert the byte array into individual Strings outside of the synchronized block.
Now you just have to decide whether this would be more confusing for the junior programmer

Ulrich

raf.read(buffer);

Danger Will Robinson, Danger!
read() is not guaranteed to read the requested number of bytes. Take a look at readFully().

UlrichHere are my questions:

Sorry, I did FBNS a while back, so your questions don't apply to me.

John I do not wrap the inValidRecord() record inside the synchronized block of the read(), update() etc methods ? I suppose you could do it. The reason I don?t is that I want to wrap as little of code as possible inside the synchronized block. Maybe Andrew or Bharat or indeed your good self will let me know if this is a good principle or not!

It is good to limit the amount of code inside a synchronized block. But there is a slight chance that having this validation outside the synchronized block could cause you to read a record which has been deleted.
Personally I think you could go one of two ways here: either have the validation inside the synchronized block which will limit the race condition, or read the entire record as one long buffer within the synchronized block and then run your validation and conversion routines on that read buffer.
Regards, Andrew

[Ulrich Heeger]

Hi Bharat, Andrew, John,
thanks a lot for your answers
Hi Bharat,

I just got through going through your post. Ulrich, it seems to me that you have gotten two cocepts related to locking mixed up. lock, unlock, and islocked methods perform the logical record level locking. You can use whatever concept you want to achieve this: people are using lock-manager, WeakHashMap (yours truly), HashSet coupled with a vector etc. Regardless of the data-structures being used, you have to account for a particular record being worked on using these three methods: lock, unlock, and islocked. The purpose of logical record locking is to inform other threads/clients that this particular record is locked currently and should be considered off-limits until further notification.
The purpose of synchronizing the RAF file-pointer is entirely different. Since the same file pointer is being used by different clients (remember it is defined in the DataSchema class which is designed to be a Singleton), it can be moved around unintentionally by competing threads which are working on different records. If we don't synchronize to make sure that the file-pointer moves as we wish, i.e., read or write entire physical records, we will end-up with getting weird data which is an unpredictable mixture of bytes from different records of the RandomAccessFile.

Sorry for confusion, I haven't clearly explained my concept. But I think your clarifation correponds to the reasons why I use now 2 mutex.
Because before doing the act of locking one record, you will within the lock-method acquire the (synchronization) lock of a mutex like reservedRecords, and if you want to make a reading you will also acquire the (synchronization) lock of a mutex to ensure thread safe access to the disk.
So you could use the same mutex - reservedRecords - for both methods:
lock(){
synchronized(reservedRecords)
....
}
readRecord(){
synchronized(reservedRecords)
....
}
I think the confusion was caused because in case one - within the lock-method - the function of the mutex reservedRecords serve also to ensure no concurrent access of the HashMap.
But in case two- within the readRecord()-method - the mutex serves only to avoid concurrent access to the synchronized code block. Here you could use another mutex, like the static raf,
because you don't need any action on the HashMap. Think about the definition of mutex, the only raison d'�tre of a mutex is, like Andrew wrote:

Mutex is a shortening of "Mutual exclusion" - a mutual (common) device which allows one client access to something, while excluding all others. So the object you synchronize on becomes a mutex.

That means that you synchronize a special object not necessarily to protect the object itself but the code block, you understand?
Greetings
Ulrich