Win a copy of The Little Book of Impediments (e-book only) this week in the Agile and Other Processes forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

create() method and synchronization

 
Ziji (Jay) Zhang
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here is a snap shot of my create() method,
this method is in my DBFileAccess class, which is a Singleton class.

// check duplicate record and may throw DuplicateRecordException
boolean isReuse = true;
int recNo =-1;
readWriteLock.readLock().lock();
try{
recNo = deletedRecordNumber.get(0);
}finally{
readWriteLock.readLock().unlock();
}
if (recNo <=0 ){
try {
recNo = getTotalRecord() +1; // synchronized on raf
isReuse = false;
} catch (IOException ex) {

throw new DBFileAccessException(ex);
}
}else {
isReuse = true;
}
// I think up to here is OK,
// write () method writes into db file, synchronized on file instance
// potentially throws RuntimeException

write(new Record(DBSchema.VALID_RECORD, recNo , data));
if (isReuse ){
readWriteLock.writeLock().lock();
try{
deletedRecordNumber.remove( new Integer(recNo));
}finally{
readWriteLock.writeLock().unlock();
}
}

The question I have is, what if the current thread swapped out, another thread comes in, does the write(), and removed recNo from the deletedRecordNumber list?

When client call create method, lock() of Data class is not called, as
recNo could be a deleted record or doesn't exist yet.

Any comments and suggestions?

Ziji
 
rinke hoekstra
Ranch Hand
Posts: 152
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Ziji


1) There is no MUST to reuse deleted recs. It makes you life much more simple if you just ignore this instruction - and again: remember that one of the base instructions is to keep things simple. I did not reuse deleted entries, and I've seen people pass with 400 points (100%) doing the same. New recs are just appended in that case. Of course, you'd have to motify that decision in choices.txt.

2) why the complicated thing with an explicit lock? I just used synchronized.

Hope it helps
 
Ziji (Jay) Zhang
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks rinke!
1) There is no MUST to reuse deleted recs. It makes you life much more simple if you just ignore this instruction - and again: remember that one of the base instructions is to keep things simple. I did not reuse deleted entries, and I've seen people pass with 400 points (100%) doing the same. New recs are just appended in that case. Of course, you'd have to motify that decision in choices.txt.


I may take this approach. that will make life easier!

2) why the complicated thing with an explicit lock? I just used synchronized.


I was following Andrew's book, in this case, using ReadWriteLock is more efficient than classic synchronized, as it allow multiple reading.

Just for purely understanding,
If I move write() into the lock block, will that be OK?
But then I face nested locking issue, since within write(), it also locks the FilePointer instance.

Here is the reformated chuck of the code:

if (isReuse ){
readWriteLock.writeLock().lock();
try{
write(new Record(DBSchema.VALID_RECORD, recNo , data));
if (deletedRecordNumber.contains(new Integer(recNo))
deletedRecordNumber.remove( new Integer(recNo));
}finally{
readWriteLock.writeLock().unlock();
}
}

(Note: this create method is in DBFileAccess, it is a singleton,
meaning multiple threads can use it's methods concurrently.)

Any comments!

Ziji
 
Jason Moors
Ranch Hand
Posts: 188
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would be very surprised if anyone got 100% without implementing this requirement, and I'm interested how you could justify not implementing it.

The JavaDoc for the created methods, states that deleted records are reused, not to mention the optimisation aspect, how can you justify the wasted disk space?

Keep it simple but not at the expense of the requirements.

Jason
[ July 19, 2007: Message edited by: Jason Moors ]
 
Gabriel Vargas
Ranch Hand
Posts: 145
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I agree with Rinke, you can use a syncrhonized block or can use ReentrantLock class , i mixed both strategies. I can see than your code can have a trouble. I mark with asterisks where i think that trouble can happen.



In your code you lost your lock over the monitor and later try to get again this lock, i think that this can cause error in data and maybe a lot of headaches to you because you split atomic operations. I see in Andrew's examples than read lock and write lock are chained, if my memory not fails, he gets a read lock, then gets a write lock, then releases read lock and finally releases write lock (another advantage of new concurrency classes). Please check if i'm wrong before you have that headaches .

I hope it help you.
 
Gabriel Vargas
Ranch Hand
Posts: 145
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jason Moors:
I would be very surprised if anyone got 100% without implementing this requirement, and I'm interested how you could justify not implementing it.

The JavaDoc for the created methods, states that deleted records are reused, not to mention the optimisation aspect, how can you justify the wasted disk space?

Keep it simple but not at the expense of the requirements.

Jason

[ July 19, 2007: Message edited by: Jason Moors ]


Hi Jason,

Strictly specifications tell me that:


// Creates a new record in the database (possibly reusing a
// deleted entry)


I emphasaize in word "possibly", i think this is possible.

And in the other part tell:


A clear design, such as will be readily understood by junior programmers, will be preferred to a complex one, even if the complex one is a little more efficient.


It could be interpreted in many ways, i personally interpret than a complex design involves use deleted entries and i think isn't are expected database grows a lot, for these reason i think don't use deleted entries is a better choice than use it, but it doesn't mean than your opinion is wrong, only i think that you will decide you must put in your choices.txt

I hope it help you.
 
Ziji (Jay) Zhang
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In your code you lost your lock over the monitor and later try to get again this lock, i think that this can cause error in data and maybe a lot of headaches to you because you split atomic operations. I see in Andrew's examples than read lock and write lock are chained, if my memory not fails, he gets a read lock, then gets a write lock, then releases read lock and finally releases write lock (another advantage of new concurrency classes). Please check if i'm wrong before you have that headaches


Gabriel,
I will check Andrew's book to confirm.

I read anothor thread talking about lock in create method,
http://www.coderanch.com/t/185162/java-developer-SCJD/certification/nx-All-URLy-Bird-read.

Where it locks entire create method on instance of RandomAccessFile.
This may have lower performance, but it is a conservative approach.
I may do the similar thing with create().

Ziji
 
Ziji (Jay) Zhang
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Also, by locking entire create method body,
I don't need to maintain a deleted recNo list. I can just search for the first deleted record, overwrite record if found, otherwise append at the end. All done in one synchronized block. Without using nested or chaining locks.

Ziji
 
rinke hoekstra
Ranch Hand
Posts: 152
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jason Moors:
I would be very surprised if anyone got 100% without implementing this requirement, and I'm interested how you could justify not implementing it.

The JavaDoc for the created methods, states that deleted records are reused, not to mention the optimisation aspect, how can you justify the wasted disk space?

Keep it simple but not at the expense of the requirements.

Jason

[ July 19, 2007: Message edited by: Jason Moors ]


Hi Jason,

The guy in this thread got 400 points while ignoring this requirement.

I think you can justify this because it really makes the application much
more simple, and that is a requirement too.

I don't believe that inefficient disk space use is a serious argument against it, given the very simple database and the fact that even the most simple new computer nowadays has at least an 80 GB hard disk.
 
Jason Moors
Ranch Hand
Posts: 188
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It's seems from Rinke's thread that people have passed without reusing deleted records, although I personally think I've implemented a clean and simple design.


// Creates a new record in the database (possibly reusing a
// deleted entry)


Gabriel I guess it comes down to your interpretation of 'possibly', to me this means that it will reuse a deleted record if one exists, not that the developer might 'possibly' implement the functionality

I guess as long as you document your decisions you should be okay, I'm just not willing to take that chance for the sake of a few lines of code.

Jason
 
Ziji (Jay) Zhang
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am going to implement create method by synchronized on instance of RandomAccessFile. As synchronized is reentrant, meaning, synchronize multiple time on the same object by the same thread is perfectly ok.

It is hard to achieve this by using ReentrantLock as ReentrantLock is
a separate object.


// create method body:
synchronized(raf){
checkForDuplicate() // also synch on raf, may throw DuplicateKeyException.
getFirstDeletedRecNo() // inside this method, synch on raf
if (recNo <0) recNo = getExistingTotalRecordCounts() +1;
write(data, recNo); // synch on raf



}

Ziji
 
Ziji (Jay) Zhang
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am going to implement create method by synchronized on instance of RandomAccessFile. As synchronized is reentrant, meaning, synchronize multiple time on the same object by the same thread is perfectly ok.

It is hard to achieve this by using ReentrantLock as ReentrantLock is
a separate object.


// create method body:
synchronized(raf){
checkForDuplicate() // also synch on raf, may throw DuplicateKeyException.
getFirstDeletedRecNo() // inside this method, synch on raf
if (recNo <0) recNo = getExistingTotalRecordCounts() +1;
write(data, recNo); // synch on raf



}

Ziji
 
Gabriel Vargas
Ranch Hand
Posts: 145
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Jason Moors:
It's seems from Rinke's thread that people have passed without reusing deleted records, although I personally think I've implemented a clean and simple design.



Gabriel I guess it comes down to your interpretation of 'possibly', to me this means that it will reuse a deleted record if one exists, not that the developer might 'possibly' implement the functionality

I guess as long as you document your decisions you should be okay, I'm just not willing to take that chance for the sake of a few lines of code.

Jason


Hi Jason.

Yes i agree with you, functionality must be implemented, but there is two (or more ) ways to do this (possibly), reuse a deleted entry or append at the end of the file. I select this way because in at the begin of the assigment it states:


... The IT director does not anticipate much reuse of the first Java technology system, but intends to use that system as a learning exercise before going on to a web based system.


I see this assigment like a functional prototype, yes, the best option is reuse deleted entries but is more simpler don't reuse deleted entries (If this is a prototype it doesn't mean this can work wrong, it must work fine) and i find some other statements than fits in prototype idea. I think some threads advices keep it simple for the assigment, i think there is a statement (really two) than emphasizes this:


A clear design, such as will be readily understood by junior programmers, will be preferred to a complex one, even if the complex one is a little more efficient. Code complexity, including nesting depth, argument passing, and the number of classes and interfaces, should be reasonable.


I think if you want implement reuse deleted entries is fine but i also think than don't use deleted entries is fine (simpler, less efficient, but fine) .

I hope it help us.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic