[hbase10] Address #701 by mimicking the same locks from the HBase 0.9…

[manolama]

…8 client in the HBase 10 client.

[allanbank]

Just delete the line. Don't comment out.

[manolama]

doh, thanks.

[allanbank]

#701 asked for this lock to be removed, if I am reading it correctly.

[manolama]

He did mention it but I need confirmation from an HBase dev that the table fetch is thread safe in 1.X. I think it wasn't in the past.

[busbey]

Connection objects in hbase 1.y are threadsafe, so getTable is fine. The returned Table object is not threadsafe, but making them is supposed to be lightweight and done per-thread when needed.

[allanbank]

This synchronized block should not be needed. The closing of the connection is protected by the decrement of the atomic integer.

[manolama]

Good point, yanked the sync.

[allanbank]

Can we merge this try into the try after the "Terminate right now" comment?

[allanbank]

If you merge the try above into this try you would have something like:

if( THREAD_COUNT.getAndIncrement() == 0) {
    String table = getProperties().getProperty(TABLENAME_PROPERTY, TABLENAME_PROPERTY_DEFAULT);
    try {
        if (connection == null) {
            // Initialize if not set up already.
            connection = ConnectionFactory.createConnection(config);

            // Validate the table name.
            final TableName tName = TableName.valueOf(table);
            connection.getTable(tName).getTableDescriptor();
        }
    } catch (java.io.IOException e) {
        throw new DBException(e);
   }
}

[manolama]

Good point, merged em.

[busbey]

Connection objects in hbase 1.y are threadsafe, so getTable is fine. The returned Table object is not threadsafe, but making them is supposed to be lightweight and done per-thread when needed.

[manolama]

Yanked the lock so we should be good now @busbey, thanks!

[busbey]

this needs to be in a lock that covers checking if it's null and setting it to null, othewise we race the initialization above that only creates a new connection when it's null.

[manolama]

But Alan was right in that this should only fire when the atomic int returns a 0 at which point only one thread will perform the null check and close + null the connection.

Or are you thinking of the case where there may be some initialization issue wherein a run initializes multiple threads but the initializations fail and possibly cleanup is executed on a thread before initialization begins in another thread?

[busbey]

Right exactly.