delete method code

[Tim Fernandez]

How is this for the delete code:

  
 
public void delete(int recNo) throws RecordNotFoundException {
try {
long recOffset = seekFileOffset(recNo);
synchronized (dbFile) {
dbFile.seek(recOffset);

if (isRecordDeleted(dbFile.readByte())) {
throw new RecordNotFoundException();
}
 
dbFile.seek(recOffset);
dbFile.writeByte(DELETEDRECORD);
}
} catch (IOException ioe) {
throw new RecordNotFoundException("Cannot find the specified record to update");
}
 

Please give your comments.

Thanks in advance,
tim

[Andrew Monkhouse]

Hi Tim,

Do you have the following comment in the instructions for your required lock() method?

// Locks a record so that it can only be updated or deleted by this client.

If so, how are you ensuring that a record can only be deleted by the client who locked the record?

Regards, Andrew

[Ed Tse]

Andrew, I have almost the exact same code as Tim and Andrew, your comment is super. I completely missed that detail. So be for we delete or update, we should check the vector that stores all the locked record. If it exists, allow it to be deleted or updated. Is that about it?
[ March 05, 2006: Message edited by: Ed Tse ]

[Saheed Adepoju]

Hi all
This is shameless, but i am using this opportunity to ask about what you guys think about my own delete method. Here it goes:

 public void delete(int recNo, long lockCookie) throws RecordNotFoundException, SecurityException {
 
        synchronized(contractorRecords){
           validateRecord(recNo);
            if(!reservedContractors.containsValue(lockCookie)){
                throw new SecurityException("Client Didnt Lock Record");
            }else {
                try {
                   if(recNo == 1){
                       raf.seek(offset);
                   }else{
                       raf.seek(offset + ( recNo - 1 ) * (recordLength + VALID_RECORD_INDICATOR ));
                   }
 
                    raf.writeByte(INVALID_RECORD_INDICATOR);
 

 
                }catch(IOException io){
                    throw new DataAccessException("Error Accessing Data File",io);
                }
 

validateRecord() throws RecordNotFoundException if the record is non-existent or has been deleted. I think i have everything checked for. Your inputs will highly be appreciated.

[Carlo Spelgatti]

How do you manage the deleted records, if your delete method is like this Tom?
I also need to put the deleted record in a list, managing the deleted records.

Carlo

[Tim Fernandez]

Originally posted by Andrew Monkhouse:
Hi Tim,

Do you have the following comment in the instructions for your required lock() method? If so, how are you ensuring that a record can only be deleted by the client who locked the record?

Regards, Andrew

Shouldn't this be handled in the db.client code?Meaning, the code is as is right now but when the method is called from of the db.client classes then a call to data.lock() is made first before calling the delete() method.

[Kevin Conaway]

Tim,

I think you should make every effort to protect the client (when I say 'client', I meant a client of the database) from themselves. So, if a client requests a modification (update/delete), you should check to make sure they have the lock for that record before making the modification.

Kevin

[Tim Fernandez]

Originally posted by Kevin Conaway:
Tim,

I think you should make every effort to protect the client (when I say 'client', I meant a client of the database) from themselves. So, if a client requests a modification (update/delete), you should check to make sure they have the lock for that record before making the modification.

Kevin

How can we make this possible?Does it mean i need to call the lock method in Data before deleting a record?

By the way, when i say client class, i mean, client.* In this case, the clients themselves should make the call to the lock method to make sure the update/delete operation is only exectued by them.

I don't see though how this can be implemented in the delete method itself other than maintaining a map which contains the lock owners and then checking the map if contains something before making the delete operation.

regards,
tim

[Andrew Monkhouse]

Hi Tim,

Shouldn't this be handled in the db.client code? Meaning, the code is as is right now but when the method is called from of the db.client classes then a call to data.lock() is made first before calling the delete() method.

The client should do what you suggest, however the instructions indicate that the Data class itself should enforce that this is happening.

I don't see though how this can be implemented in the delete method itself other than maintaining a map which contains the lock owners and then checking the map if contains something before making the delete operation.

You have the basic concept for the solution. There are multiple ways you can achieve this - rather than me suggesting one (or having me suggest multiple solutions ) I suggest you think about it, and explore your ideas here.

Regards, Andrew

[Tim Fernandez]

Originally posted by Andrew Monkhouse:
Hi Tim, You have the basic concept for the solution. There are multiple ways you can achieve this - rather than me suggesting one (or having me suggest multiple solutions ) I suggest you think about it, and explore your ideas here.

Regards, Andrew

Hi Andrew,

I've modified the following code to incorporate locking

  
 
public void delete(int recNo) throws RecordNotFoundException {
recordLock.writeLock.lock();
                try {
                    try {
long recOffset = seekFileOffset(recNo);
 
synchronized (dbFile) {
dbFile.seek(recOffset);

if (isRecordDeleted(dbFile.readByte())) {
throw new RecordNotFoundException();
}
 
dbFile.seek(recOffset);
dbFile.writeByte(DELETEDRECORD);
}
} catch (IOException ioe) {
throw new RecordNotFoundException("Cannot find the specified record to update");
}
 
              } finally {
                  recordLock.writeLock.unlock();
              }
 
  

recordLock here is an instance of ReentrantReadWriteLock. The additional code assures that whoever started the lock on the record, is the only one who can unlock it.

Is this okay?

regards,
tim

[Ed Tse]

Andrew, can you shine more light? My take will be like this

in lock(), get the Thread.currentThread().getName(), if it's empty, assign an unique name to it and associate with the record that this thread locked. Would this be an acceptable approach?

[Andrew Monkhouse]

Hi Tim,

I think I am missing some part of your overall design. How are you associating recordLock with the client?

Regards, Andrew

[Andrew Monkhouse]

Hi Ed,

in lock(), get the Thread.currentThread().getName(), if it's empty, assign an unique name to it and associate with the record that this thread locked. Would this be an acceptable approach?

That depends on a number of other issues.

If you are using Sockets, then this is probably acceptable.

If you are using RMI

is your client calling some remote book() method (where the book() method on the server calls lock() and delete()) - if so, then this might be acceptable.

Is your client calling lock() itself - if so, how are you ensuring that the thread name will remain constant across two separate remote method invocations, given that they can end up running on different threads?

Regards, Andrew

[Ed Tse]

Andrew,
I am going to use RMI and probablt have a facade that does the book method with lock and unlock. However, is there another other way to do it that is better?
[ March 08, 2006: Message edited by: Ed Tse ]

[Bob Nedwor]

The most important thing K&B taught me on Threads:
When a thread tries to access some synchronized code, it needs to obtain the lock on the entire object where that synchronized code resides. So no other thread can access any other synchronized code in that same object until the original thread finishes.

In both the examples above, you guys are making it tough for two different threads to come in at the same time and delete two different records. The 2nd thread has to wait, even if it is deleting a different record.

Tim and Saheed, it looks like you are both tying up all the synchronized code in your entire Data objects. This may not be bad with 33 records, but from a design standpoint I totally disagree.

Just my two cents, but tell me if you think I am wrong...
--Bob
[ March 09, 2006: Message edited by: Bob Nedwor ]

[Tim Fernandez]

Originally posted by Andrew Monkhouse:
Hi Tim,

I think I am missing some part of your overall design. How are you associating recordLock with the client?

Regards, Andrew

Hi Andrew,

I just created this recordLock (ReentrantLock) and it is not all associated to the client. It's just that i thought enclosing the the code i have above with this recordLock will ensure that the one who locked can only be the one to update it and unlock it. In the SCJD book though, this is not at all done (ensuring that only a client who locks the update code can update the code) or is it?

[Saheed Adepoju]

 
Client 1 wants to book record 1
Client 2 wants to delete record 1
 
Client 2 calls delete(within Adapter class) 
Client 2 acquires lock()
Client 1 calls book()
Client 1 calls lock() -- waits cause it is within structure
Client 2 calls delete()
Client 2 deletes record
Client 2 unlocks() record calls notifyall() notifies waiting threads on datastructure
Client 1 acquires lock() again and then throws RecordNotFoundException
 
 

So Bob, i really dont know your take on this scenario but I wouldnt mind if you gave me what you think. I have an adapter class and i dont think there is too much "sync"ing here. And also i wuldnt mind if i get an insight into your statement

Tim and Saheed, it looks like you are both tying up all the synchronized code in your entire Data objects. This may not be bad with 33 records, but from a design standpoint I totally disagree.

Thanks!


[ March 09, 2006: Message edited by: Saheed Adepoju ]

[Tim Fernandez]

Originally posted by Tim Fernandez:


Hi Andrew,

I just created this recordLock (ReentrantLock) and it is not all associated to the client. It's just that i thought enclosing the the code i have above with this recordLock will ensure that the one who locked can only be the one to update it and unlock it. In the SCJD book though, this is not at all done (ensuring that only a client who locks the update code can update the code) or is it?

Forget about ReentrantLoc. and what if in my data class, i have something like the code below. The dFile is the worker class (does all the dirty file operations). What is inside dFile.delete(recNo) is the same as what i've earlier posted but without the ReentrantLock.

   
      public void delete(int recNo) throws RecordNotFoundException {
                try {
lockManager.lock(recNo, this);
dFile.delete(recNo);
} finally {
lockManager.unlock(recNo, this);
}
      }
 

Any comments?

[Bob Nedwor]

Saheed:
I don't see any reason why that scenario wouldn't work. But it is a different point, than what I was trying to explain (multiple contractors). But I take it back if your public void delete() method in your post on March 06, 2006 03:03 AM, is in your "adapter class" (which I hope is specific to each contractor, at least). What class is this public void delete() method in???

As long as a thread can go in and delete or update a specific contractor,
without having to wait on another thread that is trying to do something with a different contractor you should be OK.

Tim:
Your dFile class looks better, but it depends how it is instanciated. Is it a contractor-specific instance? To me a good design would allow one Thread that is trying to delete contractor A, NOT have to depend on waiting until another Thread is finished deleting contractor B. If you have both threads trying to access the lock on the same dFile object, you don't have such a great design in my opinion. So if a separate dFile object is launched for a given contractor, you are the man and your design rocks.. If not, I am not going to say that you will fail, and your system will probably still work great by appearance, but we get 30 points for OO design.

I hope this makes sense.
--Bob

[Saheed Adepoju]

Hi all
Bob: Thanks alot, i get your point and i will look at my code critically trying to resolve allowing a contractor to delete another record without having to wait for another thread to notify it even though it is working on the record the it wants to delete. Am i right? The public void delete() i posted on the 6th of March is in my Data.java. I however dont have a delete() in my adapter class! Thanks for your critical input!

[Bob Nedwor]

Cool, yes and also remember: "Locking (80 points)"!!

Check out Chapter 4 of Andrew's book and / or Ch9 of the K&B book.