Umeshkumar9414 commented on PR #8547:
URL: https://github.com/apache/hbase/pull/8547#issuecomment-5319268076

   > The move of `RSGroupAdmin.proto` from `hbase-rsgroup` to `hbase-protocol` 
is not necessary and the way you've left the state of the POM in 
`hbase-rsgroup` the protobuf compilation still happens there but now there's no 
input files. I guess it works?? but is messy.
   > 
   > We move RSGroups out of the separate module into core in HBase 3 and up, 
so there's that, but the way we hedged RSGroups in branch-2 puts everything 
else in `hbase-rsgroup`. It's fine and even customary to have protos outside of 
`hbase-protos`. We have the protos for REST broken out into `hbase-rest`, for 
example, so this is maybe a smell.
   > 
   > This is not a dealbreaker, but there is an alternative that does not 
require it. Consider just using `RSGroupTableAccessor` from `hbase-client`. 
Skip the coprocessor RPC and read the `hbase:rsgroup` system table directly via 
the accessor that already exists in `hbase-client`. Something like:
   > 
   > ```java
   > private RSGroupInfo getRSGroupInfo(String host, int port) throws 
IOException {
   >   if (!RSGroupTableAccessor.isRSGroupsEnabled(conn)) {
   >     return null;
   >   }
   >   Address addr = Address.fromParts(host, port);
   >   for (RSGroupInfo info : RSGroupTableAccessor.getAllRSGroupInfo(conn)) {
   >     if (info.containsServer(addr)) {
   >       return info;
   >     }
   >   }
   >   return null;
   > }
   > ```
   > 
   > I also like this approach better because the invocation of the coprocessor 
is brittle. It works today but any subclass or wrapper would break it, if 
someone is extending rsgroups and implementing their own enhanced coprocessors. 
Unlikely but not impossible.
   
   Thanks a lot. @apurtell . I missed this. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to