Manjit wrote:which one is it? if pollDatabase is creating a new map, then nothing it is doing will affect getOutstandingAlerts.
jtahlborn ,you are right that pollDatabase operation creates a brand new copy of alerts ,but getOutstandingAlerts might try to retrieve a value when the pollDatabase operation is updating value.Making the instance variable volatile will not help here.
I was thinking that getOutstandingAlerts method should create an exact copy of alert and retrieve the value from the copy ,rather than the original alert.This way the client code dont have to worry about what the pollDatabase does to the original alert instance variable.Is my strategy good.maybe there is more code that you aren't showing here? you are essentially describing a copy-on-read solution, which is very inefficient if reads outnumber writes. my proposed solution is sort of like copy-on-write. if all the getOutstandingAlerts method is doing is calling get() on the map, my proposed solution will work just fine.
Manjit wrote:Yes, that's the whole point of ConcurrentHashMap. I don't see why it's a problem for your design -- not that I could tell much about your design from what you posted, but it sure looked like one thread was doing a "put" and the other thread was doing a "get". If that's all it is then ConcurrentHashMap should be fine.
Clap,initially I did make it ConcurrentHashmap ,but I believe concurrent hashmap doesnt lock the collection during a get.This means when one thread is updating the map ,the second thread can get values from the map.The get operation of the map doesnt lock the map.
Manjit wrote:because volatile ensures correct memory visibility between threads. if you didn't use volatile, then a caller to getOutstandingAlerts() could see some partially initialized/corrupt version of the map.jtahlborn wrote:Jtalhorn,I think I got your point.Basically you are suggesting that by creating a brand new map in each pollDatabase operation, then I dont have to worry about get operation ,simply because the client code is still looking at old memory location(old copy) and hence should not worry about values getting changes by pollDatabase method.
if the pollDatabase() method is getting a complete new copy of the alerts every time, and the alerts map isn't being modified at all after load, then you can just:
- make the alerts member variable volatible
- create a new HashMap each time pollDatabase() is called, fill it in, and then assign it to the alerts member variable.
I got it.
But the small doubt is that why even I should bother about making the instance variable volatile ? Your suggestion should work without making it volatile