No need for a locking manager?

[Max Habibi]

/*

Originally posted by Philippe Maquet:
[
If the line is now <client-code> --> <network> --> <Data> --> <LockManager>, Data simply delegates its locking job to some smarter class to achieve it, but IMO, by doing so, it's still implementing lock/unlock. Right ?

yes . As I said before, I like your solution a lot.

BTW, my Data class also uses MetaData, Field, Index, Cache and a few other helper classes which have no other purpose than to help me to present a Data code wich is easier-to-read and easier-to-maintain.
Max
//data.lock
//synch on staticMap
//insert this and recono unto static method (if it's not already there: otherwise wait)
//exit synch block
Nice !
But I suppose that your unlock method would be written :

//data.unlock
//synch on staticMap
//remove this and recono 
//exit synch block
//[B]notifyAll()[/B]

If it's right, just two (little) remarks :

Each time a record is unlocked, all (if any) threads waiting for the lock compete to get it. It goes against performance and order (I like order )

I like order too, as all good engineers should. However, the nature of Java threading is that is likes disorder. If I may quote from my own book, p. 81.
the metaphor of children greedily trying to get ice cream is not an arbitrary one. Competing threads do not behave like polite adducts in a grocery store line. There is no sense of order. All compete voraciously, and based on the underlying operating system, priorities, and the other variables, on or another wins.

I love metaphors too : it's like if in a MacDonald, each time a Big Mac is served, all people in the line had to fight with others to get a chance to be the next happy customer. A lot of energy lost, and ... what a mess !

Didn't I use this metaphor in my book? Like McBurger example? the point here is that Java offers a mechanism to work out how threads interact. The second part of your implementation, which seems to force order, is a little concerning. It seems that You're adding complexity to address a non existent requirement that clients be treated in a FIFO order.

In comparison, for each recNo in a HashMap, the value is a LinkedList of pending lock objects. As each waiting thread is waiting on its own Lock object, in unlock() I just need to notify the next Lock in the list.

The instructions say that "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.". Well with notifyAll(), you'll consume a few CPU cycles just to compete for the lock, which is a (slight ) deviation from the requirements.

Very slight, I would say . Also, remember that notify is a bit like volatile. It should work, but sometimes, it just doesn't seem to. I wouldn't risk failing the assignment over it: not a few non-measureable cpu cycles.
All best,
M

[Rob Pearson]

Hi,
I've just been following this thread and was interested in the following:

In comparison, for each recNo in a HashMap, the value is a LinkedList of pending lock objects. As each waiting thread is waiting on its own Lock object, in unlock() I just need to notify the next Lock in the list.
The instructions say that "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.". Well with notifyAll(), you'll consume a few CPU cycles just to compete for the lock, which is a (slight ) deviation from the requirements.

Very slight, I would say . Also, remember that notify is a bit like volatile. It should work, but sometimes, it just doesn't seem to. I wouldn't risk failing the assignment over it: not a few non-measureable cpu cycles

I was also concerned about the consuming no CPU cycles requirement and was looking at something similar to Philippe's idea, but I haven't convinced myself that its actually required.
One question though, what happens if the record gets deleted while others are waiting to lock it? and then maybe re-used/re-created? On delete, would you remove that record from the hashMap? Can you actually delete an object that you are synchronized on?
Cheers
Rob

[Billy Tsai]

THere is no need for a lock manager , I didnt implement one

[Philippe Maquet]

Hi Rob,
I was also concerned about the consuming no CPU cycles requirement and was looking at something similar to Philippe's idea, but I haven't convinced myself that its actually required.
There is no real difference between having each lock() caller waiting on its own object and all lock() callers (for a given recNo) waiting on a common object. In fact, I need a Lock object mainly to store the caller's thread, the cookie and the time at which the lock was granted. So, as any caller gets such an object (which is unique to this call), it's better (but also easier) to let it wait on this object. As, by design, I am sure that the caller is the only thread waiting on it, I may use notify() in place of notifyAll() (Since Max's answer, I need to check notify(), but in the meantime ...).
Now "consuming no CPU cycles" is a requirement. But it just means that the caller must "wait" on some (any) object till the lock can be granted. Nothing more IMO. The (slight) deviation from the requirements I was talking about in my previous message was just a wink to Max.
One question though, what happens if the record gets deleted while others are waiting to lock it?
Nothing special as updateRecord and deleteRecord both throw a RecordNotFoundException.
and then maybe re-used/re-created?
Nothing special neither, if the caller re-reads the record before updating it (that's why it's good practice to do so) :

lock()
read()
if record_is_unchanged
   update()
else
   throw_some_exception_or_ignore ?
finally unlock()

Can you actually delete an object that you are synchronized on?
When you ask a lock which cannot be granted to you immediately, you just synchronize on an object before waiting. That object is not the record and ... the record can be deleted before you wake up (be notified). That's life !
Best,
Phil.

[Philippe Maquet]

Hi Billy,

THere is no need for a lock manager , I didnt implement one

It would be so great if I could be so clear and concise in my choice.txt file !
Cheers,
Phil.

[Andrew Monkhouse]

Hi Max,
I am also enjoying this discussion.

Andrew: We are talking about FBNS here. This is the one where Sun provided the Data class. So we have some limitations. The lock method should be implemented in the Data class or in a class that extends Data.
I don't believe we should be making one instance of Data class per connection.
Max: Ok, that's fine. However, there's no reason, based on the requirements, to constrain ourselves design-wise at this point. That is, this is a design constraint you're placing on yourself, not one that being placed on you by the SCJD.

This is why I specified that we (both Patrick, the topic starter, and myself) are talking about FBNS.
FBNS provides the Data class, along with the comment that it is complete with the exception of the lock, unlock, and criteriaFind methods.
If you allow for one instance of Data class per client, then you are going to have to make changes to the provided code. Particularly the create method and the variable which holds the number of records in the database.
I dont believe that these changes are in keeping with Sun's statement that these classes are complete.

Sun's requirement that the Data.unlock method know who owns the lock: that's not open to interpretation, since the javadoc comment on the data.unlock explicitly requires this.

<PedanticMode>
It is not in the JavaDoc. The JavaDoc simply states:
Unlock the requested record.
@param record The record number to lock.
The paragraph in the instructions is:
The unlock method simply removes the lock from the specified record. If an attempt is made to unlock a record that has not been locked by this connection, then no action is be taken.
</PedanticMode>
And the whole point of my argument is that I believe that this sentence is open to interpretation. I dont believe it is requiring the ownership of the lock to be tracked within the Data class, especially since doing so will require a change to the Data class or to the method signatures.

Further, your implementing makes the actual locking/ unlocking vestigial

Hmmm - my implementation has lock and unlock doing the locking and unlocking of records. This is where all clients go to attempt to gain a lock. This is where they will block if they cannot get a lock. Once the lock has been granted, then a class that is more appropriate to tracking ownership of the lock will track that it now has the lock.
Your method of tracking ownership within the Data class requires the use of "this" within the Data class. This is fine while we can assocate one "this" per connected client. But if a later requirement used a connectionless protocol (http / MQ ...) then you no longer have a "this" to associate within the Data class. My way, the Data class does not care about who owns the locks. The class that is handling the connectivity (so the servlet, or the class receiving the MQ connection) knows how to track the session, so it can worry about whether this session (connection) owns the lock and can therefore unlock it.
You also suggested a book() method server side.
However I disagree with this for FBNS. FBNS has the requirement that your client program [...] should include a class that implements the same public methods as the suncertify.db.Data class. My belief is that this is telling us that the book method for FBNS has to be client side.
I look forward to your comments.
Regards, Andrew

[Andrew Monkhouse]

Hi Patrick

The easiest thing to do would be to overload lock, allowing the second argument to indicate the owner.
lock(int){ lock(int, null);
lock(int, Object src){ // do locking; }
Is this allowed?

I also considered doing this. It might be possible, since the overridden methods would only be called by your code server side. I think that if you extend Data class, putting your extended lock and other methds into it, then you are not violating the requirements.

Additionally, i've found that a LockManager is only good for detecting dead-lock. RemoteConnections themselves can ensure that they only unlock what they have locked. I delegated all requests to lock to the LockManager, however with refactoring LockManager may be eliminated. The only advantage for LockManager (except for seperating responsibilities) is to have a localised mapping of connections and locks. This is really only useful for dead-lock recovery. Who is actually implementing this?
Anyone have any example code for detecting a dead-lock recovery?

I think we need to be clear as to what you mean by "dead-lock recovery".
Do you mean recovery from client deaths? (In which case I would prefer it if everyone started talking about dead clients - personal peeve of mine ). I think that dead client recovery can be handled at the connection as well - and this is what I did.
Or do you mean deadlock (A is waiting for a lock owned by B, B is waiting for a lock owed by A). In which case I think FBNS does not allow for it. A call to lock must block until the lock is granted - no option for throwing exceptions here. Some of the newer assignments may have different requirements that may allow for deadlock detection (I think Philippe described his requirements in such a way). And my interpretation (there's that word again ) of Kathy Sierra and Bert Bates comments when talking about the new assignment was that deadlock was extremely important (but Max's interpretation, IIRC, was that they were talking about thread deadlock not client deadlock).

When do we introduce the constraint that only connections which called the lock() can successfuly unlock it? Should it be declared in Connection? This would mean that the contract wasn't quite the same as that of the Data class?

I believe that it is handled inside the connection code. In this way it is invisible to the client, and the contract should be the same. The client is just calling connection.unlock(int). The connection then looks at the record that is being unlocked - if it is owned by this connection, then Data.unlock(int) is called. If it is not owned by the connection then the method returns without doing anything.
Regards, Andrew
[ July 23, 2003: Message edited by: Andrew Monkhouse ]

[Andrew Monkhouse]

Hi Philippe

Andrew: If you have a LockManager as a front end to the Data class (so <client-code> --> <network> --> <LockManager> --> <Data> ) then it becomes easy.
Philippe: <client-code> --> <network> --> <Data> --> <LockManager> is even easier

This can work well for UrlyBird (which you are doing) but does not fit well in the Fly By Night Services assignment that Patrick is doing.
Regards, Andrew

[Andrew Monkhouse]

Billy THere is no need for a lock manager , I didnt implement one
Philippe It would be so great if I could be so clear and concise in my choice.txt file !

I can see it now: reason for choosing sockets over RMI: because I felt like it.
Regards, Andrew
[ July 23, 2003: Message edited by: Andrew Monkhouse ]

[Patrick Cobbett]

Hi Andrew, it appears we are taking a similar approach.
The Data class provides a basic locking mechanism whereby any client can lock and unlock records.
It is the connection which must introduce the handling of multiple connections.
Therefore, although the signatures (except for thrown IOExceptions) for Data and Connection remain the same, the contract for unlock(int) adds the constraint that only the connection which locked a record may successfully unlock it.
I have a class which implements my Connection interface, called LocalConnection.
I also have an interface which extends Connection called RemoteConnection which is implemented by RemoteConnectionImpl.
Now each connection maintains a reference to a List of locked records. When it locks a record it adds the record number to it's list of locked records. However, before invoking unlock(int) on data it checks that it first owns that lock. Now the lock methods to accomplish this resided in RemoteConnectionImpl. Afterall, there are likely to be multiple Connections from clients. However, shouldn't this code also reside in LocalConnection? Even though typically LocalConnection is used alone, if it does not provide the locking mechanism imposed by Connection then the contract would not be satisfied.
Therefore, having written the lock and unlock methods for both LocalConnection and RemoteConnectionImpl, i realized that both classes are pretty much identical. Alot of code duplication.

public class LocalConnection implements Connection
public class RemoteConnectionImpl implements RemoteConnection

Would it not be logical for RemoteConnectionImpl to extend LocalConnection? I am saying that RemoteConnectionImpl is a LocalConnection which can be exported remotely. Surely any methods included with LocalConnection are associated with a RemoteConnection? Rather than being seperate responsibities, they infact share them, but with RemoteConnectionImpl implementing RemoteConnection (which is a marker interface) declaring an additional way inwhich it could be used. Afterall, RemoteConnection extends Connection, shouldn't RemoteConnectionImpl extend LocalConnection?
what is your view on this?

[Andrew Monkhouse]

Hi Patrick

Now the lock methods to accomplish this resided in RemoteConnectionImpl. Afterall, there are likely to be multiple Connections from clients. However, shouldn't this code also reside in LocalConnection?

I think it is a very good idea to have the code in LocalConnection. Especially if RemoteConnectionImpl extends LocalConnection: that way either connectivity option will have the same code in use. Less chance of something working in one mode and not in another. And, as you said, it means that your contract will be satisfied in both.

Would it not be logical for RemoteConnectionImpl to extend LocalConnection?

See above.
Regards, Andrew

[Philippe Maquet]

Hi Max,

Didn't I use this metaphor in my book? Like McBurger example?

Oops, sorry ! Where our inspiration for metaphors is coming from ? We have so many sources ... But my MacDonald one comes for sure and in direct line from your book ! Sorry again.

As I said before, I like your solution a lot.

Yes, and thank you to recall it. The only thing which worries me a bit, as that you are - in all threads which concern locking - a hard defender of all no-LockManager-solutions. Is that for simplicity ?

The second part of your implementation, which seems to force order, is a little concerning. It seems that You're adding complexity to address a non existent requirement that clients be treated in a FIFO order.

As I wrote it elsewhere, I need a Lock object mainly to store the caller's thread, the cookie and the time at which the lock was granted. So I need to store those pending Lock objects somewhere, I mean in some collection. LinkedList seems to be the one which brings the least overhead in this case, and ... I get order for free (without additional complexity IMO).

Also, remember that notify is a bit like volatile. It should work, but sometimes, it just doesn't seem to. I wouldn't risk failing the assignment over it: not a few non-measureable cpu cycles.

Since then, I wrote :

As, by design, I am sure that the caller is the only thread waiting on it (its Lock object), I may use notify() in place of notifyAll().

But I had to check what you said about notify(). In doc for class Object, I read :

Wakes up a single thread that is waiting on this object's monitor. If any threads are waiting on this object, one of them is chosen to be awakened. The choice is arbitrary and occurs at the discretion of the implementation. A thread waits on an object's monitor by calling one of the wait methods.

"one of them is chosen to be awakened" : as I know that only one thread may be waiting on the Lock object I notify(), do I take some risk ? In other words, is it possible that while a single thread is waiting on some object, notify() doesn't work at all ? May notify() calls be "eaten" sometimes ?!
"The choice is arbitrary and occurs at the discretion of the implementation" : "arbitrary" doesn't mean "optional", right ?
OK, now even if I am right (and I hope you'll confirm it), I still worry because graders are human beings and so may have different opinions :
If I use notify() :
Grader 1: that guy doesn't know that notify() is a bit like volatile. Bad point.
Grader 2: I carefully read his code and that guy knows about notify(). Slight optimization in comparison with notifyAll(). Good point.
If I use notifyAll() :
Grader 1: that guy programs in a defensive way. Good point.
Grader 2: I carefully read his code. Why does he call notifyAll() where notify() would be sufficient ? Bad point.
As in life, there are so many such dilemma in the assignment !
Best,
Phil.

[Max Habibi]

Originally posted by Philippe Maquet:
Hi Max,

Didn't I use this metaphor in my book? Like McBurger example?

Oops, sorry ! Where our inspiration for metaphors is coming from ? We have so many sources ... But my MacDonald one comes for sure and in direct line from your book ! Sorry again.

I love being quoted to myself: Do more! I just wanted to point out that I understand what you're talking about.

As I said before, I like your solution a lot.

Yes, and thank you to recall it. The only thing which worries me a bit, as that you are - in all threads which concern locking - a hard defender of all no-LockManager-solutions. Is that for simplicity ?

Yes, and only that. I think LockMangers work, but just think they're overkill.

[/qb]
The second part of your implementation, which seems to force order, is a little concerning. It seems that You're adding complexity to address a non existent requirement that clients be treated in a FIFO order.
[/qb]
As I wrote it elsewhere, I need a Lock object mainly to store the caller's thread, the cookie and the time at which the lock was granted.

Are you still doing this? This is a serious issue that could cause you to fail, and I would hate for you to do so.
You are not guaranteed that the calling threads will be consistant for a given object. As a matter o fact, I know that they are not. I know you're thinking of documenting this, because there's a small opportunity for it to occur. However, you might as well not implement locking and document that fact. If your grader is paying attention, they'll be obligated to fail you here, because this just isn't the way RMI works, and you're not deomonstrating mastery.
M
[ July 24, 2003: Message edited by: Max Habibi ]

[Philippe Maquet]

Hi Max,
What you wrote here is terrible !

Are you still doing this? This is a serious issue that could cause you to fail, and I would hate for you to do so.
You are not guaranteed that the calling threads will be consistant for a given object. As a matter o fact, I know that they are not. I know you're thinking of documenting this, because there's a small opportunity for it to occur. However, you might as well not implement locking and document that fact. If your grader is paying attention, they'll be obligated to fail you here, because this just isn't the way RMI works, and you're not deomonstrating mastery.

I would also hate myself if I had to fail (for this or anything else).
The only time (if I can recall) someone told me about that issue is when Andrew wrote this :

Also, I am unsure about your looking at thread death as a way of determining if the client has died. I guess it depends on what thread you are using. Just using the thread running your remote class is not really valid though, as there is no guarantee that two calls to remote methods from the one client will run in the same thread.

I replied :

I use sockets and make sure that a client connection keeps the same thread. But of course, I will document that locks cannot spread over multiple threads, and explain why.

Well, in the meantime, I intend to allocate threads to connections by using a pool of threads. It means that my sockets implementation, as RMI, will not give any garantee that a given connection will execute in a given same thread.
BUT:
In my assignment the whole DBAccess interface doesn't need to be accessible from client-side. And as I find a better practice to handle locks server-side, it means that no client will never be able to execute any lock/unlock command directly.
Let's say that I have a book command which receives a customer ID and an array of recNos as parameters. It's pseudocode would be :

try 
  lock_the_records
  read_them_and_check_they_are_still_bookable
  if ok
     update_them_with_customerID
finally
  unlock_the_records

As a given method will always execute as a whole in one thread, it's seems to be OK, even in RMI. Is it right or not ??!
Best,
Phil.
[ July 24, 2003: Message edited by: Philippe Maquet ]

[Andrew Monkhouse]

Hi Philippe
I think Max was thinking that you allowed the clients to call the lock method, and that you were using RMI.
Since neither is true, I dont believe his comments apply to your situation.
Looking at what you have described I think you will be safe in your implementation.
Regards, Andrew

[Patrick Cobbett]

Hi Andrew,
I am curious about the contract for unlock(int) as declared in Connection. unlock(int) should unlock the entire database right?.. or should it just unlock the records owned by this connection.
When -1 was supplied to unlock(), my implementation simply recursively calls unlock() on all the records.
I have read implementation where connections make use of a boolean flag indicating whether that connection owns the full lock. For me this is implicit. lock(-1) recursively calls lock(int) on all the records. Other clients may freely attempt to lock or unlock any records during a full lock operation because a full lock is transparant to the Data class. The connection calls lock on each record, if a given record is locked the it blocks until it can lock it.
My question is this. Does the contract indicate that when unlock(-1) is called that unlock() should be called on all the records OR that the database should be fully unlocked as a postcondition of that call? If it is the first one, then this is the same as a connection unlocking all it's locked records right?
My worry is that you'll say that the second explaination is the case. Although i do not think it is, perhaps you can see the ambiguity.
As a result of calling lock(-1) the database is fully locked.
However, as a result of calling unlock(-1) the database is not necessarily fully unlocked.
However, lock and unlock are consistant in that they both recursively call themselves when -1 is passed as an argument.
What do you think?

[Andrew Monkhouse]

Hi Patrick
The documenation doesn't specify when (or even if) the a lock on the entire database can be released. Like you, I chose to allow the lock to be released on a call to unlock() with a parameter of -1.
I cannot see any reason why you would want to specify that the entire database should be unlocked as a post condition to calling unlock(-1). Because you have unlocked the database, you are implying that it is now available for others to use. You cannot really complain if others then start to use it.
So in your case, I would continue with what you have done: just unlock all the records that your connection owns (which should be all of them for a database lock). Don't worry if someone else locks a record that you have just unlocked.
Regards, Andrew

[Patrick Cobbett]

Hi Andrew, i had a look at your table of contents for your design decision document that you referred me to. I notice that you did actually implement a deadlock prevention mechanism. I'm curious as to how you implemented this seeing as you didn't make use of a LockManager. In one post i discussed that in my view the only advantage of a LockManager was to localise the mapping of connections to locks, enabling dead-lock prevention. Since i decided to forget deadlock prevention (on the grounds that many people did well without implementing it and being lazy), there was no need for the LockManager as each Connection can itself ensure that it does not unlock records it has not locked.
For deadlock prevention you'd need a centralized class which knows who owns what locks. Unless you've used a static data structure in a Connection implementation, or indeed a LockManager, i can't think how you've implemented it.
come on spill the beans

[Andrew Monkhouse]

Hi Patrick
My heading is misleading.
My deadlock prevention was: dont allow a client to lock more than one record
Regards, Andrew

[Philippe Maquet]

Hi Andrew,
Thank you for trying to reassure me. I just spent a restless night because of
that.

I think Max was thinking that you allowed the clients to call the lock method,
and that you were using RMI.

To me, Max is like God for a believer : you believe in him, you know HE exists,
you know HE is smart (I've read his book), but his ultimate thoughts seem
sometimes impenetrable to the common human being I am
In my mouth, it's no critic, just a compliment, Max

Since neither is true, I dont believe his comments apply to your situation.

That's what I think also.

Looking at what you have described I think you will be safe in your
implementation.

Well, I am now unsure about it. My design relies upon a big tradeoff which could be perceived as not acceptable by the grader.
Andrew, I'll probably be boring you by summarizing here the whole reasoning that lead me to design LockManager as I did, but if you keep with me here, I promise you a better solution at the end of this message.
In URLyBird 1.2.1, the basic requirements for locking are given in DBAccess interface Data must implement :

// Locks a record so that it can only be updated or deleted by this client.
// Returned value is a cookie that must be used when the record is unlocked,
// updated, or deleted. If the specified record is already locked by a different
// client, the current thread gives up the CPU and consumes no CPU cycles until
// the record is unlocked.
public long lockRecord(long recNo)
throws RecordNotFoundException;
// Releases the lock on a record. Cookie must be the cookie
// returned when the record was locked; otherwise throws SecurityException.
public void unlock(long recNo, long cookie)
throws SecurityException;

Upon those, I added my own requirements :

automatic deadlock-by-concurrency prevention (even if multiple tables are concerned)

automatic deadlock-by-crash repair

automatic unlock on timeout

with no share of responsabilities among multiple classes

I still think that those additional requirements are defendable.
My next step in the reasoning was to say : it's easy to do as far as I know who owns the locks.
As I cannot change the methods signatures (to add some Object connection parameter), I decided to use Thread.currentThread() as a lock owner.
I liked that solution because the whole locking stuff was centralized in one class whose code is not that much complex.
Here comes the tradeoff : the locks granted by my LockManager class cannot spawn multiple threads, which in turn means that they cannot spawn multiple methods server-side (because of the way RMI - and probably my future sockets implementation - allocate threads to connections).
Was that tradeoff acceptable ? I thought it was because :

my assignment does not require that DBAccess is accessible client-side, so locking may be "hidden" to the client ;

dealing with locks server-side seem to me more reliable and more efficient because I spare network connections and benefit from the try/finally construct

Now, even I am not sure that I understood what Max actually meant, I think he is right (yes I know that what I just wrote is stupid ), because some grader could mark me as follows :
Locking: Maximum=80 Deductions=40 (implementation is incompatible with client-side locks - only half the job is done).
It would be quite crual and harsh, but as I don't know him ...
So I just found out a new way to break that dilemma, and I think that new solution is the good one (so, and if it is, thank you Max for having driven me into a corner ). Besides, it does not need too much refactoring :
Let's say that I :

Rename my current "Data" class in "Database" and keep it for the most part as it is now (a multiton, allowing only one instance per database file)

Change the lock and unlock signatures in that new Database class to accept one more parameter (Object connection) which will be transmitted to LockManager

Create a new Data class which respects all DBAccess requirements, but whose constructor has this form : public Data(String fileName, Object connection). I may do that because there is nothing written in the instructions about the Data constructor. Data just need to store connection as a private strong reference and get a private Database reference. In its lock/unlock methods, Data may pass connection to the Database lock/unlock methods from inside its own lock/unlock methods. For all other methods, it will simply delegate the job to Database (same signatures).

Finally, LockManager must be refactored to :

accept Object connection in its own lock/unlock methods

store them as WeakReferences and implement a ReferenceQueue

use connection in place of thread to identify a lock owner

in my MaintenanceThread, replace my isAlive() test by a poll() on the ReferenceQueue

That's all and with no tradeoff.
What do you think about it ? Am I right ? (I'll wait a little before refactoring ).
Thank you in advance for your comments,
Best,
Phil.

[S Bala]

If we do use a LockManager, is it better to use it as a private inner class to the data class, so that only Data class has access to it. This way the locks will be secure.
Any comments?

[Patrick Cobbett]

S Bala,

If we do use a LockManager, is it better to use it as a private inner class to the data class, so that only Data class has access to it. This way the locks will be secure.
Any comments?

Sure, you could do this. Although making it a private member of Data has the same effect. It is better to use static member classes if possible rather than member class's. Your LockManager should not need to access any of Data's method or Data. Let me know which assignment you are doing as where the lock manager should resides depends on the requirements in the assignment.

[Vlad Rabkin]

Hi Patrick,
[Patrick] ... lock manager should resides depends on the requirements in the assignment
Exactly! In URLBird LockManager has to access Data methods (to throw RecordNotFoundException if a record was not found). That is why I decided not to have separate class for LockManager...
Vlad

[Philippe Maquet]

Hi Vlad,

In URLBird LockManager has to access Data methods (to throw RecordNotFoundException if a record was not found). That is why I decided not to have separate class for LockManager...

A LockManager class doesn't need to access Data. As a matter of fact, the check to see is RecordNotFoundException must be thrown must be done after the lock is granted, to make sure the record was not deleted while you are waiting for the lock.
The code gives something like this :

public long lockRecord(long recNo) {
  long cookie = lockManager.lock(recNo);
  if ( !recordExists(recNo)) {
    lockManager.unlock(recNo, cookie);
    throw new RecordNotFoundException();
  }
}

For the rest, I agree with Patrick too, except that package level for LockManager is right too, IMO.
Best,
Phil.

[S Bala]

I am in the process of designing URLyBird.
I am thinking of using a LockManager which has a private HashMap and a boolean flag and the following methods.
1) public syncr.. boolean isRecordAvailable(recno, cooie)
2) public sync.. long lockRecord
3) public sync.. unlockRecord
And in the lock() method of the Data class I can do..
sync(LockManager)
...
while
{
wait()
}
**********************
This way the Lock manager does not know anything about the Data class. Just keeps a collection of locked records.
Sb
[ July 25, 2003: Message edited by: S Bala ]

[S Bala]

I would appreciate if anybody can comment on the LockManager design. Does it look ok?

[Philippe Maquet]

Hi S,
I have a few questions before I may try to give you an opinion :

What do you put in your HashMap (key / value) ?

What does isRecordAvailable() ?

Why don't you wait in LockManager.lock() method ?

I come back to you after your reply.
Regards,
Phil.

[S Bala]

Hi Phil,
1) I am going to use, recordNumber, cookie key value pair in the HashMap.
2) I missed out one of the recordAvailable methods.. There are 2 overloaded methods.
one takes recordnumber and the other takes record number and cookie. The first one is used while locking the cookie, to make sure the record is not locked. The second one while unlocking.

3) As for waiting goes, it could be either place. Thought it would be easy to understand, if it is in the lock method. (Like one has to take turns to get into lock manager).
thanks.

[Philippe Maquet]

Hi S,
By designing a LockManager class, you decided to delegate lock/unlock to it. So I think it's better in Data just calling the lock/unlock methods of LockManager, putting there the wait() and any call to one of your isRecordAvailable methods.
For the one who reads your Data class, it's simpler to have just one line in your lock/unlock methods, telling him "OK, LockManager performs the whole job".
Now I think about questions you may ask yourself : Is my database system multi-table ? Or may it become multi-table in the future ? If you reply YES to one of them, then your LockManager design could need to take it into account.
Best,
Phil.

[S Bala]

Phil,
Thanks for your help. I just went through the
postings again. I might not have answered your 2nd question fully.
isRecordAvailable? what does it do?
I am assuming that, if an entry is in theHashMap then the record is locked. While unlocking I will take them out. So isRecordAvailable will check to see if the record is in the HasMap.
Thanks..

[Vlad Rabkin]

Hi Phil,

A LockManager class doesn't need to access Data. As a matter of fact, the check to see is RecordNotFoundException must be thrown must be done after the lock is granted, to make sure the record was not deleted while you are waiting for the lock.

I like this idea. I have done it first like you,
but than I though following:
1) Should we try first to lock the record which even didn't exist before we have tried it?
2) We first have to lock and then unlock.
I am doing now following:
a) go in queue (if necessary wait()) to get a lock
b) as soon as lock permitted check if the record exists (otherwise RecordNotFoundException)
c) acquire lock
In this way I don't need to unlock (if the record not found).
But you are right, you can avoid accessing Data directly by doing like you have done.
By the way, I use the same locking functionally for local Data and remote Data. Specification says locking for local mode is not required. Do you think it Ok if we have anyway all the locking logic in local Data?
Vlad

[Andrew Monkhouse]

Hi Philippe,
Sorry for not answering sooner - I had finally gone off to sleep

Max .... ultimate thoughts seem sometimes impenetrable to the common human being I am

]
I know what you mean - I really enjoy reading Max's posts (probably shouldnt say that or he will start blushing again). The few times when I cant quite work out what he is getting at, I suspect that Max is being deliberately obscure to try and get us to think things out for ourselves. I also find your posts are usually well thought out, with obvious thought going into not only current stated needs but also unstated needs, and future needs.

So I just found out a new way to break that dilemma, and I think that new solution is the good one (so, and if it is, thank you Max for having driven me into a corner ). Besides, it does not need too much refactoring :
Let's say that I :

Rename my current "Data" class in "Database" and keep it for the most part as it is now (a multiton, allowing only one instance per database file)

Change the lock and unlock signatures in that new Database class to accept one more parameter (Object connection) which will be transmitted to LockManager

Create a new Data class which respects all DBAccess requirements, but whose constructor has this form : public Data(String fileName, Object connection). I may do that because there is nothing written in the instructions about the Data constructor. Data just need to store connection as a private strong reference and get a private Database reference. In its lock/unlock methods, Data may pass connection to the Database lock/unlock methods from inside its own lock/unlock methods. For all other methods, it will simply delegate the job to Database (same signatures).

Finally, LockManager must be refactored to :

[/list=1]

accept Object connection in its own lock/unlock methods

store them as WeakReferences and implement a ReferenceQueue

use connection in place of thread to identify a lock owner

in my MaintenanceThread, replace my isAlive() test by a poll() on the ReferenceQueue

[/list]
That's all and with no tradeoff.
What do you think about it ? Am I right ? (I'll wait a little before refactoring )

You did not state it explicitly, but this means that you have one instance of Data class per connection, with the IO being handled by the single instance of the Database class. Correct?
If so, then I like this solution.
I made mention a couple of times that this was one way that I could see of having the Data class aware of who the owner of the lock was. I did not spell it out since we were originally talking about the Fly By Night Services assignment, and I cannot see how to make this solution work with the code that gets provided in that assignment. But for the Hotel or Contractors assignments, this should work very well.
My only question is whether you need to pass a "Connection" object to the Data class. If there is one instance of the Data class for each connection, then can the instance of the Data class itself act as your identifier to the lock methods?
Regards, Andrew.

[Philippe Maquet]

Hi Andrew,
Thank you for your reply. Phew !

You did not state it explicitly, but this means that you have one instance of Data class per connection, with the IO being handled by the single instance of the Database class. Correct?

Yes !

If so, then I like this solution.

I made mention a couple of times that this was one way that I could see of having the Data class aware of who the owner of the lock was. I did not spell it out since we were originally talking about the Fly By Night Services assignment, and I cannot see how to make this solution work with the code that gets provided in that assignment. But for the Hotel or Contractors assignments, this should work very well.

I am so happy not to have got FBNS !

My only question is whether you need to pass a "Connection" object to the Data class.
If there is one instance of the Data class for each connection, then can the instance of the Data class itself act as your identifier to the lock methods?

No because Data itself represents a connection on one table, while LockManager must be able to handle deadlocks spanned on multiple tables.
Cheers,
Phil.

[Philippe Maquet]

Hi Vlad,

By the way, I use the same locking functionally for local Data and remote Data. Specification says locking for local mode is not required. Do you think it Ok if we have anyway all the locking logic in local Data?

Yes, and I'll do the same as you. Your code will be easier to test and later maintain.
Phil.

[Vlad Rabkin]

Hi Phil,
Thanx.

[Max Habibi]

Originally posted by Philippe Maquet:
Hi S,
By designing a LockManager class, you decided to delegate lock/unlock to it. So I think it's better in Data just calling the lock/unlock methods of LockManager, putting there the wait() and any call to one of your isRecordAvailable methods.
For the one who reads your Data class, it's simpler to have just one line in your lock/unlock methods, telling him "OK, LockManager performs the whole job".
Now I think about questions you may ask yourself : Is my database system multi-table ? Or may it become multi-table in the future ? If you reply YES to one of them, then your LockManager design could need to take it into account.
Best,
Phil.

Hi Phil,
This all looks reasonable. I like your alternative.
M

[Philippe Maquet]

Hi Max,
Thank you for replying, but seeing the quote I wonder if we talk about the same posts. The ones I was refering to are those dated July 24, 2003 04:05 PM and July 25, 2003 09:31 AM (the solution to the big issue you pointed out). Are we on the same track ?
Best,
Phil.