Details

    • Reviewed

    Attachments

      Issue Links

        Activity

          Xiaolin Ha Xiaolin Ha added a comment -

          Can I work on this?

          Xiaolin Ha Xiaolin Ha added a comment - Can I work on this?
          zhangduo Duo Zhang added a comment -

          Sure, just go ahead. But please post the API list first before writing the code? Some old methods are not needed any more I guess.

          Thanks.

          zhangduo Duo Zhang added a comment - Sure, just go ahead. But please post the API list first before writing the code? Some old methods are not needed any more I guess. Thanks.
          Xiaolin Ha Xiaolin Ha added a comment -

          According to RSGroupAdmin.proto, I think we should keep these methods to Admin,

          rpc GetRSGroupInfo(GetRSGroupInfoRequest)
          returns (GetRSGroupInfoResponse);

          rpc GetRSGroupInfoOfServer(GetRSGroupInfoOfServerRequest)
          returns (GetRSGroupInfoOfServerResponse);

          rpc MoveServers(MoveServersRequest)
          returns (MoveServersResponse);

          rpc AddRSGroup(AddRSGroupRequest)
          returns (AddRSGroupResponse);

          rpc RemoveRSGroup(RemoveRSGroupRequest)
          returns (RemoveRSGroupResponse);

          rpc BalanceRSGroup(BalanceRSGroupRequest)
          returns (BalanceRSGroupResponse);

          rpc ListRSGroupInfos(ListRSGroupInfosRequest)
          returns (ListRSGroupInfosResponse);

          rpc RemoveServers(RemoveServersRequest)
          returns (RemoveServersResponse);

          Xiaolin Ha Xiaolin Ha added a comment - According to RSGroupAdmin.proto, I think we should keep these methods to Admin, rpc GetRSGroupInfo(GetRSGroupInfoRequest) returns (GetRSGroupInfoResponse); rpc GetRSGroupInfoOfServer(GetRSGroupInfoOfServerRequest) returns (GetRSGroupInfoOfServerResponse); rpc MoveServers(MoveServersRequest) returns (MoveServersResponse); rpc AddRSGroup(AddRSGroupRequest) returns (AddRSGroupResponse); rpc RemoveRSGroup(RemoveRSGroupRequest) returns (RemoveRSGroupResponse); rpc BalanceRSGroup(BalanceRSGroupRequest) returns (BalanceRSGroupResponse); rpc ListRSGroupInfos(ListRSGroupInfosRequest) returns (ListRSGroupInfosResponse); rpc RemoveServers(RemoveServersRequest) returns (RemoveServersResponse);
          zhangduo Duo Zhang added a comment -

          Seems good. So we can not modify the default group now?

          And what about adding methods like setRSGroup(TableName tableName)? Although it can be done by modifyTable, we could provide a shortcut for users?

          zhangduo Duo Zhang added a comment - Seems good. So we can not modify the default group now? And what about adding methods like setRSGroup(TableName tableName)? Although it can be done by modifyTable, we could provide a shortcut for users?
          Xiaolin Ha Xiaolin Ha added a comment -

          So we can not modify the default group now?

          We can't remove default group, and can't remove all servers from default group.

          And what about adding methods like setRSGroup(TableName tableName)? Although it can be done by modifyTable, we could provide a shortcut for users?

          Maybe somebody might ask "why you remove moveTables but add a setRSGroup...".hahaha

           

          For this issue, after using admin to call rsgroup methods, should we just delete RSGroupAdmin RSGroupAdminClient RSGroupAdminServer RSGroupAdminServiceImpl?

          Xiaolin Ha Xiaolin Ha added a comment - So we can not modify the default group now? We can't remove default group, and can't remove all servers from default group. And what about adding methods like setRSGroup(TableName tableName)? Although it can be done by modifyTable, we could provide a shortcut for users? Maybe somebody might ask "why you remove moveTables but add a setRSGroup...".hahaha   For this issue, after using admin to call rsgroup methods, should we just delete RSGroupAdmin RSGroupAdminClient RSGroupAdminServer RSGroupAdminServiceImpl?
          Xiaolin Ha Xiaolin Ha added a comment -

          Should also add 

          rpc GetRSGroupInfoOfTable(GetRSGroupInfoOfTableRequest)
          returns (GetRSGroupInfoOfTableResponse);

          And when changing the UTs, I found that having a method like setRSGroup(TableName tableName) will be very convenient.

          So I will also add this method as your suggestions.

           

          Xiaolin Ha Xiaolin Ha added a comment - Should also add  rpc GetRSGroupInfoOfTable(GetRSGroupInfoOfTableRequest) returns (GetRSGroupInfoOfTableResponse); And when changing the UTs, I found that having a method like setRSGroup(TableName tableName) will be very convenient. So I will also add this method as your suggestions.  
          Xiaolin Ha Xiaolin Ha added a comment - - edited

          In master.proto, I'll add methods as follows(also shows deprecated methods from RSGroupAdmin.proto for more clearly ):

          //rpc GetRSGroupInfo(GetRSGroupInfoRequest)
          //returns (GetRSGroupInfoResponse);

          //rpc GetRSGroupInfoOfServer(GetRSGroupInfoOfServerRequest)
          //returns (GetRSGroupInfoOfServerResponse);

          rpc MoveServers(MoveServersRequest)
          returns (MoveServersResponse);

          rpc AddRSGroup(AddRSGroupRequest)
          returns (AddRSGroupResponse);

          rpc RemoveRSGroup(RemoveRSGroupRequest)
          returns (RemoveRSGroupResponse);

          rpc BalanceRSGroup(BalanceRSGroupRequest)
          returns (BalanceRSGroupResponse);

          rpc ListRSGroupInfos(ListRSGroupInfosRequest)
          returns (ListRSGroupInfosResponse);

          //rpc RemoveServers(RemoveServersRequest)
          //returns (RemoveServersResponse);

          //rpc GetRSGroupInfoOfTable(GetRSGroupInfoOfTableRequest)
          //returns (GetRSGroupInfoOfTableResponse);

          //rpc SetRSGroupForTables(SetRSGroupForTablesRequest)
          //returns (SetRSGroupForTablesResponse);

           

          and in Admin client, keep methods added in GitHub Pull Request #613

          zghaobac what do you think of it?

           

          Xiaolin Ha Xiaolin Ha added a comment - - edited In master.proto, I'll add methods as follows(also shows deprecated methods from RSGroupAdmin.proto for more clearly ): //rpc GetRSGroupInfo(GetRSGroupInfoRequest) //returns (GetRSGroupInfoResponse); //rpc GetRSGroupInfoOfServer(GetRSGroupInfoOfServerRequest) //returns (GetRSGroupInfoOfServerResponse); rpc MoveServers(MoveServersRequest) returns (MoveServersResponse); rpc AddRSGroup(AddRSGroupRequest) returns (AddRSGroupResponse); rpc RemoveRSGroup(RemoveRSGroupRequest) returns (RemoveRSGroupResponse); rpc BalanceRSGroup(BalanceRSGroupRequest) returns (BalanceRSGroupResponse); rpc ListRSGroupInfos(ListRSGroupInfosRequest) returns (ListRSGroupInfosResponse); //rpc RemoveServers(RemoveServersRequest) //returns (RemoveServersResponse); //rpc GetRSGroupInfoOfTable(GetRSGroupInfoOfTableRequest) //returns (GetRSGroupInfoOfTableResponse); //rpc SetRSGroupForTables(SetRSGroupForTablesRequest) //returns (SetRSGroupForTablesResponse);   and in Admin client, keep methods added in  GitHub Pull Request #613 zghaobac  what do you think of it?  
          Xiaolin Ha Xiaolin Ha added a comment -

          Ping zhangduo zghao for reviewing.

          Xiaolin Ha Xiaolin Ha added a comment - Ping zhangduo zghao  for reviewing.
          zhangduo Duo Zhang added a comment -

          Pushed to branch HBASE-22514. Thanks Xiaolin Ha for contributing.

          zhangduo Duo Zhang added a comment - Pushed to branch HBASE-22514 . Thanks Xiaolin Ha for contributing.
          hudson Hudson added a comment -

          Results for branch master
          build #1649 on builds.a.o: -1 overall


          details (if available):

          -1 general checks
          – For more information see general report

          -1 jdk8 hadoop2 checks
          – For more information see jdk8 (hadoop2) report

          -1 jdk8 hadoop3 checks
          – For more information see jdk8 (hadoop3) report

          +1 source release artifact
          – See build output for details.

          +1 client integration test

          hudson Hudson added a comment - Results for branch master build #1649 on builds.a.o : -1 overall details (if available): -1 general checks – For more information see general report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report -1 jdk8 hadoop3 checks – For more information see jdk8 (hadoop3) report +1 source release artifact – See build output for details. +1 client integration test

          People

            Xiaolin Ha Xiaolin Ha
            zhangduo Duo Zhang
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: