Solved

improving cache design

Posted on 2014-01-10
6
146 Views
Last Modified: 2014-02-04
hi guys

I have somethng like this


public class CacheImpl{

Map<String,CacheData> localCache = new HashMap<String,CacheData>();

private void cacheImpl(){
String webserviceId = getWebserviceId(); //returns a uniqe webserviceId ;
cacheMap.put(webserviceId , new CacheData());
Map cache = construct(cacheMap);
}


private Map construct(Map cacheMap){
CacheData newCData = null;
//get webserviceId from cacheMap, make a webservice call and return CacheData
for(String webserivceId : cacheMap.keySet()){
 newCData = webservicecall(webserivceId);
}
 cacheMap.put(webserviceId , newCData);  
 localCache.putAll(cacheMap); //put cacheMap in localCache
}

}//end class 

Open in new window



The idea is to first check the localCache, it the key already exists , use the value .
If not, make a call to webservice, so we have a check like this in cacheImpl()

private void cacheImpl(){
String webserviceId = getWebserviceId(); //returns a uniqe webserviceId ;

if (localCache.get(webserviceId)!= null){
CacheData = localCache.get(webserviceId);
}
else{
// make webservice call
cacheMap.put(webserviceId , new CacheData());
Map cache = construct(cacheMap);
}
}

Open in new window

Question 1. Is this a good design?


 The idea is also to clean up the localCache once a day so we have another method which does that

	@Scheduled(cron = "${localCacheCleanup}")  //localCacheCleanup runs at 12.00 AM everyday
	public void localCacheCleanup() throws InterruptedException {		 
			localCache.clear(); 
			Question 2:
			//Will there be a provlem here? Lets say one thread (from executor serivice thread pool is already using localCache
			//while the scheduler thread removes the data from localCache. Any sugessions how to improve the design)
		 
	}

Open in new window


thanks
0
Comment
Question by:royjayd
  • 3
  • 2
6 Comments
 
LVL 35

Assisted Solution

by:girionis
girionis earned 50 total points
ID: 39773612
Yes, this is actually a perfectly valid design, in fact several applications work like this, think of a database pool, or a thread pool, although not the same they use the same technique. If you want to perfect your design you could preload the most used web services and add them to hash map during application start up.
0
 
LVL 26

Expert Comment

by:dpearson
ID: 39773909
This design is basically fine for a single threaded application.

However, if you are supporting multiple threads using the same cache (which it sounds like you are) then this design isn't quite right.  The problem is that HashMap doesn't support multiple threads operating on the same map well.  You can get "ConcurrentModificationExceptions" and other nasty problems if one thread modifies the map while another is reading from it.

The solution is very simple - move to using:
ConcurrentHashMap<String, CacheData>

instead of HashMap.  The rest of your code won't need to change, and you'll avoid a lot of ugly issues.

Also I think this piece of the code is actually wrong:

CacheData newCData = null;
//get webserviceId from cacheMap, make a webservice call and return CacheData
for(String webserivceId : cacheMap.keySet()){
 newCData = webservicecall(webserivceId);
}
 cacheMap.put(webserviceId , newCData);  

Open in new window


That will only record the last newCData value.  The rest are discarded.

Think it should be:

//get webserviceId from cacheMap, make a webservice call and return CacheData
for(String webserivceId : cacheMap.keySet()){
 CacheData newCData = webservicecall(webserivceId);
 cacheMap.put(webserviceId , newCData);  
}
...

Open in new window


Doug
0
 

Author Comment

by:royjayd
ID: 39779925
thanks Doug. I am still a little doubtful if just changing HashMap<String, CacheData>
to ConcurrentHashMap<String, CacheData> alone will solve the problem.

Lets say one executor service thread is executing this line
>> CacheData = localCache.get(webserviceId);

while the scheduler thread executes this line
>>localCache.clear();

and both above operations occur at the exact same time with some unlucky timing, then that would be a issue although i am using ConcurrentHashMap.

Please correct me if i am wrong.

Thanks.
0
How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

 
LVL 26

Expert Comment

by:dpearson
ID: 39780219
Those two operations should be fine.

It's sort of the goal of ConcurrentHashMap.  One thread can be reading from it (localCache.get()) which the other thread is writing to it (localCache.clear()).

Whether the thread doing the "get" sees an empty bucket or not would depend on whether the "clear" had reached that bucket of the hashmap or not, but either way the behavior is valid.  If both threads are executing at the same time, the get could return either the cached value or an empty value, but it would not error out.  Same thing for the clear - it would not be blocked or trigger an error if it was trying to clear the bucket in the map that the get was actively trying to read.

And once the clear had completed, the next request to the cache would definitely get an empty result.

So I think it's all OK.

Doug
0
 

Author Comment

by:royjayd
ID: 39780271
Thanks.

>>>the get could return either the cached value or an empty value, but it would not error out.

Agree, i think the problem would be here

if (localCache.get(webserviceId)!= null){ -->thread 1 comes to this line here and localCache is not cleared yet
CacheData = localCache.get(webserviceId); -->now when thread1 comes to this line and if localCache is cleared (by thread2), there is a potential issue
}                                         --> either a Null pointer exception will be thrown or it will return  empty value and call to 
else{                                     --> webservice will be skipped since i am already inside the if() condition.Sicne the localCache has been 
// make webservice call                   --> cleared out i would actually like to make a call to the webservice in the else section
cacheMap.put(webserviceId , new CacheData());
Map cache = construct(cacheMap);
}
}

Open in new window

0
 
LVL 26

Accepted Solution

by:
dpearson earned 450 total points
ID: 39780658
Oh yeah - that is wrong, but it's not really a design problem it's just a code bug.  I think you want this (i.e. use a local variable for the result of the lookup, not 2 calls):

CacheData cachedValue = localCache.get(webserverId) ;

if (cachedValue == null) {
    cachedValue = new CacheData() ;
    cacheMap.put(webserviceId , cachedValue;

// FYI - this pattern is a bit weird.  Usually would fill in data to "cachedValue" first and then put it in the map.  Here you're sort of putting a blank in first and then populating it.
    Map cache = construct(cacheMap);
}

return cacheValue ;
0

Featured Post

Threat Intelligence Starter Resources

Integrating threat intelligence can be challenging, and not all companies are ready. These resources can help you build awareness and prepare for defense.

Join & Write a Comment

After being asked a question last year, I went into one of my moods where I did some research and code just for the fun and learning of it all.  Subsequently, from this journey, I put together this article on "Range Searching Using Visual Basic.NET …
Java contains several comparison operators (e.g., <, <=, >, >=, ==, !=) that allow you to compare primitive values. However, these operators cannot be used to compare the contents of objects. Interface Comparable is used to allow objects of a cl…
Video by: Michael
Viewers learn about how to reduce the potential repetitiveness of coding in main by developing methods to perform specific tasks for their program. Additionally, objects are introduced for the purpose of learning how to call methods in Java. Define …
This tutorial covers a practical example of lazy loading technique and early loading technique in a Singleton Design Pattern.

706 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

22 Experts available now in Live!

Get 1:1 Help Now