Having implemented something ~similar in a library (auto-assigning
auto-increment unique IDs in a distributed application for HBase), I
took a look out of curiosity.

Given the synchronization in your class, I gather that this singleton
is going to be used from multiple threads and thus needs to be
thread-safe.

The first line of get() accesses tableCurrentMax without synchronization.
If you need a concurrent map, use
`java.util.concurrent.ConcurrentHashMap'.  If you're not familiar with
it, I recommend you read
http://developers.sun.com/learning/javaoneonline/sessions/2009/pdf/TS-5217.pdf
(slides 29-32 especially)
getPool and getHbaseConfiguration aren't thread safe since they don't
do any synchronization.  This can lead to your code crashing in
unexpected ways because you can observe a partially constructed object
and other weirdnesses like that.
If you want only a single row out of the scanner, you should call the
confusingly named setCaching method on the scan object like so:
scan.setCaching(1) – otherwise, depending on your Configuration loaded
by getHbaseConfiguration(), your scanner may end up fetching any
number of rows (possibly *many* rows) and returning just the first one
to you.
I don't see why you use MutableLong instead of
`java.util.concurrent.atomic.AtomicLong' as I believe the latter would
allow you to remove a lot of the synchronization.
You acquire a RowLock but I don't see where you release it.

I don't think you can run multiple instances of this web service at
the same time.  If you think you do, I believe there's a race
condition.
Say you have 2 instances, service A and service B.
A starts, receives a request, scans the table to discover that the
last ID assigned was 43.
In parallel, B starts, receives a request, scans the table and
discovers 43 as well.
Now A decides to assign the next ID, 42, and calls `exists' to verify
that there's no row for 42.  `exists' returns false so A proceeds to
lock this row.
In parallel, B does the same thing and attempts to lock the row too.
So now both A and B are racing to lock the same row.  Let's say that
the RegionServer gives the RowLock to A, so B gets stuck waiting until
the lock is released.
I don't see in your code where you would create the row for 42, but
let's assume A creates it and releases the RowLock.
B now gets unblocked, acquires the RowLock, and returns the ID 42 too.  Bang.

Am I missing something?

-- 
Benoit "tsuna" Sigoure
Software Engineer @ www.StumbleUpon.com

Reply via email to