;-) For backward compatibility it would be better to keep the existing behaviour and add a method, but I by default I would be lazy here and just fix a behaviour that seems broken.
On Wed, Jul 17, 2013 at 7:06 PM, Lars George <[email protected]> wrote: > Hi Nicholas, > > Knock yourself out :) > > I would either call deleteConnection() with the force flag set to true > (like deleteStaleConnection does) or check if the connection is closed - > using isClosed() - and only if it is remove it from the internal list. > > Best would be to expose the force flag to the users with an overloaded > deleteAllConnections() plus the above part of not removing not closed > connections. > > Lars > > > > On Jul 17, 2013, at 6:31 PM, Nicolas Liochon <[email protected]> wrote: > > > I had a look, I think it's a bug. > > If the connection holds resources that should be freed, well they won't > be > > freed. > > It's used in the minicluster shutdown. So we could imagine scenarios > which > > that create some test flakyness. I doubt it happens in real life, but > imho > > it's better to fix it (it there are no different opinions, I will do it > if > > you don't ;-). > > > > Cheers, > > > > Nicolas > > > > > > On Wed, Jul 17, 2013 at 10:03 AM, Lars George <[email protected]> > wrote: > > > >> Hi, > >> > >> I am working on an issue around Thrift 2 (HBASE-7035), and I am testing > >> how this all works a bit. See > >> https://github.com/larsgeorge/hbase-scanner-test for the code, which > runs > >> against a local HBase instance. Now, there is an issue that connections > do > >> not get closed as I would have expected. I close the connection in a > loop > >> over a scanner that gets single rows. Although I close it, it keeps on > >> going. Scanners may or may not be reference counted - any code calling > >> HCM.getConnection(conf) increases the count. > >> > >> So I tried a HCM.deleteAllConnections, which does this: > >> > >> public static void deleteAllConnections() { > >> synchronized (HBASE_INSTANCES) { > >> Set<HConnectionKey> connectionKeys = new HashSet<HConnectionKey>(); > >> connectionKeys.addAll(HBASE_INSTANCES.keySet()); > >> for (HConnectionKey connectionKey : connectionKeys) { > >> deleteConnection(connectionKey, false); > >> } > >> HBASE_INSTANCES.clear(); > >> } > >> } > >> > >> It iterates over all connections and "deletes" them. Then is clears the > >> entire reference list! The issue is that deleteConnection() is using the > >> refcounts and is *not* closing the connection when it is still > referenced. > >> The final clear() call simply drops them from the list of managed > >> connections. This means that we now might have dangling open > connections, > >> and unless you hold a reference there is no way that you can talk to it > >> again, not even using deleteStaleConnection() for example. > >> > >> Is that wanted? Just curious, since obviously this is not a biggie in > real > >> life. > >> > >> Lars > >> > >> > >> > >> > >> > >> >
