hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Samarth Jain (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-18359) CoprocessorHConnection#getConnectionForEnvironment should read config from CoprocessorEnvironment
Date Fri, 17 Nov 2017 19:47:00 GMT

    [ https://issues.apache.org/jira/browse/HBASE-18359?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16257476#comment-16257476
] 

Samarth Jain commented on HBASE-18359:
--------------------------------------

bq. On older versions also it is not as the CoprocessorHConnection#getConnectionForEnvironment
was taking a CoprocessorEnvironment instance not config

The issue is present in older versions. Instead of using the env passed in, the code was using
the environment of HRegionServer which is what the description also talks about.
{code
if (env instanceof RegionCoprocessorEnvironment) {
      RegionCoprocessorEnvironment e = (RegionCoprocessorEnvironment) env;
      RegionServerServices services = e.getRegionServerServices();
      if (services instanceof HRegionServer) {
        return new CoprocessorHConnection((HRegionServer) services);
      }
    }
{code}

bq. What we need in HBase is an API CoprocessorEnvironment#getConnection(Config). This call
will always make new connection (which is short circuit enabled). So caching of this and reuse
the callee has to take care. Is that ok?

I think that should work, [~anoop.hbase]. We already cache the HConnection in CoprocessorHConnectionTableFactory
by doing this: 
{code}
private synchronized HConnection getConnection(Configuration conf) throws IOException {
            if (connection == null || connection.isClosed()) {
                connection = new CoprocessorHConnection(conf, server);
            }
            return connection;
        }
{code}
It would be good if the API has explicit documentation saying it is the caller's responsibility
to make sure the connection returned by the getConnection(config) API is appropriately closed.


> CoprocessorHConnection#getConnectionForEnvironment should read config from CoprocessorEnvironment
> -------------------------------------------------------------------------------------------------
>
>                 Key: HBASE-18359
>                 URL: https://issues.apache.org/jira/browse/HBASE-18359
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Samarth Jain
>             Fix For: 2.0.0
>
>
> It seems like the method getConnectionForEnvironment isn't doing the right thing when
it is creating a CoprocessorHConnection by reading the config from HRegionServer and not from
the env passed in. 
> If coprocessors want to use a CoprocessorHConnection with some custom config settings,
then they have no option but to configure it in the hbase-site.xml of the region servers.
This isn't ideal as a lot of times these "global" level configs can have side effects. See
PHOENIX-3974 as an example where configuring ServerRpcControllerFactory (a Phoenix implementation
of RpcControllerFactory) could result in deadlocks. Or PHOENIX-3983 where presence of this
global config causes our index rebuild code to incorrectly use handlers it shouldn't.
> If the CoprocessorHConnection created through getConnectionForEnvironment API used the
CoprocessorEnvironment config, then it would allow co-processors to pass in their own config
without needing to configure them in hbase-site.xml. 
> The change would be simple. Basically change the below
> {code}
> if (services instanceof HRegionServer) {
>         return new CoprocessorHConnection((HRegionServer) services);
> }
> {code}
> to
> {code}
> if (services instanceof HRegionServer) {
>         return new CoprocessorHConnection(env.getConfiguration(), (HRegionServer) services);
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message