Concurrency design principles in practice
I have a Results object that is being written by multiple threads at the same time. However, each thread has a specific purpose and owns specific fields, so that no data is actually changed by more than one thread. The consumer of this data will not try to read it until all of the writer's records start writing it. Since I know this to be true, there is no synchronization when writing and reading data.
There is a RunningState object associated with this Results object to coordinate this work. All his methods are synchronized. When a thread is executing with its work on this Results object, it calls done () on the RunningState object, which does the following: decrements the counter, checks if the counter has passed to 0 (which indicates that all authors are done), and if it is so, puts this object on a parallel queue. This queue is consumed by the ResultsStore server, which reads all the fields and stores the data in the database. Before reading any data, the ResultsStore object calls RunningState.finalizeResult (), which is an empty method whose sole purpose is to keep the RunningState object in sync to ensure that records from all streams are visible to the reader.
Here are my problems:
1) I believe this will work correctly, but I feel like I'm breaking good design guidelines to not sync data changes to an object that is shared by multiple threads. However, if I had to add synchronization and / or separate things so that each thread only sees the data it is responsible for, it would complicate the code. Anyone modifying this area has a better understanding of what's going on anyway, or they might break something, so from a maintenance standpoint, I think a simpler code with good comments explaining how it works is the best way to go ...
2) The fact that I need to call this do-nothing method seems to be an indication of wrong design. It?
The views were appreciated.
source to share
Your design sounds sound, but it can be improved if you are using a real parallel queue, as the parallel queue from the java.util.concurrent package already guarantees what happens before the communication between the thread putting the element in the queue and the thread taking the element so it eliminates the need to call finalizeResult () on the receiving thread (so it doesn't need the call to "do nothing").
From the java.util.concurrent package description:
The methods of all classes in java.util.concurrent and its subpackages extend these guarantees to higher synchronization. In particular:
- The actions on a thread before placing an object in any concurrent assembly occurs before the actions after accessing or removing that item from the collection on another thread.
The comments in the other answer regarding using AtomicInteger instead of synchronization are also sensible (since using AtomicInteger to count the stream will perform better than synchronization), just make sure the count value after the atomic decrement (e.g. decmentAndGet ()) is compared to 0. to avoid being added to the queue twice.
source to share
This seems mostly correct if the bit is fragile (if you change the local stream of one field, for example, you might forget to synchronize it and end up doing hard traces of the data).
A big issue of concern is memory visibility; I don't think you created it. An empty method finalizeResult()
can be synchronized, but if the write streams are also out of sync with what it is syncing to (presumably this
?), There is no-before relationship. Remember that synchronization is not absolute - you are synchronizing relative to other threads that are also synchronizing on the same object. Your do-nothing method really won't do anything, won't even provide any kind of memory barrier.
You need to somehow establish a connection between the events happening between them and the stream that is ultimately reading. One way to do this without synchronization is through a variable volatile
or AtomicInteger
(or other atomic classes).
For example, each writing thread can invoke an counter.incrementAndGet(1)
object, and the reading thread can then check that counter.get() == THE_CORRECT_VALUE
. There is a connection between the volatile / atomic field and its reading, which gives you the visibility you need.
source to share
What you described is really safe, but it also sounds frank, fragile and (as you noticed) maintenance can be a problem. Without a code example, it is very difficult to say what is easiest to understand, so the already subjective question becomes frankly irrefutable. Could you ask a colleague to revise the code? (Especially the one that probably needs to deal with this pattern.) I'll trust you that this is indeed the simplest approach, but something like wrapping synchronized
blocks around records will improve security now and in the future. However, you obviously know your code better than I do.
source to share