So there I was, running a unit test. I step away and hours later it
still had not finished. It was stuck on this line of the code:
putThread.done();
> putThread.join();
So the putThread was not finishing as it was supposed to do. Well,
what does putThread.done() do:
public void done() {
done = true;
synchronized (this) {
interrupt();
}
}
Looks reasonable, wait, what is this access of a variable shared among
multiple threads outside of a sync block? Oh wait, its "ok", it's
volatile:
private volatile boolean done;
And what does the run loop of the thread look like? Again reasonable:
public void run() {
done.set(false);
int val = 0;
while (!done.get()) {
// Do HRegion.put call with some exception handling
}
}
Well in theory this all looks reasonable.
But why harmful?
I had changed the Gets to not acquire row locks, thus removing a
synchronization block between the main thread (which did Gets and then
eventually called that code up top). Two consequences, one was puts
happened much faster (good!), and a negative consequence is sometimes
the test would fail to stop the put thread. For minutes and hours
after the main thread had signalled the put thread to stop, it still
had not.
I switched the volatile boolean => AtomicBoolean and the problem went away.
In short, the use of volatile is not guaranteed to see an update from
a different thread in any timely manner. The Java Memory Model does
not ensure this. If you need to rely on passing data between threads
you need to either use a monitor lock (using synchronized keyword
either at the method level or as an explicit block) or use one of the
concurrent utilities classes - such as AtomicLong, AtomicBoolean, or
the excellent read/write locks.
If you wish to increment a counter from multiple threads, doing this:
volatile int count = 0;
// later:
count++;
will not work. You must use an AtomicInt/Long or use a monitor lock.
This is because the operation "count++" is not atomic, it is actually
3 - get, add, put.
As we code in HBase, we should avoid the use of volatile as much as
possible. The guarantees it makes are not good enough and can cause a
lot of pain figuring this out months after you originally coded it.
