Solved

improving cache design

Posted on 2014-01-10
6
153 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 27

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
Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

 
LVL 27

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 27

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

Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
Maven Project: Hibernate Dependencies Conflict 10 38
varialbe initialization 11 37
Java string replace 11 54
Eclipse for Java EE development 2 27
An old method to applying the Singleton pattern in your Java code is to check if a static instance, defined in the same class that needs to be instantiated once and only once, is null and then create a new instance; otherwise, the pre-existing insta…
Introduction This article is the last of three articles that explain why and how the Experts Exchange QA Team does test automation for our web site. This article covers our test design approach and then goes through a simple test case example, how …
Viewers will learn one way to get user input in Java. Introduce the Scanner object: Declare the variable that stores the user input: An example prompting the user for input: Methods you need to invoke in order to properly get  user input:
Viewers will learn how to properly install Eclipse with the necessary JDK, and will take a look at an introductory Java program. Download Eclipse installation zip file: Extract files from zip file: Download and install JDK 8: Open Eclipse and …

856 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