• 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

URLyBird Locking

 
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 all,
I have a new idea for the cached version of URLyBird.
Since all records are stored in a List we can add a property to the Record class to store cookie (long cookie)
Record class would have then two additional methods:
boolean isLocked(int recNo), long getLockCookie(int recNo)
In this case I don't need an additional Map object to store Locks.
What do you think about this idea?
Tx,
Vlad
P.S. Max, Andrew, Jim, I had problems publishing this topic. As I changed a fragment of my message from "getXookie(.." (instead of X-> C, otherwise I can publish it) to "getLockCookie(.. " I managed to publish it. It seems javaranch has problem accepting text which includes "getXookie" ...
[ September 09, 2003: Message edited by: Vlad Rabkin ]
 
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I saw a few threads regarding caching the database in here, and I am curious how you guys implemented it. In the app that I am working on right now, we cach the database data heavily, but only from the static tables where data changes very infrequently. Since in FBN (and in the other versions of the developer assignment, I assume) the data in the table is very dynamic (reservations are made all the time), how do you deal with the problem of refreshing the cache?
I mean, if you cache the record and at some point in time it has changed, the cache must reflect it. It seems to me that the effectiveness of the cache will be very low, as there is a very real possibility that refreshing the cache may take longer time than accessing the data directly from the database. In other words, the cache "hit" rate may be very low.
The other aspect of it is the prescription from Sun (at least in my version of the assignment) that the performance is of secondary consideration, as long as the standard solutions are utilized, and the simplicity and clarity are the primary considerations. Given that, caching the database may be a good target for a grader's scepticim.
[ September 09, 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 Eugene,
Every time a caller update database, cache will ne refreshed accordinately.
Having a cache doesn't add any complicity. Of course it has merits and problems. The most common operation will be probably find(). Cache will will greatly impact performce.
There were people who implemented a cache, who succesfully passed the test.
Regards,
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
Having a cache doesn't add any complicity.
I'll be the first one to defend an additional few lines of code to make it more reusable, clear, and expressive. But if you do it for the sake of performance, this may be a dangerous game. Does your cache have a size limit? If not, how much extra memory will you require if your database table expands to contain 10 mil records? Is there a chance that you may be introducing a memory leak (caches are often the sources of the leaks)? What happens if there is a batch job to update the database, with a lot of write() and very few read() operations after the entire database is locked, -- do you see how the cache may deteriorate the performance of your app?
This is not to say that you shouldn't cache, just that you shouldn't cache in the scope of this assignment.
 
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 Eugene,

if your database table expands to contain 10 mil records


I will switch to Oracle, DB2 ot at least MySQL

if there is a batch job to update the database


We can assume that only our GUI clients access the database.
I don't want to convince you that caching is the perfect solution.
It has own disadvantages the same as not using a cache.
I am 100% sure it DOESN't matter which design to apply (cached db or not). Both solutions are acceptable.
Major merits of cache:
- Performance, since I expect more reads then writes (it is my assumtion which I will document)
- simplicity of such methods like find or read
Major problems of cache:
- memory lack if too many records in database
Major mertis of non-cached DB:
- Simplicity (but not much)
- independance from record number in db
Major problems of non-cached DB:
- performance, expecially when too many records in database.
So, both solutions will have problems while having too many record in database.
You are right: performance is not an issue, but I still like cached db more and that is the way I would do it in a real application. Since I managed to cache db without adding a complexity, I preffered this solution.
Best,
Vlad
 
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
[Vlad]: What do you think about this idea?
I like it; that's what I do. Actually I arrived there from the opposite direction. I didn't cache at all initially, but I wanted record-level locking. This led me to create an ArrayList of RecordLock objects, one per record, containing cookie info. (Yeah, I could've used a Map, but to me it seemed simpler to let all the RecordLocks persist, even when not in use.) I got this working well. For convenience I added a boolean isDeleted field to the RecordLock. Then it occurred to me that it would be quite easy to store the String[] of record fields inside each RecordLock too. I think I changed about three lines, with no additional complexity, and suddenly I had complete caching. Performance improved substantially; added complexity was zero, at that point. Of course I also had to change the name RecordLock to Record, since the class' responsibilities had grown. In contrast, partial caching would have been somewhat more complex to implement, and with more dubious results, I believe. But implementing complete caching is pretty simple. OK, there would've been more to it if I hadn't already had a data structure to put everything in, but that's still not hard.
So, both solutions will have problems while having too many record in database.
Exactly why I decided not to worry any more about the possible OutOfMemoryError. For the number of records it would take for this to be an issue, you're going to have a BIG data file, and reading it will be SLOW. Memory is cheap; a faster precessor is not. If you've got a HUGE file, it's well worth the company's money to buy a little extra RAM so you can still keep all records in memory - otherwise, reading them for every find() is going to be painfully slow.
P.S. Max, Andrew, Jim, I had problems publishing this topic. As I changed a fragment of my message from "getXookie(.." (instead of X-> C, otherwise I can publish it) to "getLockCookie(.. " I managed to publish it. It seems javaranch has problem accepting text which includes "getXookie" ...
Yeah, the site has some guards against possible JavaScript exploits inserted into posts; this is one of them. Sorry about that. The software came with this "feature"; it comes up seldom enough that it hasn't been worthwhile to find a more elegant implementation.
 
Ranch Hand
Posts: 36
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I am interested in this topic. I am at the point where I have the data class written fully, public methods synchronised, and using cached record objects. I am intending on having a single data access object for all clients. The record level lock sounds like a good approach.
I am confused how each client will lock a record and hold or be passed back a lock cookie, or not. Could a clients threadID be used in the record objects lock
I intended on implementing DBAccess in a DataAccess class with each Data public method surrounded by lock/unlock. But what would I do with the long return from lock(). A call to a DBAccess implemented method should only return as specified by the interface.
Any ideas on this?, I realize the above could be hard to understand.
Regards
Hugh
 
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 Hugh,
It sounds like you have to return a long from your lock method. So what you need to do is make sure that the long value is reasonably unique (one way is via a Random number). Then you associate that long value with the record that is locked. This long is your cookie value.
When the client tries to call any method that requires the lock (so update, delete, or unlock) then they must pass the same cookie value as a parameter to the method. If the cookie value you receive is not the same as the one you have associated with the lock, then you throw the exception specified (if one is specified in Sun's interface).
You asked about using the thread id as a way of identifying the client. This does not apply to you since you have to use the cookies. But just for completeness - if you did not have to use cookies, then using the thread id might be OK if you are using sockets as your network layer, and you are in complete control of the creation and assignment of threads. If you are using RMI then you could not use the thread id, as there are no guarantees what thread you will use for different method invocations.
Regards, Andrew
 
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 Jim,
Tx for feedback!

I like it; that's what I do.


Waw, I am glad to hear that!
Tx,
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 Jim,
One question reagarding lock method:
In case a user invokes a lock method on the currently non-existant record there are some dangerous points.
I think I should the following in the lock method:
1. check if the record is not ouf range (>0 and < list.size())
2. acquire the lock
3. check if the record is still in db : isDeleted == false (if not unlock and throw exception)
Theoretically I could skip line 1, but if the recordnumber is out of range I would get IndexOutOfBoundsException in line 2. trying to retrieve an entry from the List (containing records).
Is it Ok to check record existance 2 times in a lock method?
If not, do you have better ideas how to handle it?
Tx,
Vlad
[ September 10, 2003: Message edited by: Vlad Rabkin ]
[ September 10, 2003: Message edited by: Vlad Rabkin ]
[ September 10, 2003: Message edited by: Vlad Rabkin ]
[ September 10, 2003: Message edited by: Vlad Rabkin ]
 
Jim Yingst
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Vlad. Yes, that's what I do. My requirements state:
Any methods that throw RecordNotFoundException should do so if a specified record does not exist or is marked as deleted in the database file.
So even for a completely non-cached file-only solution, we'd need two different checks:
(a) is the file big enough to contain this record number?
(b) has the record for this file been marked deleted?
These correspond to 1 and 3 in your post. The only difference is that in my solution step 2 is inserted, since we can't possible acquire a sync lock on an out-of-range record, but we need a sync lock for any operations on an existing record, if we want them thread-safe.
For other solutions, is there some way of avoiding having two types of existence checks in a lock method? Or do some people have different requirements here?
 
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 Jim,

Is it what you mean? At least it the way I have done it.

In this case I don't need Line 1, because you can afford just create a new Lock without having actually a record in the database. That was my previous solution.
Best,
Vlad.
[ September 10, 2003: Message edited by: Vlad Rabkin ]
[ September 10, 2003: Message edited by: Vlad Rabkin ]
 
Jim Yingst
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Is it what you mean? At least it the way I have done it.
Close. My lockCookie is a long; it can't be null. I actually also have a boolean isLocked field I forgot to mention. Alternately I considered using a cookie value of 0 to indicate unlocked, and then modify the random cookie generator to ensure it never returned 0 accidentally. But I decided a separate field would be more readable.
More importantly, I use record-level locking , which means you can't sync until after the cache.getRecord(). (Well, that's synced internally on the cache, but that's another matter.) So it's more like:

So the range check is actually performed by the cache itself, and the isDeleted check is performed separately.
Actually it's more like

And inside Record:
 
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 Jim,

I use record-level locking , which means you can't sync until after the cache.getRecord().


Yes. I know. I remember our discussion about synchronization designs.
Ok,I understand now what you ment by "inserted": it is inside Record class.
I have two questions to you:
1. Why do you call record.notify(); in lock() method?

2.
I had today the same idea to hide throwing recordNotFoundException in Record class(check if not deleted) and in Cache(check for range) class accordinatelly.
The point which prevented me to this is following:
- Record Class and Cache class are some kind of gfeneric classes, which should not take care of RecordNotFoundException. It should responsibility of Data class. Ok, I know, this is arguable.
- Doing like you did means you have a lot of places where RecordNotFoundException is thrown separately (Cache.get(), Record.lock(), Record.unlock() and so on) If let's say you want to change the message or the way RecordNotFoundException is to be thrown, you would have to do it overall.
I have only 2 methods checkRecordIsNotDeleted(), chechRecordExists() in Data class, which are called by many methods. If I want to change something, I will have to do it only inside these two methods.
What would you say to argue my points?
Best,
Vlad
[ September 10, 2003: Message edited by: Vlad Rabkin ]
[ September 10, 2003: Message edited by: Vlad Rabkin ]
 
Jim Yingst
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
1) Record Class and Cache class are some kind of gfeneric classes, which should not take care of RecordNotFoundException. It should responsibility of Data class. Ok, I know, this is arguable.
Well, I suppose you could decouple these by letting Record and Cache throw different exceptions, and then let Data catch these and rewrap them as RecordNotFoundException. I don't really see the point though. Just from the name, RecordNotFoundException sounds like something Record should be able to throw. As for Cache - well, unless you're making a Cache which is really generic and is for Objects rather than Records, well, again, RecordNotFound seems perfectly appropriate. If you are making a generic Cache, maybe it throws an ObjectNotFoundException and Data rewraps it as RecordNotFoundException. Or maybe it just returns a null, and Data checks for this. Whatever.
2) Doing like you did means you have a lot of places where RecordNotFoundException is thrown separately (Cache.get(), Record.lock(), Record.unlock() and so on) If let's say you want to change the message or the way RecordNotFoundException is to be thrown, you would have to do it overall.
I have only 2 methods checkRecordIsNotDeleted(), chechRecordExists() in Data class, which are called by many methods. If I want to change something, I will have to do it only inside these two methods.

Well I only need two points as well - one method inside the cache (for out-of-range check), and one inside Record (for isDeleted check). I chose to have more because I liked having more customized error messages. But it would be easy to refactor this. As for the InterruptedException, that's a special case - I could just aas well convert that to a RuntimeException, or maybe AssertionError. Currently this should never happen anyway. But if someone implements a timeout later, calling interrupt() on the thread might be part of that. Who knows. What to do about an interrupt is a separate discussion I think; it has little to do with whether the record "really" exists or not.
 
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 Jim,
What I like in software development is that the most important point is not the design choise, but it argumentation.
I had very simular idea (except record locking) as you represented here, but I thought it was not nice. Now you managed to convince me that it is even better as to do it in Data class
One more question, which forgot to ask you:
Why do you call record.notify(); in lock() method?
Tx,
Vlad
[ September 10, 2003: Message edited by: Vlad Rabkin ]
 
Jim Yingst
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Why do you call record.notify(); in lock() method?
Oops - that was a mistake. I didn't have my actual source code on hand to cut & paste; I did it from memory, and remembered the notify() from unlock, incorrectly placed here in lock(). D'oh! Well, ignore that. Glad to see you were paying attention.
Cheers...
 
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 All,
So, I have implemented the solution discussed above with a major difference to Jim that I do synchronize on the whole Database(not record) and I don't hide locking logic inside Record class.
Data is Singleton:

The Record class:

Can somebody comment it? Does it look Ok, or separating List of Record and HashMap for locks is simpler/better ???
Tx,
Vlad
 
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Vlad and Jim,
Given the fact that you decided to cache all records, I like your idea to couple cache and locking, a lock claimer wating on a Record instead of some locks container (HashMap or any other collection).
Vlad, I have just two questions/comments about your DBRecord class :
  • Why is your DBRecord deleted_ by default ? It seems a little weird to me at first thought, the "typical" record being not deleted.
  • As the delete operation cannot be reversed, setDeleted(boolean deleted) could set data_ to null if deleted is true.


  • Best,
    Phil.
     
    town drunk
    ( and author)
    Posts: 4118
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Originally posted by Vlad Rabkin:
    Hi All,
    So, I have implemented the solution discussed above with a major difference to Jim that I do synchronize on the whole Database(not record) and I don't hide locking logic inside Record class.
    Data is Singleton:

    The Record class:

    Can somebody comment it? Does it look Ok, or separating List of Record and HashMap for locks is simpler/better ???
    Tx,
    Vlad


    Hi Vlad,
    I like locking on the database better then locking on individual records.
    M
     
    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 Max and Phil,
    Thanx a lot for your replies! I have been waiting for them!
    Phil:

    Why is your DBRecord deleted_ by default ? It seems a little weird to me at first thought, the "typical" record being not deleted.


    That's right I have a constrctor yet and I have done it just to test some things. Of course I will throw it away.

    As the delete operation cannot be reversed, setDeleted(boolean deleted) could set data_ to null if deleted is true.


    Very good point. I guess Jim make it also. I will do it. Tx for the idea!
    Max:


    I like locking on the database better then locking on individual records..


    Max, that is not what I was asking. The question was whether it makes sense
    to store Record objects, containing cookie (indicating a lock) in one ArrayList without having separate HasMap for Locks or it is better to separate it in ArrayList for cached Record and HashMap for locks.
    1) ArrayList containing Record (String[] data, boolean isDeleted, long cookie, boolean isLocked)
    2) ArrayList containing Record (String[] data, boolean isDeleted) + HashMap containing (Integer recNo, Long cookie).
    Max, please, I want to know your opinion!

    Tx,
    Vlad
     
    Philippe Maquet
    Bartender
    Posts: 1872
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Hi Max,
    Phil:
    Given the fact that you decided to cache all records, I like your idea to couple cache and locking, a lock claimer wating on a Record instead of some locks container (HashMap or any other collection).

    Max:
    I like locking on the database better then locking on individual records.

    It's quite difficult to reply to such a sentence : no arguments to argue against...
    I like Vlad and Jim's solution because it is simple and efficient, and perfectly consistent with their main design choices (which I don't share BTW).
    Before you throw some InputBufferOverflowException and enter in some debate on semantics, I'll try to be as clear as I can be (in english) :
  • As you, they agree on this : database == one table
  • They decided to cache the database in memory
  • Unlike me, they decided to cache the whole database in memory
  • In such a way that for them, database == cache
  • From the record "point of view" (ok, it's just a metaphor, please don't debate on this), locking comes down to this : to be locked or not to be.
  • On one hand they have a memory structure which holds all records, and on the other hand a boolean status relating to the same records that they have to store somewhere
  • As far as I know (and understood), their cache offers the fastest access possible : an ArrayList of records where recNos == index in the list (by comparison, my cache is based on a Map, whose access is slower by nature)
  • Given all that, they just had to choose between two solutions as far as locking is concerned : 1) just flag their records as locked or unlocked (simple and efficient) or 2) add some container (probably some Map) just for that purpose (less simple and less efficient).
  • They chose solution 1) (the simplest and the most efficient given their db design), with this additional benefit ... for free : lock claimers wait on records instead of on some locks container (more efficient too because there is less average competition between callers)


  • Now that you know why I like Vlad and Jim's solution, please defend your "I like locking on the database better then locking on individual records".
    Mmh. You might surrender immediately, but I know that Jim wouldn't believe it. Not so fast anyway...
    Best,
    Phil.
    [ September 11, 2003: Message edited by: Philippe Maquet ]
     
    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 Phil,

    Tx for this clear explanation. I will seriously take some sentences for my choices.txt!
    I think there is only one misunderstanding there:

    Now that you know why I like Vlad and Jim's solution, please defend your "I like locking on the database better then locking on individual records".


    Max probably ment not locking, but synchronization.
    I do synchronize (read/write) on whole database, but I do lock on a single record.
    Jim does synchronize on a single record and lock on syngle record (he synchronized on the database only while retreiving record from cache)
    I and Jim both contain only one container ArrayList instead of having ArrayList for records and HashMap for locks. That's right. But
    I DO synchronize (not lock) on the whole database:
    public SYNCHRONIZE lock (...) {...}
    and Jim synchronize on the Record.

    So, understand Max that he preferres my choise, but the point we are discussing is not that one, rather than:
    Whether it makes sense to hold Record objects (with lock property) in one ArrayList (or keep Cache(ArrayList) containing Record objects(without lock property) and Lock object in separate(HashMap)?
    In Jims design - yes. And you agree with it.
    In my design? I don't know... Do your arguments in previous mail apply for my design?

    By the way, one more question to you: is it really much faster: ArrayList.get(recNo), than HashMap.get(new Integer(recNo))?
    Tx Phil,
    Vlad
    [ September 12, 2003: Message edited by: Vlad Rabkin ]
     
    Ranch Hand
    Posts: 156
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Great! This thread is great!
    I have a problem: public synchronized long lock(int recNo)
    Why it return a long value, but in my DBMain interface, this method doesn't return anything.(public void 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 Leo,
    Hi Leo,
    Sun provided different version of interfaces.
    You have to make a bit different concept of locking. In my case it is a cookie, which identify the client owning the lock. In your case you have to create your own mechanism to keep track on the lock owner. There many threads discussing possibility to do it (e.g. your connection object can be considered as a lock owner).

    Best,
    Vlad
     
    Philippe Maquet
    Bartender
    Posts: 1872
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Hi Vlad,

    I DO synchronize (not lock) on the whole database:
    public SYNCHRONIZE lock (...) {...}
    and Jim synchronize on the Record.


    OK, I missed that you synchronized on this in lock() and unlock(). But I think that - given your design - you don't need it. Synchronizing on the record to be locked, waiting() on it and being notified by a simple notify() in unlock (instead of a notifyAll()) should be OK IMO.

    By the way, one more question to you: is it really much faster: ArrayList.get(recNo), than HashMap.get(new Integer(recNo))?


    I wouldn't dare to write "much", but faster in any case, yes.
    Best,
    Phil.
     
    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 Phil,

    But I think that - given your design - you don't need it


    I do. Jim has record based locking and synchronization. That means such methods like update/read/find/create/delete have "inserted" as Jim it calls synchronization on record. I feel myself uncomfortable with this solution.
    Since I have Data singleton and file connection, allowing concurrent read/writes on different records is dabgerous. I know, nio allows atomic read/writes (there was a really fight with Max about the issue) , but Sun (to my opinion) failed to specify in which case is /read/write is atomic and in which not.
    In a real live I would use Record based locking and synchronization (even more I would use RWLLock record(not database) based synchronization), but I don't want to fail and decided for the simple, stupid solution which will never lead me to fail: record based lock/unlock, but database synchronized read/write.
    Do you remember our discussion thread about locking and synchronization designs (Jims design was #1). Ours (mine and yours RWLock) #3 and Andrews #2/4.
    I have decided to use #2/4 with exception that I want to synchronize on find method also. It is the simplest and most safe way to synchronize (of course it has the worst performance).
    So, it makes no sense to synchronize lock method on this to protect read operation (to check if the record exists) and synchronize also lock method inside Record class (what Jim makes).
    I will synchronize everything on one object this or mutex: All public methods (except getMetaData) in Data class. No more synchronized word in any other classes.
    We should not confuse my design and Jims, since the only think what they have in common is on Collection for record and its locks. The synhcronization and locking concept is totally different.
    And now back to my question: one collection or two ?
    Tx,
    Vlad
    [ September 12, 2003: Message edited by: Vlad Rabkin ]
     
    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 Vlad,

    The question was whether it makes sense to store Record objects, containing cookie (indicating a lock) in one ArrayList without having separate HasMap for Locks or it is better to separate it in ArrayList for cached Record and HashMap for locks.


    I guess it depends on what enhancements you see in the future.
    If you had a future enhancement where you do want to support read locks, you would probably want to be storing numbers of read locks (incremented/decremented for each client) as well as the cookie for the current owner.
    Likewise if you wanted to handle queueing of lock requests, so that they get handled in order. So you would want to have a sorted collection containing the clients who are requesting the lock.
    In either case you would have to start asking yourself whether the Record class is then becoming unwieldy / does not have a clearly defined purpose.
    But for the moment, I think that your Record class is clean enough to justify holding the locked / deleted status.
    So my vote is: yes, stick to just the one collection holding all your data.
    One thing I would remove though, is the boolean parameter to both the setLocked() and setDeleted() methods. You don't really want somebody setting a record as locked or undeleting a record do you? But then you might have to change your method names to reflect what you really want them to do.
    At the moment you do need to pass the boolean "true" to isLocked when you lock the record. But this is inefficient - it could be set internally when you set the cookie value.
    I am wondering about some of your variables and the methods you use to call them though. I feel some of the variables are redundant. For both the isDeleted and isLocked methods you could remove the boolean variables, and determine the values from the cookie and data variables. Pros:
  • You would save yourself a small amount of space (if you had a really large number of records this might be an issue),
  • and you would have a faster instantiation time (again, only really important if you had a really large number of records

  • Cons:
  • Verifying if data == null is slightly slower than just returning the boolean (likewise with the isLocked method


  • Regards, Andrew
     
    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 Andrew,
    Y-E-A-S!!!

    So my vote is: yes, stick to just the one collection holding all your data.


    You are the champion!!!
    You are the first who finally EXACTLY understood what I have been all the time asking for!
    I've additional variables to make the access a bit faster as you mentioned, since the memory is anyway a problem by caching the database, but you are right, I will probly refactor Record class a bit.
    T-H-A-K-S YOU!
    Vlad
     
    Max Habibi
    town drunk
    ( and author)
    Posts: 4118
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    QUOTE]Originally posted by Vlad Rabkin:
    Hi Max and Phil,
    Thanx a lot for your replies! I have been waiting for them!
    Phil:

    Max, that is not what I was asking. The question was whether it makes sense
    to store Record objects, containing cookie (indicating a lock) in one ArrayList without having separate HasMap for Locks or it is better to separate it in ArrayList for cached Record and HashMap for locks.
    Max, please, I want to know your opinion!

    Tx,
    Vlad

    Hi Vlad,
    First, a story. My father is a Ph.D in mathematics, and simply the smartest man I've ever met. Growing up, he taught my to love math with a burning passion. I was solving differential equations in the ninth grade while the other kids were outside playing, because he was such a good teacher. However, because I was a geek, I would not infrequently come to him, all excited, because I was sure I had 'proven' a new theorem. He would pour over my proof, then patiently explain what was wrong with it. Finally, after the 50th such 'proof' he said to me: 'look, I've found a new way to eat'. He locked eyes with me, picked up his fork, and circled his head with it before putting it in his mouth . More on this later.

    I, personally, think it's might be a good idea to have just a simple data structure, either a Map or an ArrayList and Records with the requisite isLocked, get Cookie, methods, as Jim suggested. Thus, I think a single structure is fine: I don't think you need a separate structure for locks: just incorporate them into records themselves. However, I don't really like Jim's record-level locking for two reasons.
    1) you're locking on two objects(Data.this and static cache structure), when you don't need to for architectural reasons. I'm a threading purist, so I hate, hate, hate nested locks. They are sometimes necessary, but I don't think this is one of those cases.
    2) My second reason is a weaker, aesthetic one. The design of locking on a given record doesn't obviously allow for the addition or deletion of records. That is, between the time you synchronize on Data.this and time to call the cache method(which is synchronized), the nature of the cache could change. To me, it just seems messy. Also, you're not really buying any efficiency here. To wit, a client C_1 who wants record R_1 still has to lock the entire cache for time x in order to achieve record R_1. another client C_2 who wants record R_1 still has to lock the cache is order to see if anyone has R_1. This is true in either implantation, weather you lock on data.this first or not. Do I need a fork?
    I'm not saying it can't work, I'm just saying I don't like it.
    All best,
    M
     
    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 Max,

    ...picked up his fork, and circled his head with it before putting it in his mouth



    It looks like you really have understood it, because all your ideas in the javaranch and in the book are simple, but reasonable. I spend one month designing and implement my assignement. Then I suddenly found javaranch...
    I am doing already 2 months nothing else then trying to make my design simpler... You are 100% right - a good developer is than one who can develop simply! I am learning it now


    Thus, I think a single structure is fine: I don't think you need a separate structure for locks: just incorporate them into records themselves.


    Finally I've got also your answer on the question of this thread. Thanx!
    It was the last design decision in project. I really appreciate your advice.


    However, I don't really like Jim's record-level locking for two reasons.


    Well, I personally like his idea, since he is the only who gained fine-grained synchronization, but agree I decided to follow extreme simplicity and do what we already discussed 2 ago (synchronizing on whole collection or Data.this).

    I hate, hate, hate nested locks


    1) Why do you use Vector in your book?
    You synchronize on reservedDVDs explicetely. I know, it is not a real nested lock, anyway why?
    2) He doesn't have them!!! Here is thread where he carefully explained his design: https://coderanch.com/t/183982/java-developer-SCJD/certification/Database-Thread-Safety-pls-review
    He doesn't synchronize on Data.this! He has two locks db-lock(on Collection) and record lock, but he didn't use one synchronized block inside other!!!

    To me, it just seems messy


    Well, I would let Jim advocate his design, since it is no the one I've choisen...
    Max I know that you are busy now, because of your new book. Thank you very much that you have found time to review this topic!
    Best,
    Vlad
     
    Jim Yingst
    Wanderer
    Posts: 18671
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    [Philippe]: OK, I missed that you synchronized on this in lock() and unlock(). But I think that - given your design - you don't need it. Synchronizing on the record to be locked, waiting() on it and being notified by a simple notify() in unlock (instead of a notifyAll()) should be OK IMO.
    Just in case it's still unclear: Vlad syncs on the Data instance throughout the operation; I sync first on the cache, to get the Record, then I drop that sync and then sync on the Record for the remainder of the operation. I agree Vlad wouldn't need the Data sync if he held a Record sync at the same time, but since he isn't syncing on a Record at all, he does need to sync on something, since Data is shared amongst multiple threads.
    [Vlad]: Since I have Data singleton and file connection, allowing concurrent read/writes on different records is dabgerous. I know, nio allows atomic read/writes (there was a really fight with Max about the issue) , but Sun (to my opinion) failed to specify in which case is /read/write is atomic and in which not.
    As you know I agree with you that atomicity is not guaranteed; however in my solution it's not necessary. Different thread can read or write to differnet parts of the FileChannel concurrently; I have no problem with that. I never use the implicit-position methods of FileChannel; I always supply the position explicitly in a read or write, so different threads can have different explicit positions concurrently; no problem. The only problem would be if two threads were reading and writing (or both writing) to the same record at the same time. That, I prevent by syncing on the Record. Actually thanks to caching there's never any file reading after startup either, so it's just concurrent writes to the same position that are a potential problem.
    Alternately, it's possible to ensure atomicity of FileChannel's writes and reads by wrapping them in sync clocks, on, say, the FileChannel. I posted code in the past showing how that works, though it's not part of my current solution since the additional sync is unnecessary. And it's one of those "evil" nested syncs, though it's done in such a way that it can't possibly create deadlock because the inner sync block doesn't even have any reference to anything other than the FileChannel (and no one else has this), so it can't possibly create a deadlock by requesting sync on something that's already synced by another thread.
    [Max]: 1) you're locking on two objects(Data.this and static cache structure), when you don't need to for architectural reasons. I'm a threading purist, so I hate, hate, hate nested locks. They are sometimes necessary, but I don't think this is one of those cases.
    There's no nesting of these two syncs. I sync on the cache to get the Record; then I exit that sync and sync on the Record instead. Much as if my caching object were a Vector:

    [Max]: 2) My second reason is a weaker, aesthetic one. The design of locking on a given record doesn't obviously allow for the addition or deletion of records. That is, between the time you synchronize on Data.this and time to call the cache method(which is synchronized), the nature of the cache could change. To me, it just seems messy.
    It's true that there are some subtle and potentially messy issues for create() and delete(); my final code is pretty clean and takes care of these concerns, but there were several places I might hve gone astray (and which a unior programmer might later screw up if they're not careful.) The kay is to ensure that there's only ever one unique Record object corresponding to a given record number. If you delete a record, the Record is still there, with isDeleted = true, until you recycle it later and set isDeleted = false, and put new data in the fields. For create(), you request a new Record from the cache, and it either recycles an old one, or creates a new one. This process needs to be synced so that if there's any concurrent create() request, it doesn't use the same Record, or create a new Record at the same position. Again, the final code is not complex-looking, but it is a bit fragile if someone mucks with it without understanding.
    [Max]: Also, you're not really buying any efficiency here. To wit, a client C_1 who wants record R_1 still has to lock the entire cache for time x in order to achieve record R_1. another client C_2 who wants record R_1 still has to lock the cache is order to see if anyone has R_1. This is true in either implantation, weather you lock on data.this first or not.
    Well if both clients are competing for the same record R_1, there's no performance benefit at all; that's true. Much more commonly, if two clients are competing for different records, then they do still both have to ccompete for the same cache lock first. So there's still a bit of contention. But once C_1 has R_1, and C_2 has R_2, they are independent. This is beneficial (performance-wise) as long as whatever processing that needs to be done on a Record takes significantly more time than the initial acquisition from the cache. Now if all the field data is in the cache too, and we're talking about reads, the difference is not that big. The only processing that needs to be done is to check the isDeleted to make sure the record is valid, then clone the String[] array of fields, and return the clone. OK, that's not a lot more complex than the initial Record acquisition, so we don't get a big win here. However for any update(), create(), or delete(), the record processing also involves waiting for a write to the file. Even with FileChannel, this is going to take longer than the simple sync lock + ArrayList get() which the cache lookup entails - it's I/O after all. So for anything involving writing to the file, there is a benefit to allowing different writes to proceed concurrently (after the initial cache lookup.) I'm not saying it's a big difference necessarily, considering that I've argued elsewhere that reads are likely much more common than writes (thanks to find()). But there is a difference.
    It's sort of odd - I originally put in record-level locking without caching. (That is, I had an ArrayList of tiny RecordLock obbjects containing locking info, but no fields.) In this case, the fact that read() required I/O meant that using Record-level sync achieved a notable performance boost, which I did directly obverve when I first moved from Data-level sync to Record-level sync. In turn, the existence of this ArrayList of RecordLocks made it really, really easy to transition to a full-cache solution in which the RecordLocks became Records which had field data as well as locking info. Now, with full caching, reading from the file is not so common, and so there's significantly less benefit to Record-level sync than there was when I first put it in. So I've gone from
    1) sync on Data, no field data in memory
    2) sync on Records, no field data in memory
    3) sync on Records, all field data in memory
    and am now contemplating
    4) sync on Data, all field data in memory.
    Which is what Vlad has, and Max says he prefers. That's I don't think I'll actualy do it, because given that I've already solved the issues of Record-level sync, there's no that much compelling reason to take it out now. Except for those pesky junior programmers who just might do something really stupid later, and muck things up... They can do that in any design really, but I'll concede they have some extra opportunities for mischief in my design that they wouldn't in Vlad's.
    So, I'm not strongly opposed to Vlad's solution; just wanted to clear up some of the confusion about how my current design actually works
    [ September 12, 2003: Message edited by: Jim Yingst ]
     
    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
    Vector cache = new Vector();
    You use Vectors, Jim?
     
    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 Jim,

    ... I never use the implicit-position methods of FileChannel;


    Exactly. Sorry, I forgot it. So, reading/writing different records with filechannel is not a problem.


    ... but I'll concede they have some extra opportunities for mischief in my design that they wouldn't in Vlad's.


    That is the only reason why decided to synchronize on Data.this. In the real project (as I as earlier) I would defently take a challenge and go your way, more over I would allow concurrent read.
    Thanx all of you for one more very usefull discussion!
    Vlad
     
    Jim Yingst
    Wanderer
    Posts: 18671
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Certainly not! I just wanted an example Max would understand better, since he uses them in his book. I already gave Max crap about this back here. My point was, though I dislike Vectors, they don't introduce problems with deadlock due to nested locks, even if called from within another sync block. Because from withint the Vector code, no other sync calls are made. My code has a similar design here, so even if it was nested sync (which it's not, currently) there wouldn't be deadlock. Unless the junior programmer really works at it. E.g. if they had access to the Vector code and we had to worry about them rewriting Vector on us, then I suppose there'd be more cause for worry about deadlock. But really, if junior programmers want to hose the system by rewriting things at random, there's little we can do to stop them; I've provided quite enough insulation against this threat, IMO.
     
    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
    Certainly not!
    Oh jeez, you almost gave me a heart attack with that Vector. Thanks for the clarification.
     
    reply
      Bookmark Topic Watch Topic
    • New Topic