[CONTROLLER-722] ShardWriteTransaction needs to synchronize access to its SnapshotBackedWriteTransaction Created: 23/Aug/14 Updated: 24/Aug/14 Resolved: 24/Aug/14 |
|
| Status: | Resolved |
| Project: | controller |
| Component/s: | mdsal |
| Affects Version/s: | Helium |
| Fix Version/s: | None |
| Type: | Bug | ||
| Reporter: | Tom Pantelis | Assignee: | Tom Pantelis |
| Resolution: | Cannot Reproduce | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Operating System: All |
||
| External issue ID: | 1609 |
| Description |
|
The SnapshotBackedWriteTransaction has no internal synchronization and thus is not safe for multiple-thread access, concurrent or not. This is the intention for performance reasons - let the client synchronize it if they need to (like the JDK collections classes do). ShardWriteTransaction actor receives write/merge/delete/ready messages via akka and, internally, it has a SnapshotBackedWriteTransaction that each message modifies. While messages are not sent concurrently by akka and updates in the SnapshotBackedWriteTransaction will not occur concurrently, they may be sent serially by multiple akka threads. One aspect of multi-threading in java and the java memory model is ensuring that writes done one thread are published or made visible to other threads. This requires some form of synchronization (i.e. synchronized block, java.concurrent.Lock, volatile keyword, final keyword on instance fields, thread enter/exit ...). We could simply synchronize on the SnapshotBackedWriteTransaction instance but this carries the exclusive lock overhead that we don't need as we won't have concurrent access. Instead, we could use a separate volatile variable as a read/write memory barrier "guard" for publishing writes across threads, as explained here by this snippet in Java Concurrency in Practice: "The visibility effects of volatile variables extend beyond the value of the volatile variable itself. When thread A writes to a volatile variable and subsequently thread B reads that same variable, the values of all variables that were visible to A prior to writing to the volatile variable become visible to B after reading the volatile variable." |
| Comments |
| Comment by Tom Pantelis [ 24/Aug/14 ] |
|
In reading akka's documentation, specifically http://doc.akka.io/docs/akka/snapshot/general/jmm.html, akka defines the following happens-before rule: "The actor subsequent processing rule: processing of one message happens before processing of the next message by the same actor." "This means that changes to internal fields of the actor are visible when the next message is processed by that actor. So fields in your actor need not be volatile or equivalent." So akka takes care of synchronization of actor internal state between messages. Makes sense as akka's main premise is to "alleviate the developer from having to deal with explicit locking and thread management". This bug is not needed - closing. |