patternjavaMinor
Having full atomicity against all the threads without impacting performance or throughputs
Viewed 0 times
withoutfullatomicitytheallthreadshavingagainstimpactingthroughputs
Problem
I am working on a project in which I construct a URL with a valid hostname (but not a blocked hostname) and then execute that URL using
So if any block list of hostnames is present, then I won't make a call to that hostname from the main thread and I will try making a call to another hostname.
By block list, I mean whenever any server is down, its hostname is on the block list.
Below is my background thread code. It will get the data from my service URL and keep on running every 10 minutes once my application has started up. It will then parse the data coming from the URL and store it in a
TempScheduler
Below is my
RestTemplate from my main application thread. I also have a single background thread in my application which parses the data from the url and extracts the block list of hostnames from it.So if any block list of hostnames is present, then I won't make a call to that hostname from the main thread and I will try making a call to another hostname.
By block list, I mean whenever any server is down, its hostname is on the block list.
Below is my background thread code. It will get the data from my service URL and keep on running every 10 minutes once my application has started up. It will then parse the data coming from the URL and store it in a
ClientData class variable -TempScheduler
public class TempScheduler {
// .. scheduledexecutors service code to start the background thread
// call the service and get the data and then parse
// the response.
private void callServiceURL() {
String url = "url";
RestTemplate restTemplate = new RestTemplate();
String response = restTemplate.getForObject(url, String.class);
parseResponse(response);
}
// parse the response and store it in a variable
private void parseResponse(String response) {
//...
// get the block list of hostnames
Map> coloExceptionList = gson.fromJson(response.split("blocklist=")[1], Map.class);
List blockList = new ArrayList();
for(Map.Entry> entry : coloExceptionList.entrySet()) {
for(String hosts : entry.getValue()) {
blockList.add(hosts);
}
}
// store the block list of hostnames which I am not supposed to make a call
// from my main application
ClientData.replaceBlockedHosts(blockList);
}
}Below is my
ClientData class. replaceBlockedHosts method will oSolution
Your code is thread-safe as it is. [But not really... see the comments below.]
-
Be careful with method and variable names.
-
Try to have each method doing only one thing. For example,
-
-
Instead of either
-
Your non-class
-
Be careful with method and variable names.
TempScheduler does not really have anything temporary. callServiceURL does a lot more than calling a URL. blockHost(hostname) would be better as addToBlockedHosts(hostname). call is non-descriptive. ClientData only contains information related to host names.-
Try to have each method doing only one thing. For example,
callServiceURL should just return some json. Then parseResponse should take that json and return a list of host names. Finally, some other functions should add the host names to the set of blocked hosts.-
volatile has the same effect as AtomicReference and makes the code simpler. See this post.-
Instead of either
AtomicReference or volatile you could just have a final ConcurrentHashMap which you would clear when you receive a new list.-
Your non-class
ClientData with only static methods makes me cringe. Please make it a normal class. Create an instance of ClientData in the main thread and pass it to the secondary thread when you create it.Context
StackExchange Code Review Q#61536, answer score: 5
Revisions (0)
No revisions yet.