HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Having full atomicity against all the threads without impacting performance or throughputs

Submitted by: @import:stackexchange-codereview··
0
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 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 o

Solution

Your code is thread-safe as it is. [But not really... see the comments below.]

-
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.