[ https://issues.apache.org/jira/browse/HBASE-18298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16179597#comment-16179597 ]
stack commented on HBASE-18298: ------------------------------- What do I look at? RB is different to v5 here? I'm going w/ v5 because it looks much better (smile). Woah. CoprocessorRegionServerServices is way cut down. It looks good. The below cast is hackery for now because this class is deprecated and going away [~anoop.hbase]? rsServices = (RegionServerServices) this.env.getCoprocessorRegionServerServices(); If so, that makes sense. The below creates a new zk instance? 176 ZooKeeper zk = new ZooKeeper(ensemble, sessionTimeout, null); 177 re.getSharedData().putIfAbsent(zkkey, new ZKWatcher(zk)); It gets closed? You think we don't have to reuse because this an example? nit: s/getFromOnlineRegions/getOnlineRegion/ singular. This is good public interface OnlineRegions extends ImmutableOnlineRegions { What you think here: HRegionServer rs = (HRegionServer) env.getCoprocessorRegionServerServices(); No way around it? And later, should we allow getting reference to zk? Though AccessController should be coming internal so wouldn't worry about it.... I think this is ok... because it test: assert regionEnv.getCoprocessorRegionServerServices() instanceof RegionServerServices; 66 RpcServerInterface server = ((RegionServerServices) regionEnv 67 .getCoprocessorRegionServerServices()).getRpcServer(); I think this patch looks great now. +1. > RegionServerServices Interface cleanup for CP expose > ---------------------------------------------------- > > Key: HBASE-18298 > URL: https://issues.apache.org/jira/browse/HBASE-18298 > Project: HBase > Issue Type: Sub-task > Components: Coprocessors > Reporter: Anoop Sam John > Assignee: Anoop Sam John > Priority: Critical > Fix For: 2.0.0-alpha-4 > > Attachments: HBASE-18298.patch, HBASE-18298_V2.patch, > HBASE-18298_V3.patch, HBASE-18298_V4.patch, HBASE-18298_V5.patch > > -- This message was sent by Atlassian JIRA (v6.4.14#64029)