Using CountDownLatch for blocking the reads if writes are happening

Question

I am trying to implement lock by which I want to avoid reads from happening whenever I am doing a write on my three maps.

Requirements:

• Reads block until all three maps have been set for the first time.
• Now second time, If I am updating the maps, I can still return all the three old maps value(before the updates are done on all three maps) or it should block and return me all the new three maps value whenever the updates are done on all the three maps.

As I have three Maps - primary , secondary and tertiary so it should return either all the new values of three updated maps or it should return all the old values of the map. Basically, while updating I don't want to return primary having old values, secondary having having new values, and tertiary with new values.

It should be consistent, either it should return old values or it should return new values after/while updating the maps. In my case, updating of maps will happen once in three months or four months.

And from the main application thread, reads will be happening on the getter of those three maps at a rate of 1000 requests per second. So the get call on the three maps are really performance critical. I was thinking to use ReentrantLock or ReentrantReadWriteLock but got stick to CountDownLatch because of simplicity-

Below is my ClientData class in which I am using CountDownLatch -

public class ClientData {

    public static class Mappings {
        public final Map<String, Map<Integer, String>> primary;
        public final Map<String, Map<Integer, String>> secondary;
        public final Map<String, Map<Integer, String>> tertiary;

        public Mappings(
            Map<String, Map<Integer, String>> primary,
            Map<String, Map<Integer, String>> secondary,
            Map<String, Map<Integer, String>> tertiary
        ) {
            this.primary = primary;
            this.secondary = secondary;
            this.tertiary = tertiary;
        }
    }

    private static final AtomicReference<Mappings> mappings = new AtomicReference<>();
    private static final CountDownLatch hasBeenInitialized = new CountDownLatch(1);

    public static Mappings getMappings() {
        try {
            hasBeenInitialized.await();
            return mappings.get();
        } catch (InterruptedException e) {
            Thread.currentThread().interrupt();
            throw new IllegalStateException(e);
        }
    }

    public static void setMappings(
        Map<String, Map<Integer, String>> primary,
        Map<String, Map<Integer, String>> secondary,
        Map<String, Map<Integer, String>> tertiary
    ) {
        setMappings(new Mappings(primary, secondary, tertiary));
    }

    public static void setMappings(Mappings newMappings) {
        mappings.set(newMappings);
        hasBeenInitialized.countDown();
    }
}

And 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 in those three maps as shown above.

public class TempScheduler {

    private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);

        public void startScheduler() {
            final ScheduledFuture<?> taskHandle = scheduler.scheduleAtFixedRate(new Runnable() {
                public void run() {
                try {
                    callServers();
                } catch (Exception ex) {
                    ex.printStackTrace();
                }
                }
            }, 0, 10, TimeUnit.MINUTES);
        }
    }

    // call the servers and get the data and then parse 
    // the response.
    private void callServers() {
        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) {
        //...       
        ConcurrentHashMap<String, Map<Integer, String>> primaryTables = null;
        ConcurrentHashMap<String, Map<Integer, String>> secondaryTables = null;
        ConcurrentHashMap<String, Map<Integer, String>> tertiaryTables = null;

        //...

        // store the data in ClientData class variables if anything has changed  
        // which can be used by other threads
        if(changed) {
            ClientData.setMappings(primaryTables, secondaryTables, tertiaryTables);
        }
    }
}

Problem Statement:

Is there any way I can improve the above ClientData class somehow? Any kind of performance improvements? Or by using any other ways instead of CountDownLatch?

I will be using ClientData class in main application thread like this -

Mappings mappings = ClientData.getMappings(); 
// use mappings.primary 
// use mappings.secondary 
// use mappings.tertiary

I am suspecting, I can avoid AtomicReference here -

The atomic reference may not actually be required since both get and set operations synchronize on hasBeenInitialized.

Answer 1

Going through some smaller items in your code. Your Mappings class is public, which is fine, but it should also be final. There is no reason for people to extend your class. The constructor should be private, and all the Map fields should be private as well, but you should have getters for those maps... getPrimary(), getSecondary(), etc.

On the ClientData class, the method setMappings(Mappings...) content should be merged in to the setMappings(Map ..., Map.. Map) method, and that method should not be public either, assuming the background thread is defined in the same package as you.

Restricting these permissions will make your application better defined.

The remaining concern with your code is whether the Map instances themselves are Mutable (well, they are mutable, but should they be?). I would recommend making them readonly using Collections.unmodifiableMap() which will prevent the actual Map content from becoming corrupted by some thread polluting the data.