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-18298) RegionServerServices Interface cleanup for CP expose
Date Sat, 09 Sep 2017 07:18:00 GMT

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

Anoop Sam John commented on HBASE-18298:

bq.This is not right? public CoprocessorRSServices getRegionServerServices() {
I feel this is ok.  I mean the old name.. What we return is the RS services.  We have a separate
Interface specific for the CPs.  Still u people feel the name has to be getCoprocessorRSServices?
bq.What you think of this CPServices Interface boss? 
I think this way is ok. WDYS?
bq.getServerName seems a bit odd in a CP.
I also did not add it 1st.. Seems some of the tests and test CP impl uses this. May be it
is ok to add? ServerName is any way Public exposed. Just a getter which says abt the server
host/post details.. May be of not much use. Still can we have it?
bq.The region methods we could add to OnlineRegions?
Why I have duplicated it here is, the OnlineRegions can not be exposed. Becuase it contains
addToOnlineRegions, removeFromOnlineRegions. We can not expose these.  So OnlineRegions interface
is been maintained still which is having all interface APIs to interfact with online regions
bq. Interface to Map of online regions.  In the  Map, the key is the region's encoded name
and the value is an {@link Region} instance.
And the ones which has to be exposed is been duplicated in the exposed interface.
Same way did for Server interface also.. Most of the methods there should NOT be exposed.
So I have removed the Limited private audience from it and duplicated one or 2 methods which
needs to be exposed to CPs into CoprocessorRSServices
bq.Is the getFileSystem in this Interface ever different than that you'd get with a FileSystem.getFileSytem(Conf)?
Same. May be we can remove from CP interface u mean? Am ok.
bq.We have Stoppable... Could add isStopping to this interface. That'd take care of isStopping.
But that contains stop() method also.. So if we make the CoprocessorRSServices is-a Stoppable,
then we have to expose stop() also. Am trying to avoid all such APIs been exposed. Even now
also the HRS is not a Stoppable type.
bq.Just call CoprocessorRSServices CoprocessorServices?
This is the services for CPs provided specifically by RS. Like the APIs around the online
regions etc. So this name is more clear?
bq.BTW, should we also cleanup the MasterServices for CP? There is a connected issue HBASE-12260.
Thanks for pointing out. I missed this..  Ya we must do.. Will take care as part that old
issue.  Lets clean that also. So when we make a new interface for this master side, we will
name it as CoprocessorMasterServices.
bq.What about renaming the method? getCoprocessorRSServices()?
Mentioned abt this in above comment. U ok?

Thanks for the reviews.

> 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

This message was sent by Atlassian JIRA

View raw message