patternjavaMinor
Lock for preventing concurrent access in client data
Viewed 0 times
dataclientforpreventingconcurrentaccesslock
Problem
I am trying to implement lock by which I don't want to have reads from happening whenever I am doing a write.
Here is my
I need to wait on the
I decided
Here is my
ClientData class in which I am using CountDownLatch:public class ClientData {
private static final AtomicReference>> primaryMapping = new AtomicReference<>();
private static final AtomicReference>> secondaryMapping = new AtomicReference<>();
private static final AtomicReference>> tertiaryMapping = new AtomicReference<>();
// should this be initialized as 1?
private static final CountDownLatch hasBeenInitialized = new CountDownLatch(1)
public static Map> getPrimaryMapping() {
try {
hasBeenInitialized.await();
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
return primaryMapping.get();
}
public static void setPrimaryMapping(Map> map) {
primaryMapping.set(map);
hasBeenInitialized.countDown();
}
public static Map> getSecondaryMapping() {
try {
hasBeenInitialized.await();
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
return secondaryMapping.get();
}
public static void setSecondaryMapping(Map> map) {
secondaryMapping.set(map);
hasBeenInitialized.countDown();
}
public static Map> getTertiaryMapping() {
try {
hasBeenInitialized.await();
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
return tertiaryMapping.get();
}
public static void setTertiaryMapping(Map> map) {
tertiaryMapping.set(map);
hasBeenInitialized.countDown();
}
}I need to wait on the
get calls on three AtomicReferences I have. Once all the writes has been done on the three AtomicReferences I have with the set call, I'll allow making the call to three getters which I have.I decided
Solution
The fact that you must be able to swap in a new set of maps at any time--even if only once every three months--requires a design change. Here are the requirements as I understand them:
Taken together, you really must combine the maps into a new data structure which is returned to clients and replaced in whole by the background thread. Storing it in its own atomic reference allows this new class to avoid concurrency and locks altogether.
Update
Now let's throw some code behind the above changes.
There are some other improvements I would suggest but did not implement.
- Reads block until all three maps have been set the first time.
- Reads receive a consistent set of maps. In other words, you cannot return the old secondary map after returning the new primary map.
Taken together, you really must combine the maps into a new data structure which is returned to clients and replaced in whole by the background thread. Storing it in its own atomic reference allows this new class to avoid concurrency and locks altogether.
- Keep the latch to block reads before the first set of maps have been loaded.
- Combine the three maps in a simple data holder.
- Store the data holder in an atomic reference (if necessary) instead of each map individually.
- Set all three maps in one call by storing a new data holder instance. Since all three are stored in one call, the latch can use an initial count of 1.
Update
Now let's throw some code behind the above changes.
public class ClientData {
public static class Mappings {
public final Map> primary;
public final Map> secondary;
public final Map> tertiary;
public Mappings(
Map> primary,
Map> secondary,
Map> tertiary
) {
this.primary = primary;
this.secondary = secondary;
this.tertiary = tertiary;
}
}
private static final AtomicReference 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(); // @rolfl covered this
throw new IllegalStateException(e);
}
}
public static void setMappings(
Map> primary,
Map> secondary,
Map> tertiary
) {
setMappings(new Mappings(primary, secondary, tertiary));
}
public static void setMappings(Mappings newMappings) {
mappings.set(newMappings);
hasBeenInitialized.countDown();
}
}There are some other improvements I would suggest but did not implement.
- Avoid using static methods when possible to ease testing and alternate implementations.
- The atomic reference may not actually be required since both get and set operations synchronize on
hasBeenInitialized.
- If clients of
ClientDatacan handle receiving empty mappings, you can drop the latch and initialize the class with three empty maps. This would reduce all synchronization to the atomic reference.
Code Snippets
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(); // @rolfl covered this
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();
}
}Context
StackExchange Code Review Q#48442, answer score: 7
Revisions (0)
No revisions yet.