6

I have a stateful bean in an multi-threaded enviroment, which keeps its state in a map. Now I need a way to replace all values of that map in one atomic action.

public final class StatefulBean {

    private final Map<String, String> state = new ConcurrentSkipListMap<>();

    public StatefulBean() {
        //Initial state
        this.state.put("a", "a1");
        this.state.put("b", "b1");
        this.state.put("c", "c1");
    }

    public void updateState() {
        //Fake computation of new state
        final Map<String, String> newState = new HashMap<>();
        newState.put("b", "b1");
        newState.put("c", "c2");
        newState.put("d", "d1");

        atomicallyUpdateState(newState);
        /*Expected result
         *  a: removed
         *  b: unchanged
         *  C: replaced
         *  d: added*/
    }

    private void atomicallyUpdateState(final Map<String, String> newState) {
        //???
    }
}

At the moment I use ConcurrentSkipListMap as implementation of a ConcurrentMap, but that isn't a requirement.

The only way I see to solve this problem is to make the global state volatile and completely replace the map or use a AtomicReferenceFieldUpdater. Is there a better way?

My updates are quite frequent, once or twice a second, but chance only very few values. Also the whole map will only ever contain fewer than 20 values.

Almas Abdrazak
  • 3,209
  • 5
  • 36
  • 80
Jan
  • 173
  • 2
  • 9
  • Might it be easier to build a new map with the updated values and then swap it in atomically? Depends on whether your client code retains references to the map or the bean, I guess? – brabster May 14 '18 at 13:07
  • Do you need `updateState` to be atomic? – schaffe May 14 '18 at 13:09
  • @schaffe no, only the `atomicallyUpdateState` to be atomic – Jan May 14 '18 at 13:12
  • What race condition do you think to avoid by swapping atomically? – bowmore May 14 '18 at 13:13
  • @brabster Only the bean will be referenced, the map itself will be totally private It feels kind of wrong to build and swap the whole map to only update a few values. But maybe my feeling is wrong and replacing is the best way. – Jan May 14 '18 at 13:15
  • @bowmore I try to avoid an inconsistent state: `this.state = new HashMap(newState);` I fear that between creating and assigning the new state the thread gets interupted and a different thread replaces some values, which than will be reverted when the first thread continues – Jan May 14 '18 at 13:19
  • Currently this.state is final, you cannot reassign it. It's also not clear if the entirety of the map is exposed (is there a getter or so that returns the Map?). So I'm still not clear about what problem you try to avoid. Somehow you imply that the different entries have a semantic relationship that matters to clients of the class, which? – bowmore May 14 '18 at 13:28
  • @Jan then you need a way to lock access to the map for the time of update. Locking is a simplest approach, but it is also possible to apply versioning and CAS here. – schaffe May 14 '18 at 13:30
  • @Jan replacing the value (i.e. the whole map) would seem to be the simplest solution and unless there's any significant performance concerns (although using locking is likely to perform worse and less predictably unless the map is huge) I'd try that before anything requiring more advanced knowledge. It's how a functional programmer would do it. - added as an answer. – brabster May 14 '18 at 13:32
  • @bowmore The map itself will be hidden from its clients, values will be only accessed via "value getter" `public String getValue(String key) {...}` While there are lot of updates to the map, singular values might only be updated after minutes or even hours. The values will be used for further computations and decisions in my programm: If an allready updated value is replaced through an older version because of a racecondition, that would leave my programm in an incorrect state for quite some time I can change the `StatefullBean` and make the map non-`final`. – Jan May 14 '18 at 13:40
  • I assume that a client calling `getValue()` with two different keys in succession is supposed to get values of the same state? However if you internally change the state (atomic or otherwise) you cannot guarantee that. You can't, because the (atomic) state change can happen between the two `getValue()` calls. – bowmore May 14 '18 at 15:28

6 Answers6

2

The simplest, least fuss method is to switch the map instead of replacing map contents. Whether using volatile or AtomicReference (I don't see why you'd need AtomicReferenceFieldUpdater particularly), shouldn't make too much of a difference.

This makes sure that your map is always in proper state, and allows you to provide snapshots too. It doesn't protect you from other concurrency issues though, so if something like lost updates are a problem you'll need further code (although AtomicReference would give you CAS methods for handling those).

The question is actually rather simple if you only consider the complete atomic replacement of the map. It would be informative to know what other operations affect the map and how. I'd also like to hear why ConcurrentSkipListMap was chosen over ConcurrentHashMap.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • believe or not, we have such a case where we can't really do anything about the function chaining.. (long story), but the short version is that `map.computeIfAbsent("a", x -> { return map.computeIfAbsent("a", y -> 1); });` for `CHM` will fail, but it will not for the other – Eugene May 14 '18 at 14:29
  • @Eugene I see. Sounds interesting. – Kayaman May 14 '18 at 18:45
2

Approach with CAS and AtomicReference would be to copy map content on each bulk update.

AtomicReference<Map<String, String>> workingMapRef = new AtomicReference<>(new HashMap<>());

This map can be concurrent, but for "bulk updates" it is read-only. Then in updateState looping doUpdateState() until you get true and that means that your values has been updated.

void updateState() {
    while (!doUpdateState());
}

boolean doUpdateState() {
    Map<String, String> workingMap = workingMapRef.get();
    //copy map content
    Map<String, String> newState = new HashMap<>(workingMap); //you can make it concurrent

    newState.put("b", "b1");
    newState.put("c", "c2");
    newState.put("d", "d1");

    return workingMapRef.compareAndSet(workingMap, newState);
}
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
schaffe
  • 399
  • 2
  • 16
0

Extend a map implementation of your choice and add a synchronized method:

class MyReplaceMap<K, V> extends HashMap<K, V> //or whatever
{
    public synchronized void replaceKeys(final Map<K, V> newMap)
    {
        //.. do some stuff
    }
}

Of course, you could always make state non-final volatile and re-assign it (assignment is atomic)

private volatile Map<String, String> state = new HashMap<>();

//...

final Map<String, String> newState = new HashMap<>();
newState.put("b", "b1");
newState.put("c", "c2");
newState.put("d", "d1");
state = newState;
Michael
  • 41,989
  • 11
  • 82
  • 128
  • I *think* the OP wants to block reads, until the entire map has been updated, so if there is an update process, block reads, until updates happened, but this is just a guess – Eugene May 14 '18 at 14:39
0

Since the map is quite small, it's probably enough to just use synchronized in all places you access it.

private void atomicallyUpdateState(final Map<String, String> newState) {
    synchronized(state) {
        state.clear();
        state.putAll(newState);
    }
}

but don't forget any, like all occurances of things like

String myStatevalue = state.get("myValue");

need to become

String myStatevalue;
synchronized (state) {
    myStatevalue = state.get("myValue");
}

otherwise the read and update are not synchronized and cause a race condition.

daniu
  • 14,137
  • 4
  • 32
  • 53
0

As client code maintains a reference to the bean not the map, replacing the value (i.e. the whole map) would seem to be the simplest solution.

Unless there's any significant performance concerns (although using locking is likely to perform worse and less predictably unless the map is huge) I'd try that before anything requiring more advanced knowledge.

It's how a functional programmer would do it.

brabster
  • 42,504
  • 27
  • 146
  • 186
0

Use ReadWriteLock can help to automically replace all values in a Map.

private static final ReadWriteLock LOCK = new ReentrantReadWriteLock();

private void atomicallyUpdateState(final Map<String, String> newState) {
    LOCK.writeLock().lock();
    try {
        state.clear();
        state.putAll(newState);
    } finally {
        LOCK.writeLock().unlock();
    }
}
TongChen
  • 1,414
  • 1
  • 11
  • 21