lock() > delete() > unlock() > BANG!

[Simon Cockayne]

Hi all,

My assignment states:

"Any methods that throw RecordNotFoundException should do so if a specified record does not exist or is marked as deleted in the database file."

When deleting a record a typical client might expect to:
lock() the record, delete() the record and then unlock() the record.

But according to the "API", unlock() should throw RecordNotFoundException because by that time my record IS marked deleted.

Thoughts?

Simon

[Jar Jaquiso]

Hello Simon,

I had the same problem and I solved it by only throwing a RecordNotFoundException in the unlock method only when the record does not exist at all, not when it is marked as deleted.

The lock method only looks for existing, not deleted records. This means nobody can lock a deleted record. The update and delete methods verify that the given record is locked, so no record is able to be updated after being deleted.

Cheers,

Jar

[Mike Ngo]

I don't see how unlock() can throw RecordNotFoundException. If the lock cookie is invalid, i.e. it does not exist or it does not match the recordNo, then a SecurityException is thrown. Otherwise, the lock is released.

[Mike Ngo]

Actually my question would be what is the deleteRecord api for? Allowing CSR to delete records is unthinkable.

[Simon Cockayne]

Hi Jar,

I went to sleep thinking about the delete method and I woke up thinking about the delete method.

(I take comfort in the fact that at least I didn't dream about the deleted method).

I see that you are saying since you do not allow lock() to lock a deleted method then unlock() would never have to unlock a deleted record.


That's fine...EXCEPT...what if the client DOES call unlock() on a deleted record? The assignment says you SHOULD throw RecordNotFoundException in this case and you are not doing that...

Cheers,

Simon

[Jar Jaquiso]

Hi Simon,

Well, if you mean that a client calls unlock for a deleted record without previously calling lock, yes, that would take us to the situation where a RecordNotFoundException should be thrown and is not thrown.

But it would be extremely difficult to get to this situation (though not impossible) because:

The client should be working in an erroneous manner, that is not locking, doing some operation and unlocking, but only unlocking.

The client should send to the unlock method a valid and deleted record ID. This is very difficult as my application does not serve any deleted records, ergo the client must guess.

If the client succeds in the previous 2 points there would not be much harm done as the only thing that would happen is that an exception would not be thrown, but the database would not be modified nor would any of the applications internal objects. And as a matter of fact my implementation is using a java.util.concurrent.locks.ReentrantLock object to manage locking so when the client tries to unlock an unlocked record a java.lang.IllegalMonitorStateException (a runtime exception) will be thrown.

So, even though a client can try to unlock an unlocked record it will get an exception (not a RecordNotFoundException, but even this could be arranged by capturing the IllegalStateException an throwing a RecordNotFoundException).

I hope this helps you,

Jar

[Simon Cockayne]

Jar,

Here's another anomaly.

Lock should throw RecordNotFoundException on a deleted record.

But if that is the case, how can you reuse a deleted record (possibly required by create())? Surely the deleted record to be reused should be locked before it is reused, but any attempt to lock it with lock() should throw RecordNotFoundException!

Simon

[Simon Cockayne]

Jar et al,

Ok I have had a revelation.

I cannot use lock/unlock properly with deleted records if I keep to the assignment spec.

So...I'll just also create lockDeletedRecord() and unlockDeletedRecord().

I mean the assignment spec does not say..."You have to use the lock() and unlock() methods in the interface for ALL your locking needs".

Si

[Mike Ngo]

As far as the database concerns, a deleted record does not exist so if you try to lock a deleted record an exception should be thrown.
It can happen. If you have two clients looking at the same list of records and one client deletes a record and the other client tries to lock and update it later.
Can it happen in a URLyBird application? Probably never. But here you are writing a database server so you should assume that it can happen.

[Simon Cockayne]

Mike,

What about the case where create() wants to reuse a deleted record?

Surely then you would want to lock the deleted record first, to make sure that two clients aren't both trying to reuse the deleted record?

Simon

[Mike Ngo]

I mean the assignment spec does not say..."You have to use the lock() and unlock() methods in the interface for ALL your locking needs".

I think the DBAccess interface is for implementing a low-level database layer. You don't expose the lock() and unlock() api to the higher-level software. If you are using a real DBMS like Oracle and you send in a UPDATE SQL statement, Oracle will do the lock/unlock work for you.

[Simon Cockayne]

Mike,

I hear what you are saying.

But regardless of whether you expose or hide the methods in the data access class at a higher level...the data access class Data.java MUST implement the interface.

And the interface description says RecordNotFoundException should be thrown if the record is not found or is deleted.

Si

[Mike Ngo]

What about the case where create() wants to reuse a deleted record?

The assignment does not include implementing a complete database using a file. But in general, when a record is deleted it is simply marked as deleted so that when a query is done, it simply ignores the deleted records.
When a new record is inserted, the database searches for a deleted record that is big enough to hold the new record and reuses the space.
In the case of URLyBird, the new record has to be inserted at the end of the file because the data file format does not allow gaps between records.

Surely then you would want to lock the deleted record first, to make sure that two clients aren't both trying to reuse the deleted record?

This is bad. The clients should only call createRecord() and let your database take care of that. Reuse of deleted records or inserting new records at end of file is a database implementation issue.
[ November 05, 2006: Message edited by: Mike Ng ]

[Simon Cockayne]

Mike,

Ok...client calls createRecord().

createRecord calls lock ( on a deleted record) > insert data > mark as UNdeleted > unlock.

But..according to the spec...lock on a deleted record should throw recordnotfoundexception.

Si

[Mike Ngo]

Ok...client calls createRecord().

createRecord calls lock ( on a deleted record) > insert data > mark as UNdeleted > unlock.

createRecord should not call lock/unlock on deleted record. It should only checks for duplicate record. But that is not specified in the specs.

But..according to the spec...lock on a deleted record should throw recordnotfoundexception.

It will never happen in my URLyBird application because I don't allow CSR to delete records

But It CAN happen. Remember you are writing a database server so other applications might delete the records.
Consider this scenario:
1) A CSR is looking to book a room and retrieve a list of available rooms.
2) After the CSR gets the list of rooms, the manager (using a different application) decides that one of the rooms should not be available and delete it from the database.
3) The CSR now sends in a booking request for the deleted room, i.e. The CSR is looking at stale data. The booking process might be like this:
updateRecord() -> lock (deleted) record -> update fields -> unlock

So the lock record step should throw RecordNotFoundExc because the record does not exist. As far as lockRecord() is concerned, the deleted record does not exist.

[Jar Jaquiso]

Hello Simon,

About locking and creating a record, in my specs it has no sense as the lock method takes a record ID to be locked. This means it is not a database lock, it is a record lock.

So, when creating a new record the client does NOT have to previously use the lock method, as there is still no record to be locked, thus no problem in the create method.

The lock of the deleted record in order to reuse it is internal, the client knows nothing about reused records.



Jar

[ November 06, 2006: Message edited by: Jar Jaquiso ]
[ November 06, 2006: Message edited by: Jar Jaquiso ]

[Mark Smyth]

Originally posted by Jar Jaquiso:
Hello Simon,

About locking and creating a record, in my specs it has no sense as the lock method takes a record ID to be locked. This means it is not a database lock, it is a record lock.

So, when creating a new record the client does NOT have to previously use the lock method, as there is still no record to be locked, thus no problem in the create method.

The lock of the deleted record in order to reuse it is internal, the client knows nothing about reused records.



Jar

[ November 06, 2006: Message edited by: Jar Jaquiso ]

[ November 06, 2006: Message edited by: Jar Jaquiso ]

This is the view that I have also taken, for we cannot know the number of the newly created record to until create returns the record number to us. I have a synchronized create method to handle any the possiblity of multiple clients trying to create a record in the same slot. It is also important a it create can change the size of the file. A new create operation cannot start until the previous one has one has competed. TThe update and delete method should never be allowed to write to any deleted slots (unless threre is a serious error in the code) that are eligible for reuse so there ught to be no issue on the record locking front.
[ November 06, 2006: Message edited by: Mark Smyth ]

[rinke hoekstra]

What about the case where create() wants to reuse a deleted record?

Simple: just avoid that case, and don't reuse deleted records. There is no MUST that tells you to reuse them. If you just append all newly created records, that makes life a bit simpler.