You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Xu Cang (JIRA)" <ji...@apache.org> on 2018/09/29 07:35:00 UTC
[jira] [Comment Edited] (HBASE-20627) Relocate RS Group pre/post
hooks from RSGroupAdminServer to RSGroupAdminEndpoint
[ https://issues.apache.org/jira/browse/HBASE-20627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16632858#comment-16632858 ]
Xu Cang edited comment on HBASE-20627 at 9/29/18 7:34 AM:
----------------------------------------------------------
Hi, [~yuzhihong@gmail.com]
(I am asking this question based on branch-1 code)
By working on HBASE-21117, I have some questions regarding your patch here.
I see in the patch you use a set of booleans to track whether a hook is called or not.
Though I failed to find the place you reset those booleans.
One example is,by adding two lines (in bold) in #testNamespaceConstraint
The test fails because of *observer.preRemoveRSGroupCalled* is already *"true".*
@Test
public void testNamespaceConstraint() throws Exception {
*assertTrue(!observer.preRemoveRSGroupCalled);*
*assertTrue(!observer.postRemoveRSGroupCalled);*
So later on in lines here,
*rsGroupAdmin.removeRSGroup(groupName);*
*assertTrue(observer.preRemoveRSGroupCalled);*
*assertTrue(observer.postRemoveRSGroupCalled);*
Also,
in this line :[https://github.com/apache/hbase/blob/branch-1/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java#L133]
will call this:
[https://github.com/apache/hbase/blob/branch-1/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java#L136]
which sets the boolean "preRemoveRSGroupCalled" to true
So, in my opinion, this test
+ assertTrue(observer.preRemoveRSGroupCalled);
losses its meaning in the test.
I failed to understand the point of asserting these two boolean values.
Please let me know if I missed something.
Thanks!**
was (Author: xucang):
[~yuzhihong@gmail.com]
(I am asking this question based on branch-1 code)
By working on HBASE-21117, I have some questions regarding your patch here.
I see in the patch you use a set of booleans to track whether a hook is called or not.
Though I failed to find the place you reset those booleans.
One example is,by adding two lines (in bold) in #testNamespaceConstraint
The test fails because of *observer.preRemoveRSGroupCalled* is already *"true".*
@Test
public void testNamespaceConstraint() throws Exception {
*assertTrue(!observer.preRemoveRSGroupCalled);*
*assertTrue(!observer.postRemoveRSGroupCalled);*
So later on in lines here,
*rsGroupAdmin.removeRSGroup(groupName);*
*assertTrue(observer.preRemoveRSGroupCalled);*
*assertTrue(observer.postRemoveRSGroupCalled);*
Also,
in this line :[https://github.com/apache/hbase/blob/branch-1/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java#L133]
will call this:
[https://github.com/apache/hbase/blob/branch-1/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java#L136]
which sets the boolean "preRemoveRSGroupCalled" to true
So, in my opinion, this test
+ assertTrue(observer.preRemoveRSGroupCalled);
losses its meaning in the test.
I failed to understand the point of asserting these two boolean values.
Please let me know if I missed something.
Thanks!**
> Relocate RS Group pre/post hooks from RSGroupAdminServer to RSGroupAdminEndpoint
> --------------------------------------------------------------------------------
>
> Key: HBASE-20627
> URL: https://issues.apache.org/jira/browse/HBASE-20627
> Project: HBase
> Issue Type: Bug
> Reporter: Ted Yu
> Assignee: Ted Yu
> Priority: Major
> Fix For: 2.1.0, 1.4.5
>
> Attachments: 20627.branch-1.txt, 20627.v1.txt, 20627.v2.txt, 20627.v3.txt
>
>
> Currently RS Group pre/post hooks are called from RSGroupAdminServer.
> e.g. RSGroupAdminServer#removeRSGroup :
> {code}
> if (master.getMasterCoprocessorHost() != null) {
> master.getMasterCoprocessorHost().preRemoveRSGroup(name);
> }
> {code}
> RSGroupAdminServer#removeRSGroup is called by RSGroupAdminEndpoint :
> {code}
> checkPermission("removeRSGroup");
> groupAdminServer.removeRSGroup(request.getRSGroupName());
> {code}
> If permission check fails, the pre hook wouldn't be called.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)