• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

NX: createRecord and locking

 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would like to get an opinion on my design for the createRecord functionality in the Data class. In my create, I call a method findAvailableRecordNumber(), which figures out if I can re-use an existing deleted record or if I need to append on to the end of the file. I am considering the record number (i.e. placement in the database file) as my
key. I am trying to prevent the locking problem where some other thread grabs this available record number after I find it, but before I lock it
and use it. In this situation, I am currently throwing a DuplicateKeyException. (I have seen the posts on this topic, and lots of folks don't feel this is necessary. However, I thought that it made sense in this particular situation.)
I set the find and lock logic up in a loop to be executed a maximum number of times. I originally coded it as a while (true), but did not want to end up in an infinite loop. The persistNewRecord method is where the real persistence work is done, and is called in the case where we get a valid lock. The persist method is synchronized.
Note: the lockRecordForAdd method that is referred to below is in addition to my required public long lockRecord(long recNo) method. In the normal required method, my specs require me to throw RecordNotFoundException in the case where the record does not exist or is out there but marked deleted. However, for a create case like this, that is actually the situation I am looking for. i.e. I WANT it to either be deleted or non-existent.
Here is the pseudo code for my create:

Questions:
1. Opinions on the logic here?
2. One other question regarding my persist logic - Down in the bowels of my persist method, I do one of two things:
a) update an existing record (set delete byte to false, updateRecord method)
b) append to end of file (by set new length on file, set delete byte to false, updateRecord method)
In b) above, is it bad to use the common updateRecord method to get the data out there for my add? I liked it, because I only had to do the file manipulation in one place. But is it confusing to be doing an "update" within create functionality?
Thanks. Any help appreciated.
TJ
 
joe black
Ranch Hand
Posts: 103
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Since no other methods need to find an available record number then whynot just place your code for findAvailableRecordNumber() in the create() and synchronize the method?
 
Bill Robertson
Ranch Hand
Posts: 234
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Terry,
Just a few general comments considering I am not sure which project you
have:
1. Depending upon how you set up your Data class the DuplicateKeyException
should never occur. Is this part of your interface or the interface
provided by Sun? If you have to check for it because its part of Sun's interface then you of course must do it, if you are throwing it by your
own design then I would re-think your design.
2. What do your directions say in regards to other threads waiting on a
record and then being notified when the record is available. I don't know
if your directions address this, but I know your code doesn't seem to address this.

In b) above, is it bad to use the common updateRecord method to get the data out there for my add? I liked it, because I only had to do the file manipulation in one place. But is it confusing to be doing an "update" within create functionality?

Yes its confusing
adios mi amigo, bill
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the comments. I have URLyBird 1.2.1.
Joe - good point. I split it out just to make the original method shorter and cleaner, but you're right, with the logic only used in one place I would not have to.
Bill -

1. Depending upon how you set up your Data class the DuplicateKeyException
should never occur. Is this part of your interface or the interface
provided by Sun? If you have to check for it because its part of Sun's interface then you of course must do it, if you are throwing it by your
own design then I would re-think your design.

The DuplicateKeyException is part of my interface. I wish it wasn't , because it's causing me some grief right now. I still see the potential for it's use in the locking problem I'm trying to prevent. (i.e. the following from my original post)

I am trying to prevent the locking problem where some other thread grabs this available record number after I find it, but before I lock it
and use it. In this situation, I am currently throwing a DuplicateKeyException. (I have seen the posts on this topic, and lots of folks don't feel this is necessary. However, I thought that it made sense in this particular situation.)

From Bill -

2. What do your directions say in regards to other threads waiting on a
record and then being notified when the record is available. I don't know
if your directions address this, but I know your code doesn't seem to address this.

My directions are similar to others about waiting, and my normal lock method on the Data class handles things similar to Max's book. I'm still confused about the create though, because if the problem I'm trying to prevent occurs, I would not want it to wait for the resource to become available, because it potentially never would. (i.e. Thread 1 attempting to add a new record at spot 33, Thread 2 beats Thread 1 to it and adds record 33. I would not want Thread 1 to wait forever for spot 33.)
Is this situation something people are typically preventing from happening?
Thanks for the feedback!
TJ
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think I am making more out of this than I should.
Here are a couple followup questions from my posting above -
1. Do I even need locking functionality in a create? I only have 1 instance of my Data object, so as long as I synchronize my create method, would I be ok without specifically locking the new record number?
2. pseudo code for my synchronized create could go like this:

I think this would simplify my code a great deal and also cover what is in the specs.
Thoughts?
TJ
[Andrew: Broke up really long lines]
[ January 04, 2004: Message edited by: Andrew Monkhouse ]
 
Bill Robertson
Ranch Hand
Posts: 234
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Thread 1 attempting to add a new record at spot 33, Thread 2 beats Thread 1 to it and adds record 33. I would not want Thread 1 to wait forever for spot 33.

As Joe said, why not just synchronize this process. In fact, I can't
imagine that any module that determines record numbers is not done
in a sychronized manor.
You may also not want to call you record generator until the very last
moment.
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Bill. That separate record determination method that I have is synchronized, I just refactored it out to a separate method to make the big create method shorter.
To summarize then,
1. Are you basically saying that as long as this record determination process is synchronized, I won't ever get in the crazy situation I am trying to prevent?
2. Likewise, is it true then that I really don't need any type of locking on the create?
3. Do you agree that the pseudo code above seems right? (The read record would probably never find a valid record out there and would process the create successfully. I would basically just be doing this check to meet the requirements of my spec for the DuplicateKeyException)
Thanks.
TJ
 
Bill Robertson
Ranch Hand
Posts: 234
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

To summarize then,
1. Are you basically saying that as long as this record determination process is synchronized, I won't ever get in the crazy situation I am trying to prevent?

Yup!!!

2. Likewise, is it true then that I really don't need any type of locking on the create?

Yes you need locking!!! You need to lock your filehandler or you cache
(depends on how its designed.). When you create you use seek(), correct?
Well what happens if you seek to the correct position but then someone
else gets a hold of your filehandler and performs its own seek on the file.
No the position in the file is all screwed up. Remember, you can have the instances of filehandler you want, but there is only one file. And don't think I am telling you to lock the file. I am saying you need to synchronize on a variable that points to the file.
This variable is static in all cases from what I have seen.

3. Do you agree that the pseudo code above seems right? (The read record would probably never find a valid record out there and would process the create successfully. I would basically just be doing this check to meet the requirements of my spec for the DuplicateKeyException)


I am assuming the psuedo code is incomplete but it looks great.
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Bill - I'm glad you're on the forum today because this is really helping me out.
I have followup question on your quote:

Yes you need locking!!! You need to lock your filehandler or you cache
(depends on how its designed.). When you create you use seek(), correct?
Well what happens if you seek to the correct position but then someone
else gets a hold of your filehandler and performs its own seek on the file.
No the position in the file is all screwed up. Remember, you can have the instances of filehandler you want, but there is only one file. And don't think I am telling you to lock the file. I am saying you need to synchronize on a variable that points to the file.
This variable is static in all cases from what I have seen.

My situation is this:
I have 1 instance of Data singleton.
Data has one instance of schema variable.
Schema has the cached list of rooms and also the RandomAccessFile.
My Data methods that update the file/cache are all synchronized and they keep the file/cache in synch.
Since I only have 1 data instance, and all methods that manipulate the file are synchronized, I'm thinking I don't need any extra record specific locking on the file handler or cached list. When I said I don't need any type of locking on the create, I mis-spoke. What I meant was that I did not need the special record locking (i.e. in my lockedRecords list with cookie) that is dealt with in the Data lock method.
My understanding is that by having 1 data instance, one RandomAccessFile pointer and synchronizing on the data manipulation methods, I won't have to do any further synchronization on the cache list or file handler.
Am I still missing something?
TJ
 
Bill Robertson
Ranch Hand
Posts: 234
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
From what I see, you seem right on.
Question for you, are you synchronizing any methods in your Schema given
it contains functions that manipulate the file handle. I understand that
you coded it such that there is no way anyone can call these methods
except in a synchronized manor via you Data class. But its just a thought.
And actually if you have, there could be some deadlock issues lurking in
the background.
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
"deadlock issues lurking" is a scary expression
It is one of my big fears right now, and unfortunately I don't have any real world experience with this.
To answer your questions, I am synchronizing a couple methods in my schema class ---
1. getHotelRooms() (i.e. the cache list) and
2. addHotelRoom() (i.e. add to cache list)
I use the getHotelRooms in my Data create, update, delete, and find methods all of which are synchronized on the Data instance
I use the addHotelRoom in my Data create method
My Data instance is set up following the normal singleton approach - public getInstance method with a private constructor to enforce only one instance. However, when I looked back in my schema, the schema constructor is public. So even though I force my one Data instance to only have one instance of the schema, I can't really prevent others (i.e. Sun's dreaded automatic failure test) to construct instances of schema, perhaps all pointing to the same RandomAccessFile pointer.
Are these the lurking problems you are talking about?
Would I be better off to have my schema be some type of inner class to my Data object? Then I wouldn't have to synchronize schema methods or worry about public constructor because only my Data instance could get at them.
Ideas?
TJ
 
Bill Robertson
Ranch Hand
Posts: 234
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have not completed my assignment due to a few issues and this is
one of them. Plus I work full time and I am getting my masters at
Harvard so I am rather busy.
Thus, start a new post on this or I will within the next few days.
Your design is very very similar to mine. I keep going back and forth
on whether or not to synchronize on those methods in Schema. (my class
is called DBFileHandler). As far as reading or writting bad data
I don't think we need to synchronize them because no class can get
to them except through the synchronized code in Data. But, it seems
strange to leave them un-synchronized considering they do file manipulation.
Your thoughts.
As a side note, are you remembering to unlock your current record
before you throw an exception in an update, book, delete...etc.
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Bill - sounds like your life is busy.
Thanks for the tip on unlocking records - I don't have that in there yet. I assume that I also need to make sure only to update the cache list if the database file update succeeds. (i.e. update the file first and then continue on to the cache list update if it was successful. Otherwise, bail out and neither gets updated.)
1. Is the above what you do?
2. Do you use a WeakHashMap or just a HashMap when storing your locked records? (I still need to research this a little - I currently just have a HashMap.)
Discussion on Data/Schema issues is continued here at your suggestion.
Thanks Bill.
TJ
 
Bill Robertson
Ranch Hand
Posts: 234
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Thanks for the tip on unlocking records - I don't have that in there yet. I assume that I also need to make sure only to update the cache list if the database file update succeeds. (i.e. update the file first and then continue on to the cache list update if it was successful. Otherwise, bail out and neither gets updated.)
1. Is the above what you do?
2. Do you use a WeakHashMap or just a HashMap when storing your locked records? (I still need to research this a little - I currently just have a HashMap.)

1. No. I cache nothing. Its safer and more simple to go to the database directly at all times. However, this is much slower and less practical - but for this project its fine. But, if you have caching working I would leave it, given its a much more elegant solution and you learn a lot more -at least it seems that way.
2. I use a Hashmap that holds the record numbers of the locked Contractors.
Search on WeakHashMap. You need it only under certain circumstances and I am not 100 percent comfortable telling you those circumstances. Basically though I believe its only really needed if you are locking from the client
side and if you client crashes a long goes a record with it. With a weakhashmap it automatically gets rid of unreferenced objects after a certain point in time - unlike plain old hashmap. I lock from the server side so it doesn't matter if a client goes down. There has been tons of threads regarding the use of a weakhashmap. Am I making any sense to you?
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Bill.
TJ
 
HaoZhe Xu
Ranch Hand
Posts: 222
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
How does your findAvailableRecordNumber work? If you lock a record that has not been created, will the lock work?
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
HaoZhe -
I don't do the logical record locking in my create method. My create method is synchronized, and I use the singleton Data approach where the only way to get at the data manipulation is via the synchronized Data methods.
The way I find the available record number is:
- go through the records to see if there is a deleted one out there for re-use
- if so, use that one
- if not, figure out what the next record number in the file should be for appending to the end
Hope that helps.
TJ
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic