hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anoop Sam John (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-18359) CoprocessorHConnection#getConnectionForEnvironment should read config from CoprocessorEnvironment
Date Thu, 16 Nov 2017 04:29:00 GMT

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

Anoop Sam John commented on HBASE-18359:

[~samarth.jain@gmail.com], [~sergey.soldatov].   Coming to this jira again.
In trunk, things are changed a lot and there is no CoprocessorHConnection any more.   We have
CoprocessorEnvironment#getConnection(). This will give  short circuited connection. Means
any calls, via this connection , on regions in same RS will go a short circuited path.
On the old code u mentioned, we were passing CoprocessorEnvironment. This is the object u
r getting from HBase core and you can just GET config from that. This is the same config that
Region uses.  So am not sure how u can create custom conf. You can not really set any config
in CoprocessorEnvironment object.  I guess what u were doing is just changing /setting some
config attributes on the config getting from CoprocessorEnvironment.  That will be wrong.
Because the conf object is a shared one for every thing in this Region. All CPs over this
Region and Region, Store and level down will use this changed config.  What ideally should
be done is clone the config and set the new attributes. If we were having an API which takes
Config, it would have been possible. Also CoprocessorHConnection was a Private class.
So this is not the ideal path for what you want to do.
Checking the new trunk code also, seems you can not do what u really want. Because all calls
CoprocessorEnvironment#getConnection() will return a same Cluster connection object which
is created in HRS.   So if u need to have custom configs to be used for these connection,
only way would be to have a CoprocessorEnvironment#getConnection(Config) API and create new
connection (Short circuited) in the impl.  Not good to create the connection on every call.
We have to decide who will do the caching of the connection and reuse.
cc [~Stack]

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

View raw message