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 >> > > > > >> > > > >> > > >> > >> >
