Gary, Todd: Please take a look at patch v4 for HBASE-4508 and give feedback on the next step.
Thanks On Thu, Sep 29, 2011 at 6:52 AM, Ted Yu <[email protected]> wrote: > Thanks for being considerate, Bright. > > For this new parameter, instead of hbase.client.per.config.inst, can we > name it 'hbase.connection.per.config' ? > Basically from its description, connection sharing is concerned. > > Cheers > > > On Thu, Sep 29, 2011 at 6:47 AM, Bright Fulton <[email protected]>wrote: > >> Gary, Todd, Ted, although 0.90.3 hbase-rest (CDH3u0, u1) leaks ZK >> connections on every request (via HBaseAdmin ctor), preserving client >> behavior in a point release is a good idea, so for discussion I've >> included an hbase.client.per.config.inst param which defaults to true >> in the v1 patch attached to HBASE-4508. >> >> Bright >> >> >> On Wed, Sep 28, 2011 at 6:22 PM, Gary Helmling <[email protected]> >> wrote: >> > Hi Ted, >> > >> > Thanks for pointing it out. Looking through the patch I did see that >> > forcing separate connections was supported by tweaking the instance ID >> > value. The problem I'm pointing out is not that it can't be done, but >> that >> > it would require code changes on the user's part. As an HBase user, >> this is >> > not what I would expect when doing a minor version upgrade. >> > >> > Admittedly, the scenario I'm calling out is likely to be rare (maybe no >> one >> > at all is doing it). But it is valid. >> > >> > I don't want us to be ossified by compatibility concerns or preserve >> poor >> > behavior because that's the way it was previously done. On the other >> hand, >> > I think we should only be making incompatible changes in a minor release >> if >> > there are extremely compelling reasons for doing so. The fact that >> there >> > are many installs successfully running 0.90 without this patch makes me >> > question whether or not it's extremely compelling. >> > >> > Are there other fixes that this patch provides to connection handling >> > (outside of the HConnectionKey identity) that are otherwise broken in >> 0.90 >> > (previous connection leaks, etc)? >> > >> > I completely agree that HBASE-3777 is a better approach to connection >> > handling than the previous object identity behavior and I think it >> creates a >> > better end user experience. But it is a disruptive change. >> > >> > At the very least, if this change goes in, it must be clearly flagged as >> an >> > incompatible change with a explanation of the changed behavior in >> release >> > notes. >> > >> > --gh >> > >> > >> > On Wed, Sep 28, 2011 at 2:51 PM, Ted Yu <[email protected]> wrote: >> > >> >> Gary: >> >> Karthick and I devised the following >> (HConstants.HBASE_CLIENT_INSTANCE_ID) >> >> for the scenario you listed below: >> >> >> >> /** >> >> * Parameter name for unique identifier for this {@link Configuration} >> >> * instance. If there are two or more {@link Configuration} instances >> >> that, >> >> * for all intents and purposes, are the same except for their >> instance >> >> ids, >> >> * then they will not be able to share the same {@link Connection} >> >> instance. >> >> * On the other hand, even if the instance ids are the same, it could >> >> result >> >> * in non-shared {@link Connection} instances if some of the other >> >> connection >> >> * parameters differ. >> >> */ >> >> public static String HBASE_CLIENT_INSTANCE_ID = " >> hbase.client.instance.id >> >> "; >> >> >> >> FYI >> >> >> >> On Wed, Sep 28, 2011 at 12:06 PM, Gary Helmling <[email protected]> >> >> wrote: >> >> >> >> > Changing the connection identity behavior in the middle of a release >> >> series >> >> > seems like a bad idea. >> >> > >> >> > The 0.20 releases did connection identity based on Configuration >> >> contents, >> >> > 0.90 changed this to Configuration instance identity, then 0.90.5 >> would >> >> be >> >> > going back to contents again (acknowledged with a smarter subset and >> >> guards >> >> > against changes)? If anyone running 0.90 relies on the current >> behavior >> >> to >> >> > enforce separate connections (for whatever reason), using separate >> >> > Configuration instances, this would break that behavior and appear as >> a >> >> > regression right? >> >> > >> >> > Changing these underlying assumptions in a minor release doesn't seem >> >> > right. I agree it's nice to have the backport for those interested >> in >> >> > trying it. But I'd need some convincing that the current 0.90 >> behavior >> >> is >> >> > completely broken rather than sub-optimal to agree to include it. >> >> > >> >> > --gh >> >> > >> >> > >> >> > On Wed, Sep 28, 2011 at 9:55 AM, Ted Yu <[email protected]> wrote: >> >> > >> >> > > One reason for my endorsement is that it would take 0.92 quite some >> >> time >> >> > to >> >> > > reach the level of stability of 0.90.4 >> >> > > I really think HBASE-3777 would benefit HBase users a lot, and >> reducing >> >> > > potential future inquiry about connection-related issues. >> >> > > >> >> > > Of course, backporting increases the amount of work for validation >> of >> >> > > 0.90.5 >> >> > > But I think it is worth it. >> >> > > >> >> > > My two cents. >> >> > > >> >> > > On Wed, Sep 28, 2011 at 9:47 AM, Jean-Daniel Cryans < >> >> [email protected] >> >> > > >wrote: >> >> > > >> >> > > > I'm -0 at the moment, it's a big patch to include in a point >> release. >> >> > > > >> >> > > > I'm glad the work was done tho because it means those interested >> >> (like >> >> > > > me) can directly patch it in and test it (at my own risk). >> >> > > > >> >> > > > J-D >> >> > > > >> >> > > > On Wed, Sep 28, 2011 at 9:29 AM, Ted Yu <[email protected]> >> wrote: >> >> > > > > Hi, >> >> > > > > Bright Fulton has volunteered to backport HBASE-3777 to 0.90 >> >> > > > > I endorse his effort. >> >> > > > > >> >> > > > > If you have comment(s), please share. >> >> > > > > >> >> > > > > I will open a new JIRA for this effort if this motion passes. >> >> > > > > >> >> > > > > Thanks >> >> > > > > >> >> > > > >> >> > > >> >> > >> >> >> > >> > >
