Lock on reading records

[Anna Hays]

Is there a need for locking a record when reading? I think reading is performed very frequently and locking is a big performance hit for a large DB (I know for the assignment, it won't be too big). But if I don't lock it, the record it is reading might be modifying by another thread at the same time. Any suggestions how to overcome this issue???
Oh when I say locking, I mean block synchronizing inside the read(). Not lock(); read(); unlock; Thanks for any advice!

[Satish Avadhanam]

Originally posted by Anna Kafei:
Is there a need for locking a record when reading? I think reading is performed very frequently and locking is a big performance hit for a large DB (I know for the assignment, it won't be too big). But if I don't lock it, the record it is reading might be modifying by another thread at the same time. Any suggestions how to overcome this issue???
Oh when I say locking, I mean block synchronizing inside the read(). Not lock(); read(); unlock; Thanks for any advice!

Hi Anna, that's right. If we don't lock the record, there is a potential danger that we might be reading the changed records. I think its better safe to lock the record while reading.
Hope this helps, thanks.

[Jacques Bosch]

All IO is synchronized anyway, right...!?
So I didn't lock recs when reading during a find.
But I did lock when doing a book / update to make sure I have the latest data.
In my book method, like this:
1) Lock rec.
2) read latest copy of rec.
2.1) compair to current copy. throw ex if not same.
3) Update rec with book owner.
4) unlock rec.
Other than that, I don't think locking for reads is helpful, providing reads (and writes) are synchronized.
J

[Anna Hays]

Hmm... I don't see anywhere saying IO is synchronized. And even if it is, it will need to lock the class, since the db file could be opened more than once, ie, more than one instance of the db file at anyone time.
I kinda reluctant to synchronize the read() method... cos even if I do, it won't really have the effect cos it doesn't require a lock cookie to read the record. unless I use a single instance of the db file for all reads, but this is a nightmare to keep it upto date with all changes of data.
I don't know!! Am I over complicating the assignment??

[Jacques Bosch]

Keep things simple. Instructions specifically say simple is better.
You have to explicitly synchronize your i/o.
What I did.
Data class was a singleton, i.e. one raf object. All access to the raf is done in code synchronized on the raf. So in terms of i/o it's all thread-safe already.
Then you just have to handle your multiple clients with the record locking approach.
Makes sense?
J

[Anna Hays]

If there is only one raf object in the singleton Data class, you are not allowing mulitple writes. Am I right? Like you can not write to the db file at the same time even to a different record. Because there is one file pointer with the raf object. Let me what you think.

[Jacques Bosch]

That is right. Only one i/o operation can be done at any one time. But trust me, for this assignment's scope, that performs just great, and keeps things simple!
According to my opinion anyway!

[Jason Mowat]

Anna,
FWIW, there has been extensive discussion regarding the very points you raise. My solution to the problem was to implement a ReadWriteLock class to handle this very issue. Details of this decision can be found here.
In short, I chose to create a separate class and pushed all read and write operations through it. The class was essentially a thread manager that guards against reads during writes, etc. I received 80/80 on my locking portion, so the decision was good in Sun's eyes.
Hope that helps!
Cheers,
Jason

[Anna Hays]

Thank you guys! I was thinking of doing what Jacques do cos I am a complicated person. Then I review my assignment requirements last night, it says must allow record locking. So I guess locking the entire DB is not an option. So I will go for the LockManager.
Jason, can you give me a reference to Grand's ReadWriteLock pattern please? Thanks!

[George Marinkovich]

Originally posted by Jacques Bosch:
That is right. Only one i/o operation can be done at any one time. But trust me, for this assignment's scope, that performs just great, and keeps things simple!
According to my opinion anyway!

I think Jacques is correct.

[Anna Hays]

So you think when the must requirement says 'A data access system that provides record locking...' does not mean record level locking and means it is OK to lock the entire DB during read/write operations?? If so then I can not think of why the lock cookie is required as provied the the DB interface...
I am doing the Bodgitt and Scarper assignment by the way.

[Jacques Bosch]

I also had B&S.
Think of it this way. Synchronizing on one raf object prevents data from being corrupted by multiple i/o operations. But that does nothing for making sure that multiple clients don't arbitrarily modify records.
You still need to lock > process > unlock to preserve data integrity.
Think of the locking and the synchronization as two different things with different purposes.
J
[ February 22, 2004: Message edited by: Jacques Bosch ]

[Don Wood]

Hi Anna,
There are two kinds of locking that are required.

Record locking which is used by the higher level functions when a record is updated or deleted. This is used to prevent two threads from modifying the same record at the same time. Concurrency comes into play when two (or more) threads are modifying different records. They each have a lock on the record being modified and as long as the records are different the threads are not blocked. If your assignment specifies a lock cookie, it is this level of locking which defines and uses the cookie.

There is a lower level of locking required when the modify or delete needs to read or write the file. This is because seek and read or seek and write are not atomic operations. Imagine the chaos if one thread performed a seek just before a write then lost its time slice and another thread did a seek to a new location then executed a read. When the first thead got another time slice, it executes the write operation to the wrong location in the file. This is why all file i/o (seek followed by either read or write) must be done under a synchronized block.
This is what Jacques meant when he said

Only one i/o operation can be done at any one time.

So with these two levels of locking, the threads run concurrently for different records. When they get down to the level of reading or writing the file, file i/o is synchronized so the threads go one at a time.
Now back to the original question:

Is there a need for locking a record when reading? I think reading is performed very frequently and locking is a big performance hit for a large DB (I know for the assignment, it won't be too big). But if I don't lock it, the record it is reading might be modifying by another thread at the same time. Any suggestions how to overcome this issue???
Oh when I say locking, I mean block synchronizing inside the read(). Not lock(); read(); unlock; Thanks for any advice!

Since all file i/o must be done under a synchronized block, it is not possible to read a partially modified record. You have the lock, no other thread can be writing at the same time. This is why some (perhaps most?) people have decided not to use record level locking when reading a record
[ February 21, 2004: Message edited by: Don Wood ]

[Anna Hays]

I see what you are saying I think. This is my code:

 // locked records, with record number and lock cookie key value pairs
    private static Hashtable lockedRecords = new Hashtable();  
    
    public String[] read(int recNo) throws RecordNotFoundException {  
        String[] record = new String[FIELD_NAMES.length];         
        try {
            RandomAccessFile dbFile = new RandomAccessFile(DBFILE, "r");
            // read record
            if (flag != VALID) {
                throw new RecordNotFoundException("No record found error");
            }
            dbFile.close();
        } catch (EOFException eof) {
            throw new RecordNotFoundException("No record found error");
        }
        return record;
    }    
    public void update(int recNo, String[] data, long lockCookie)
        throws RecordNotFoundException, SecurityException {
        
        // check if given lock cookie = currenly lock cooke if it exists
        /* cookie matched, write record data to DB */
        try {
            RandomAccessFile dbFile = new RandomAccessFile(DBFILE, "rw");
            // write record
        } catch (IOException ioe) {
            System.out.println("Error update record: IO error");
        }   
    }
    public void delete(int recNo, long lockCookie)
        throws RecordNotFoundException, SecurityException {
        
        // check if given lock cookie = currenly lock cooke if it exists
        /* cookie matched, write record data to DB */
        try {
            RandomAccessFile dbFile = new RandomAccessFile(DBFILE, "rw");
            // write record
        } catch (IOException ioe) {
            System.out.println("Error update record: IO error");
        }   
    }
    public long lock(int recNo) throws RecordNotFoundException {
        // record exists, lock it
        synchronized (lockedRecords){
            while (lockedRecords.containsKey(new Integer(recNo))) {
                try {
                    lockedRecords.wait(100); // wait 0.1 second
                }
                catch (InterruptedException ie) {
                    System.out.println(ie);
                    break; 
                }
            }
            // thread gets the lock for the record even if it is interrupted
            lockedRecords.put(new Integer(recNo), lockCookieWrapper);
        }
        return lockCookie;
    }
    public void unlock(int recNo, long cookie)
        throws RecordNotFoundException, SecurityException {
        Long cookieWrapper = new Long(cookie);
        synchronized (lockedRecords) {
            Long lockingCookieWrapper = null;
            if (lockedRecords.containsKey(new Integer(recNo))) {
                lockingCookieWrapper 
                    = (Long) lockedRecords.get(new Integer(recNo));
            }
            if (lockingCookieWrapper!= null 
                && lockingCookieWrapper.equals(cookieWrapper)) {
                lockedRecords.remove(new Integer(recNo));
            } else {
                throw new SecurityException("Invalid lock");
            }
            lockedRecords.notifyAll();
        }
    }

So do you think it has a potential reading partially modified data??? I think so, cos there are different instance of raf. So should I make the raf static? If this is so, then I dont see why we need lock cookie since we just lock the static raf object instead.
[ February 21, 2004: Message edited by: Anna Kafei ]

[Don Wood]

Hi Anna,
I take this code to be pseudocode as the try-catches aren't completely correct. Also, there some missing dbFile.close() calls.
Given that you are creating a new ref for every read and write, it is not necessary to lock the raf. Each one is new and no other thread will access it to change the file pointer. This solution will work, it just seems like a high amount of overhead to open the file again for every read and write.
Your original question:

So do you think it has a potential reading partially modified data??? I think so, cos there are different instance of raf.

Yes, you are right to worry about this. To prevent partially modified data with this design, requires that the record be locked for every read.

So should I make the raf static? If this is so, then I dont see why we need lock cookie since we just lock the static raf object instead.

If you make the raf a singleton, then that requires the two levels of locking I mentioned earlier.
About the lock cookie - does the assignment require a lock cookie? If so, you must have it. If not, then I am not clear what purpose it serves.
Another point about locking. In your lock routine, you have lockedRecords.wait(100) and try again; I believe that this violates the requirements since you have to make the thread sleep until the lock becomes available. The thread needs to wait indefinitely until the thread that owns it unlocks it and issues a notifyall(). (If you are not sure about why it is notifyall() instead of notify(), search the forum as there has been a lot of discussion on that point.)

[Anna Hays]

Thanks for your reply. Yes my code was a pseudo code cos it is too lengthy for a post and too much to read, so I put comments in instead.
I am thinking of locking when reading, but inside the lock() method, it calls the read method, that will give me a dead lock. I could just use the isValid() that just to check the flag of a record and throw RecordNotFound exception if it is not valid. That's it! Modify the lock() method as

synchronize(lockedRecords) {
  // check if record has a lock cookie
  // if not, check isValid
  // if it is valid, put a cookie in to claim the lock
  // else, throw RecordNotFoundException
  // return lock cookie
}

Then in the read(); do

lock(recNo);
//read record
unlock(recNo);

Hopefully this will allow concurrent write/read when it is diff record hey? What do you think??
For the wait(100); one. I make it to wait 0.1 second because, if the other thread which had the lock released the lock, but some how interrupted before notifyAll, this thread still can claim the lock after 0.1 second. Otherwise the threads that are waiting for the lock will stuck there! What's your opinion on this???
[ February 23, 2004: Message edited by: Anna Kafei ]

[Don Wood]

Hi Anna,
I think that if you leave the timeout in the lock code that you will lose points.
Somewhere in the assignment instructions, there is probably a sentence like the following:

Any attempt to lock a resource that is already locked should cause the current thread to give up the CPU, consuming no CPU cycles until the desired resource becomes available.

If your instructions have a statement like this (and I think all the assignments have it) then you need to have threads that are waiting on a lock go to sleep and stay there until the lock is released.
You are making the server do extra work waking up threads checking and going back to sleep to handle what is really a rare case. On a busy server this could really slow things down.

[Anna Hays]

Thank you very much for pointing that out! I will remove it.

[Anna Hays]

After careful reading abbout Jacques Bosch's posts a few times. I finally understand what he meant about why you still need a lock cookie in his design. I was only thinking thread safe, not data integrity. That's a good one to think about. I also will try his approach too, that should clear up a lot of issues when multiple threads are trying to create a record at the same time. Have learnt a lot from you guys, thanks a lot!