I am using the record number itself as the cookie that is returned. This is fine, right?
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Originally posted by Andrew Monkhouse:
Hi Satish,
You are right - this will work. However you may have unnecessary processing happening.
Some questions for you:Why are you locking the records for read operations? (There is one valid case for that - just want to see if it applies in your case)
Andrew I am not using static RAF, so many threads will be writing, deleting, updating at the same time. The reason why I am locking db while reading is -- if I want to read record number 7, and if I'm not using locking, there might be another thread, which wants to delete the record or update the record. So while I'm reading, I'm giving room for another threads to change the data. For this reason, I'm locking records before reading. Is it correct? And you are saying about a valid case, is this the valid case or its a different scenario?
Why are you notifying other threads in step 1e?
Consider the psuedo code
Lets assume t1, t2, t3, t4, t5 threads are present.
t1, t2, t3 wants to operate on record number 100, t4 on 200, t5 on 300.
Each thread has to lock the record number in lockedRecords before performing any function on that record. To do this, they have to obtain a lock on lockedRecords itself and then check if the record number is already locked. If the record is already locked they will wait by calling wait() on lockedRecords.
Now coming to the example, if t1 gets a lock on lockedRecords, it checks if record 100 is already locked or not, if not it locks record 100 and returns from lock() method. Then reads and unlocks. Meanwhile, if t2 or t3 calls lock() and gets lock on lockedRecords, they see that record 100 is already locked. So they will wait by calling wait() on lockedRecords. But if t4 or t5 gets lock on lockedRecords, they will lock their records and goes on. notifyAll() is for threads t2, t3 which are waiting by calling wait() method. You think this is OK?
Why do you lock the entire database for a find operation? What does the find operation return to the calling class?
The same reason as why I'm locking while doing read operation. find operation returns the records which match the given criteria.
Many people are trying to add some (minimal) security / safety into their application by using non guessable cookies. With your scheme, there is nothing to stop me writing a program that tries to unlock a record I don't own the lock for - since I know what the cookie will be, it is easy for me to do.
If you do use the record number as the cookie, then I would recommend you put a comment in your design documents stating that safety is not a design requirement. I would also recommend that you do not put in your API documentation the fact that you are returning the record number - that way a future modification could make a more safe solution without breaking documented functionality.
OK Andrew. First, I will try to change cookie to provide some security, if I can't I will document it accordingly. Thanks.
Regards, Andrew
Originally posted by Andrew Monkhouse:
1 Why are you locking the records for read operations? (There is one valid case for that - just want to see if it applies in your case)
Originally posted by Satish Avadhanam:
Andrew I am not using static RAF, so many threads will be writing, deleting, updating at the same time. The reason why I am locking db while reading is -- if I want to read record number 7, and if I'm not using locking, there might be another thread, which wants to delete the record or update the record. So while I'm reading, I'm giving room for another threads to change the data. For this reason, I'm locking records before reading. Is it correct? And you are saying about a valid case, is this the valid case or its a different scenario?
Originally posted by Andrew Monkhouse:
2 Why are you notifying other threads in step 1e?
Originally posted by Satish Avadhanam:
Consider the psuedo code
Lets assume t1, t2, t3, t4, t5 threads are present.
t1, t2, t3 wants to operate on record number 100, t4 on 200, t5 on 300.
Each thread has to lock the record number in lockedRecords before performing any function on that record. To do this, they have to obtain a lock on lockedRecords itself and then check if the record number is already locked. If the record is already locked they will wait by calling wait() on lockedRecords.
Now coming to the example, if t1 gets a lock on lockedRecords, it checks if record 100 is already locked or not, if not it locks record 100 and returns from lock() method. Then reads and unlocks. Meanwhile, if t2 or t3 calls lock() and gets lock on lockedRecords, they see that record 100 is already locked. So they will wait by calling wait() on lockedRecords. But if t4 or t5 gets lock on lockedRecords, they will lock their records and goes on. notifyAll() is for threads t2, t3 which are waiting by calling wait() method. You think this is OK?
Originally posted by Andrew Monkhouse:3 Why do you lock the entire database for a find operation? What does the find operation return to the calling class?
Originally posted by Satish Avadhanam:
The same reason as why I'm locking while doing read operation. find operation returns the records which match the given criteria.
Originally posted by Andrew Monkhouse:Many people are trying to add some (minimal) security / safety into their application by using non guessable cookies. With your scheme, there is nothing to stop me writing a program that tries to unlock a record I don't own the lock for - since I know what the cookie will be, it is easy for me to do.
If you do use the record number as the cookie, then I would recommend you put a comment in your design documents stating that safety is not a design requirement. I would also recommend that you do not put in your API documentation the fact that you are returning the record number - that way a future modification could make a more safe solution without breaking documented functionality.
Originally posted by Satish Avadhanam:
OK Andrew. First, I will try to change cookie to provide some security, if I can't I will document it accordingly.
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Originally posted by Andrew Monkhouse:
So are you opening the RAF and closing it with each operation? For example, each call to read() will result in a new instance of the RAF being opened, read, and then closed?
This is the scenario where you might want to consider record locking for reads.
Originally posted by Andrew Monkhouse:
The call to notifyAll() should be unnecessary.
Originally posted by Andrew Monkhouse:
Personally I think that this is going to be slow and reduce concurrency, with little benefit
Originally posted by Andrew Monkhouse:
You have said you are returning records, not record numbers.
Originally posted by Andrew Monkhouse:
Note that I did say that you could keep with your current design, as long as you document it.
My personal preference is to try and make a more robust solution than is necessary (and normally Mark jumps in with a comment that simple designs are preferable to complex solutions).
Originally posted by Andrew Monkhouse:
Note that returning dirty data is acceptable (and almost impossible to avoid without sacrificing concurrency). You just have to verify the data when it comes time to book the record. (You have to decide what level of checking you are going to do when you do the booking, and justify that in your design decisions document.)
Regards, Andrew
Originally posted by Satish Avadhanam:
Sorry. I meant I'm returning record numbers.
Do you think the locking is OK now?
I think all we can do to avoid is to read the correct data by locking db and once we read there is no way to keep up with other threads and updating the returned records to the customer right?
The returned records are good only before the read/find method returns. For our project scope, once the read/find method returns, its not required to update the client side dataset. Am I right?
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Originally posted by Andrew Monkhouse:
Hi Satish,
Hmmm - you have said that you understand that the notifyAll() is unnecessary in the lock() method, but you have not said that you are removing it. So I do not know whether your locking is OK or not
Andrew, I'm going to remove it.
I believe that there is no need to update the returned records.
OK Andrew.
Regards, Andrew
Originally posted by Satish Avadhanam:
I hope and wish you, george, phil and other big people make some comment ...
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Never trust an airline that limits their passengers to one carry on iguana. Put this tiny ad in your shoe:
New web page for Paul's Rocket Mass Heaters movies
https://coderanch.com/t/785239/web-page-Paul-Rocket-Mass
|