• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

B&S, Q: good synchronization?

 
hatim osman
Ranch Hand
Posts: 105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi there...
I am almost done implementing my Data class. However, to be sure and confident of what I have done so for, I need to ask some questions:

1. Which is better, in other wards safer and correct, to synchronize the RandomAccessFile instance representing the underlying database file, or synchronize on an arbitrary object, in order to allow concurrent access to the database file
2. When a record is read from the database file, if one of the fields was blank, then which is more appropriate to do:
a. assign null to the corresponding cell in the String array to be returned.
b. leave things unchanged and assign whatever value to the field cell in the String array to be returned.

My idea is, the absence of value means null, Am I correct? Because if I do so, then I have to be consistent and trim the padded spaces in case the field text value is less then the length of the field in question!? so the String array returned will conatin only exact length of the fields and disregard spaces.
hope to hear from you soon.

Thanx
Hatim
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Hatim,
1. Which is better, in other wards safer and correct, to synchronize the RandomAccessFile instance representing the underlying database file, or synchronize on an arbitrary object, in order to allow concurrent access to the database file
I don't see where your difference is. Whether you are synchronizing on the RAF, or synchronizing on an arbitrary object, you will be in a synchronized block. So only one thread will have access to the physical data file at any given time.

2. When a record is read from the database file, if one of the fields was blank, then which is more appropriate to do:
a. assign null to the corresponding cell in the String array to be returned.
b. leave things unchanged and assign whatever value to the field cell in the String array to be returned.

My idea is, the absence of value means null, Am I correct? Because if I do so, then I have to be consistent and trim the padded spaces in case the field text value is less then the length of the field in question!? so the String array returned will conatin only exact length of the fields and disregard spaces.
null != the empty string (which is what your later comment of trimming spaces sounds like).

Personally I would be looking at what is going to be used at either end of the transmission: what are you going to display in the GUI - a null or an empty string. What are you going to store back in the database when the record is updated - a null or an empty string?

Regards, Andrew
 
hatim osman
Ranch Hand
Posts: 105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi there...
Thanx Andrew for replying. Well, your comments regarding whether fields should be null or no is quite clear.
However, my methods that deal with locking is not synchronized, and I designed my class so that these methods are only called within synchronized blocks. I believe that synchronization that deals with locking and unlocking records are very independent from synchronization that deals with access to the underlying database file (accessing the file shouldn't prevent other threads from locking or unlocking records). So my code looks something like this:


Is it a good programming practice to have more than one synchronized blocks within a single method? Honestly, I never came across something like this, but that also doesn't mean that it doesn't exists. Any comments?

Thanx in advance
Hatim
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Hatim,

Thanks - I now understand your question a bit better.

I think this is very good practice, and done correctly it can improve concurrency.

This is more obvious when you look at (for example) the read() and lock() methods - why should a user be blocked from read() method because another client is using the lock() method; or why should a user be blocked from the lock() method because another client is using the read() method? The (hopefully) obvious answer is that they shouldn't be: their is nothing in common between the two methods so they should be able to run independantly. But to do that requires them to have different mutexes, which then means that code that needs to use both the lock collection object and the file object will possibly look very similar to your suggested delete method.

Which is a long and convoluted way of saying that I agree with your concepts .

Now, on to other things:
  • At the start of your delete method, you verify that the record is locked with the correct cookie. Wouldn't this same code be repeated in other methods (unlock() and update() methods)? If so, wouldn't it make sense to refactor your code to have a private method that verifies the cookie? (And possibly a private method that verifies the existance of records)
  • Hashtable is a syncronized class. If your variable named hashtable is actually a Hashtable then you might not need the explicit synchronization on it within the delete() method. The problem, though, is that you probably will still need explicit synchronization on the hashtable in the lock() method which means that you would be unnecessarily nesting synchronized blocks (with the danger that if anybody changes your code in the future so that the explicit synchronization is on a different object you have the potential for deadlock ). I would strongly recommend that you investigate whether you do need a collection which is internally synchronized, and if not, change to one of the new collections. If you do need it, you might want to consider using Collections.synchronizedMap() (or one of the other similar methods in the Collections class) to create a syncronized collection - this way it will be explicitly clear to the person reading your code that you have determined that a synchronized collection is preferable (in such a case I would also recommend putting implementation comments plus a comment in your design decisions document explaining why you have done this).
  • Regards, Andrew
     
    hatim osman
    Ranch Hand
    Posts: 105
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi there,
    Your remarks are always valuable. I have already realized that, and used a collection that is not synchronized at all, but then I restrict access to it within synchronized blocks only, to make things easier to understand and also to take off a heavy suit of work from the data structure. I have even changed my data structure implementation to a LinkedList, and implemented an immutable inner class LockedRecord that encapsulates information regarding records that are locked :




    what do you think? I still have some unclear matters regarding the unlock method. It says that, this method should throw SecurityException and RecordNotFoundException. Now, what should I do in case a thread attempts to unlock a record that is not locked in the first place and therefore has no reference in the data structure:
    1. Throw a RecordNotFoundException (but what if the record is indeed a valid record but just not locked)
    2. Throw a SecurityException ("Yuk")
    3. return peacefully

    Also, you have proposed a method that checks whether a record is valid or no; well logically, this method then needs to access the file to check the validity byte. hence, it must lock the RAF object! which will lead to my first state of nested synchronization:


    [
    }
    return flag == VALID_RECORD_BYTE;
    }

    The lock and unlock methods must then call this method too (factoring the functionality), to determine the validity of the record in question,



    The code looks fine for me, but is there any danger, will such semantics cause a deadlock (note that they synchronize on two diff. objects)?

    I also intend to implement the private isValidCookie() method which synchronize on the data structure, and should be called within CRUD methods the modify the file, such like the delete method. Is retrieving an object from a data structure (without remiving the object) require synchronization when the data structure is accessed by multiple thread?

    thanx in advance
    Hatim
     
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander
    Pie
    Posts: 12007
    215
    C++ Firefox Browser IntelliJ IDE Java Mac Oracle
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Hatim
    what do you think?
    Sorry, I don't understand what you are asking about here.

    I still have some unclear matters regarding the unlock method. It says that, this method should throw SecurityException and RecordNotFoundException. Now, what should I do in case a thread attempts to unlock a record that is not locked in the first place and therefore has no reference in the data structure:
    1. Throw a RecordNotFoundException (but what if the record is indeed a valid record but just not locked)
    2. Throw a SecurityException ("Yuk")
    3. return peacefully
    In this case, won't the client be supplying a cookie other than a correct cookie for that particular (non-existant) lock? In which case, wouldn't a SecurityException be thrown?

    Or, another way of looking at it, aren't we implying that in this case the client is trying to unlock a record that they should not be trying to unlock? Again, isn't a SecurityException the right way to tell them off?

    Why do you have a "Yuk" response to this?

    you have proposed a method that checks whether a record is valid or no; (...) logically, this method (...) must lock the RAF object! which will lead to my first state of nested synchronization (...)

    The code looks fine for me, but is there any danger, will such semantics cause a deadlock (note that they synchronize on two diff. objects)?
    Yes, there is that potential danger whenever there are nested locks. You need to make sure it cannot happen in your code, and probably write implementation comments to warn the "junior programmer" of this potential problem. You might find that simply stating a rule (for example, specifying in which order the respective mutexes may be obtained) may stop such a problem from occuring.

    Is retrieving an object from a data structure (without remiving the object) require synchronization when the data structure is accessed by multiple thread?
    If there is no way that the data structure can be updated then there is no need for synchronization. However if there is even the slightest potential for the collection to be updated then you must synchronize retrievals as well, otherwise you risk NullPointerExceptions, ArrayIndexOutOfBoundExceptions and invalid data (all of which might only appear in production - there is no guarantee that you would spot them in testing, and they would be hard to track down since most of the time everything would appear to work perfectly).

    Regards, Andrew
     
    hatim osman
    Ranch Hand
    Posts: 105
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi there...
    thanx Andrew, I feel like I understood Data.java like I never had before. Wish you all the luck

    Hatim
     
    hatim osman
    Ranch Hand
    Posts: 105
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Now, lets me move to the RecordNotFoundException, when we lock a record, a RecordNotFoundException is thrown if the record doesn't exists. Logically, the unlock() method should never throw that Exception since the lock() method guarantees the record existence before it's being locked. This dilemma raises the follwoing scenarios:

    1. unlock() method should throw RecordNotFoundException if the record doesn't exists in the database file, (that means we will never be able to unlock records that we locked, and then deleted coorectly)
    consider this scenario:


    2. unlock() method should throw RecordNotFoundException, if the data structure that holds references to the locked records doesn't conatin the record in question.
    This semantic contradicts the lock() method logic, lock() method adds the record to the data structure and there is no way on earth that a re-search for the record fails.....Unless we allow two threads to modify the same locked record (very wrong)

    3. A final resolution, is to make the CRUD methods that modify the file unatomic, meaning:

    //very bad implementation of update
    public void update(int recNo, String[] data, long cookie) {
    //update the record

    unlock(recNo);//ugly, may be we still want to update the same record!
    }


    Please enlighten me guys, everytime I think I am done something comes up.

    Hatim
     
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander
    Pie
    Posts: 12007
    215
    C++ Firefox Browser IntelliJ IDE Java Mac Oracle
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Hatim,

    You might be better off creating a new topic for this - it doesn't really have anything to do with "good sycnhronization".

    Regards, Andrew
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic