• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

About:My URLyBird1.3.2 Locking

 
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I use a HashMap to lock the threads.
My lock/unlock/isLocked methods,code in Data.java which implements DBMain following:



AnotherDBMain implemented by suncertify.db.DataAdapter client end,not by suncertify.remote.DatabaseImpl server end,the segment code that lock/unlock a record in data file following:
 
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,
To my PERSONAL opinion it is not Ok, since user lock(lock/unlock/isLocked) is something different from aynchronization locks.
If a user has locked a record, but has not started updating it, another user should be able to read the record, which is not possible in your implementation.
The Interface says:
// Locks a record so that it can only be updated or deleted by this client.
It says nothing about read, find, create methods.
I know that one person, which had something simular to your design had got 100% for locking, but:
1. I am not sure that really had this design
2. I am not sure that Sun tester has carefully looked at his code.
I discussed the design with Andrew. My personal opinion it is a design which will can lead to get max scores, but which can lead you to fail.
I don't know
Could send comment in interface for you lock methods?
Max, please take a part in the topic!!!
Best,
Vlad

I am not really sure
[ September 11, 2003: Message edited by: Vlad Rabkin ]
[ September 11, 2003: Message edited by: Vlad Rabkin ]
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Vlad:
thanks for your comment.

My personal opinion it is a design which will can lead to get max scores, but which can lead you to fail.


I am afraid to fail out of life.


If a user has locked a record, but has not started updating it, another user should be able to read the record, which is not possible in your implementation.


It's correct.


The Interface says:
// Locks a record so that it can only be updated or deleted by this client.
It says nothing about read, find, create methods.


Is it said "I can only implement the lock/unlock methods for the update and delete method,another methods ignore that."?
To your opinion,How should I rework my lock/unlock/isLocked methods to let them accord with the explanation for these methods.
otherwise,How should I using the isLocked method?
thanks for all response.
[ September 11, 2003: Message edited by: xi ruo ]
 
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
private static int DATABASE_LOCK=-1;
Should be declared as "private static final int".

public void lock(int recNo) throws RecordNotFoundException {
Integer recordNumber = new Integer(recNo);
synchronized (mapLocks) {

Synchronize the entire method, instead of the block, -- it will be much more readable and 99.99% of the time spent in your method is spent in the synchronized block anyway.

catch (InterruptedException e) {
throw new RecordNotFoundException(UNEXPECTED+":Interrupt ocurred. Could not complete lock.");
}

This is bad. You are mapping the InterruptedException into a RecordNotFoundException, which is misleading to the caller of the method.
mapLocks.put(recordNumber, this);
Which object is pointed to by "this"? You said that your methods are part of Data, so "this" must be an instance of Data?
while (mapLocks.get(recordNumber) != null && mapLocks.get((recordNumber).equals(this)
There is no reason to call mapLocks.get(recordNumber) twice. Also the while loop is misleading here, -- you are not iterating and you are not waiting on a condition. A simple "if" will do it.
[ September 11, 2003: Message edited by: Eugene Kononov ]
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,

Is it said "I can only implement the lock/unlock methods for the update and delete method,another methods ignore that."?


That is how I understand URLyBird 1.1.1 assignement.
Could you please send descption of lock/unlock/isLocked method in your assignmennt? It seems our projects are a bit different: I don't have isLocked() and I don't have database lock for lock/unlock.
Best,
Vlad
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Eugene,Thank you.


private static int DATABASE_LOCK=-1;
Should be declared as "private static final int".


Sorry,it's my carelessness.In my code,I declare as this.


public void lock(int recNo) throws RecordNotFoundException {
Integer recordNumber = new Integer(recNo);
synchronized (mapLocks) {

Synchronize the entire method, instead of the block, -- it will be much more readable and 99.99% of the time spent in your method is spent in the synchronized block anyway.


Is synchronize the entire method the same as synchronize the static instance of HashMap which is called "mapLocks"?


catch (InterruptedException e) {
throw new RecordNotFoundException(UNEXPECTED+":Interrupt ocurred. Could not complete lock.");
}

This is bad. You are mapping the InterruptedException into a RecordNotFoundException, which is misleading to the caller of the method.


I rework this following:


mapLocks.put(recordNumber, this);
Which object is pointed to by "this"? You said that your methods are part of Data, so "this" must be an instance of Data?


It's correct."this" is an instance of Data.


while (mapLocks.get(recordNumber) != null && mapLocks.get((recordNumber).equals(this)
There is no reason to call mapLocks.get(recordNumber) twice. Also the while loop is misleading here, -- you are not iterating and you are not waiting on a condition. A simple "if" will do it.


Rework following:

About "if" statement or "while" statement,there is a segment saying by Andrew
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Vlad,descption of lock/unlock/isLocked method in my assignmennt:
Required Interface


Your data access class must be called "Data.java", must be in a package called "suncertify.db", and must implement the following interface:
package suncertify.db;
public interface DBMain {
// Reads a record from the file. Returns an array where each
// element is a record value.
public String [] read(int recNo) throws RecordNotFoundException;
// Modifies the fields of a record. The new value for field n
// appears in data[n].
public void update(int recNo, String [] data)
throws RecordNotFoundException;
// Deletes a record, making the record number and associated disk
// storage available for reuse.
public void delete(int recNo) throws RecordNotFoundException;
// Returns an array of record numbers that match the specified
// criteria. Field n in the database file is described by
// criteria[n]. A null value in criteria[n] matches any field
// value. A non-null value in criteria[n] matches any field
// value that begins with criteria[n]. (For example, "Fred"
// matches "Fred" or "Freddy".)
public int [] find(String [] criteria)
throws RecordNotFoundException;
// Creates a new record in the database (possibly reusing a
// deleted entry). Inserts the given data, and returns the record
// number of the new record.
public int create(String [] data) throws DuplicateKeyException;
// Locks a record so that it can only be updated or deleted by this client.
// If the specified record is already locked, the current thread gives up
// the CPU and consumes no CPU cycles until the record is unlocked.
public void lock(int recNo) throws RecordNotFoundException;
// Releases the lock on a record.
public void unlock(int recNo) throws RecordNotFoundException;
// Determines if a record is currenly locked. Returns true if the
// record is locked, false otherwise.
public boolean isLocked(int recNo)
throws RecordNotFoundException;
}
Any unimplemented exceptions in this interface must all be created as member classes of the suncertify.db package. Each must have a zero argument constructor and a second constructor that takes a String that serves as the exception's description.
Any methods that throw RecordNotFoundException should do so if a specified record does not exist or is marked as deleted in the database file.


Locking


Your server must be capable of handling multiple concurrent requests, and as part of this capability, must provide locking functionality as specified in the interface provided above. You may assume that at any moment, at most one program is accessing the database file; therefore your locking system only needs to be concerned with multiple concurrent clients of your server. 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.

 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi and Eugene,
Eugene:

This is bad. You are mapping the InterruptedException into a RecordNotFoundException, which is misleading to the caller of the method.


Eugene, Unfortunatelly the method in interface (definded by Sun) of URLyBird has no other exceptions to be thrown.
1) I throw my RuntimeException in case of InterruptedException.
2) Some people throw RecordNotFoundException or its subclassed checked exception rejecting to consider Interrupted exception as an RuntimeException.
3) Some people ignore it at all and just log the problem on the server.
I don't like 2 (as you) and 3, but it seems that at least solution 2 is also OK, since there have people who got perfect scrores by doing it.
So, I would not so critisize solution 2. I find it at least better than 3, since I beleive the client must be somehow notified about the problem.

Best,
Vlad
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,
Well, we have different interfaces: you don't have cookies and you do have isLocked method.
I don't really understand why isLocked method, since it is usefull only for internal use in the class which implements your this interface.
Still, there is following comment for lock method:


// Locks a record so that it can only be updated or deleted by this client.
// If the specified record is already locked, the current thread gives up
// the CPU and consumes no CPU cycles until the record is unlocked.


Does it says that "Locks a record so that it can only be READ by this client." ? No.
Xi, I am not able to provide with the clear answer. I personally wouldn't use such chain: lock-read-unlock. I beleive lock/unlock is usefull for such things like booking a room : lock-read-update-unlock (you read there to make sure the record is still available after you acquired a lock).
It is my personal opinion and there is no way I want to prevent you from doing what you suggested.
I still hope that somebody else can provide you with more definite response.
Just be very carefull there!
Best,
Vlad
 
John Smith
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
while(this.equals(mapLocks(recordNumber)))
About "if" statement or "while" statement,there is a segment saying by Andrew

I am not sure what segment you refer to. The "while" loop implies iteration which you don't have, and therefore the "if" would be much more expressive here.
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Vlad,
My booking room method is in the suncertify.gui.GUIController class,implement following:
DataAdapter is the class which provide read/update/find/create/delete methods

Booking a room:
method-chain like this: find-->read-->update
then,lock-chain like this: lock(-1)-->find-->unlock(-1)-->lock(recordNumber)-->read-->unlock(recordNumber)-->lock(recordNumber)-->update-->unlock(recordNumber)
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
About "if" statement or "while" statement,there is a segment saying by Andrew:


For the unlock, you need to know that you own the lock, and if so, you need to unlock it. Even if this thread was sliced out after the if statement was evaluated as true and before the resulting actions take place, nothing will have changed - no-one else could have removed your lock. Maybe coding a while statement will be better for future enhancements (I cant see how in context), but my personal feeling is that the if statement is slightly more readable in this case.

 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
My booking method is so bad that cause that my sense babelism.
I sincerely consult a good way for the booking method.
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,
Let's try to do everything step by step. Forget about GUIClient.
I would first finish with the server side.

method-chain like this: find-->read-->update


It looks like you have decided to call lock/unlock not on the client , but on the server (in your DataAdapter class).
Sequence on client find-->read-->update causes following on the server (in you implementation):
lock(-1)-find-unlock(-1)-lock(X)- read(X) - unlock(X)-lock(X) -update(X)-unlock(X). Right?
If so, you forgot to check in your update method on the server if the room still available, because betweem you read and update somebody could have booked the record!
That is actually why you need lock method:
1) lock record (everybody should be able to read a record, but no other client may change it)
2) read the record to check if the romm is still available to be booked
3) update the record
4) unlock the record
If any other client attempts to change the record at the same time he will wait (his lock call will block till the first client unlocks the record)
You have to decide where you check if the room is still available. Since you hided lock/unlock from the client your update method in adapter must do it before update record, otherwise following can happen:
Room #3 is available
1. Client 1 reads the record and knows now that the record is available
2. Client 2 reads the record and decided to book it. It is not available anymore
3. Client 1 decides also to book the room and do succesfully. It shouldn't happen, because the Client 2 has booked already the record!
Did you get the idea?
Best,
Vlad
 
author and jackaroo
Posts: 12200
280
Mac IntelliJ IDE Firefox Browser Oracle C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,
When referring to something somebody else was talking about, it is nice to also post a link to the thread where the discussion was being held. That way people can read the entire text in context if they want to.
In the section you quoted from me, from memory (I cannot find the post anymore ) I was discussing with Max whether we should use an "if" or a "while" statement in the unlock method. I felt that an "if" was OK, since the object we were checking could not change. Max felt (again this is from memory) that all such checks should be in a while loop.
Since you are storing "this" instance of the Data class in your lock, you are correct to have a separate synchronized block on the static map. You cannot synchronize on the entire method.
Regards, Andrew
[ September 11, 2003: Message edited by: Andrew Monkhouse ]
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Andrew
About the saying I quoted from you,the thread following this link:
source of the quotation


Since you are storing "this" instance of the Data class in your lock, you are correct to have a separate synchronized block on the static map. You cannot synchronize on the entire method.


In other words,you agree to separate synchronized block on the static HashMap?So,My coding like this is OK?
[ September 11, 2003: Message edited by: xi ruo ]
 
Andrew Monkhouse
author and jackaroo
Posts: 12200
280
Mac IntelliJ IDE Firefox Browser Oracle C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,
Your link points back to this page.

In other words,you agree to separate synchronized block on the static HashMap?So,My coding like this is OK?


Yes.
Regards, Andrew
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Vlad,
Now I have another idea to book a selected record instead of directly coding in the suncertify.gui.GUIController class.
I add a extra method to AnotherDBMain interface which is implemented by DataAdapter,this method definition following:

In my DataAdapter class

so,this locking chain like this:lock-read-update-unlock
Please comment.
[ September 12, 2003: Message edited by: xi ruo ]
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
sorry,Andrew,
source of the quotation
 
Andrew Monkhouse
author and jackaroo
Posts: 12200
280
Mac IntelliJ IDE Firefox Browser Oracle C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi
Wow - I am impressed you read all the way through that topic.
I assume that the code you posted was just to give an indication of what you intend to do - it is nowhere near complete code.
As such, you have got the basic calls correct, now all you need is to flesh it out.
Regards, Andrew
 
xi ruo
Ranch Hand
Posts: 40
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Andrew,


I assume that the code you posted was just to give an indication of what you intend to do - it is nowhere near complete code.
As such, you have got the basic calls correct, now all you need is to flesh it out.


Yes,this is only my notion and unimplemented.
To your opinion,Is it viable?Another question is that which end is the implementation,server or client?
[ September 12, 2003: Message edited by: xi ruo ]
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Xi,

so,this locking chain like this:lock-read-update-unlock


Yes. It looks now much better!
Best,
Vlad
 
Greenhorn
Posts: 20
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I was mainly concerned about the use of Thread.currentThread(). The RMI specifcation does not garantee that a client will always use the same server thread.
So is currentThread a proper identification of the client holding the lock?
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Arjan,

So is currentThread a proper identification of the client holding the lock

.
It is Ok for Socket Network Implementation, but If your implementation is based on RMI or you want your implementation be copatibale with RMI, the answer is NO!.
Best,
Vlad
 
Andrew Monkhouse
author and jackaroo
Posts: 12200
280
Mac IntelliJ IDE Firefox Browser Oracle C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Arjun
As Vlad pointed out, you cannot use the thread as your identifier in RMI.
However Xi was only using it for reporting purposes (which is also invalid, but he will hopefully be pulling those System.out statements out of his code before submission).
Xi was actually using the instance of the Data class as his unique identifier. This is reasonable.
Regards, Andrew
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic