[ https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16793491#comment-16793491 ]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:38 AM: ----------------------------------------------------------- Hi [~xucang], sorry, my bad. Have to correct my comments... It is : almost each test in hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup -verifies- calls the function modified. getDefaultServers() (addressed by this JIRA) is deeply invoked when HMaster starts (I uploaded the call stack just for your information), so I am not sure if we could verify it directly, but it is verified indirectly by TestRSGroupBasics#testBasicStartup() for the condition where there is no servers in other groups (all are in default group) {code:java} assertEquals(4, defaultInfo.getServers().size()); /* TestRSGroupBase#setUpTestBeforeClass() puts NUM_SLAVES_BASE=4 region servers into default rsgroup */ {code} and TestRSGroupBasics#testClearDeadServers() {code} assertEquals(2, newGroupServers.size()); /* testClearDeadServers() moves 3 region servers from default rsgroup a specific rsgroup and stop 1 region server, making 2 left*/ {code} was (Author: water): Hi [~xucang], sorry, my bad. Have to correct my comments... It is : almost each test in hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup -verifies- calls the function modified. getDefaultServers() (addressed by this JIRA) is deeply invoked when HMaster starts (I uploaded the call stack just for your information), so I am not sure if we could verify it directly, but it is verified indirectly by TestRSGroupBasics#testBasicStartup() {code:java} assertEquals(4, defaultInfo.getServers().size()); /* TestRSGroupBase#setUpTestBeforeClass() puts NUM_SLAVES_BASE=4 region servers into default rsgroup */ {code} and TestRSGroupBasics#testClearDeadServers() {code} assertEquals(2, newGroupServers.size()); /* testClearDeadServers() moves 3 region servers from default rsgroup a specific rsgroup and stop 1 region server, making 2 left*/ {code} > Improve RSGroupInfoManagerImpl#getDefaultServers() > -------------------------------------------------- > > Key: HBASE-22009 > URL: https://issues.apache.org/jira/browse/HBASE-22009 > Project: HBase > Issue Type: Improvement > Components: rsgroup > Reporter: Xiang Li > Assignee: Xiang Li > Priority: Minor > Attachments: HBASE-22009.master.000.patch > > > {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid} > private SortedSet<Address> getDefaultServers() throws IOException { > SortedSet<Address> defaultServers = Sets.newTreeSet(); > for (ServerName serverName : getOnlineRS()) { > Address server = Address.fromParts(serverName.getHostname(), > serverName.getPort()); > boolean found = false; > for (RSGroupInfo rsgi : listRSGroups()) { > if (!RSGroupInfo.DEFAULT_GROUP.equals(rsgi.getName()) && > rsgi.containsServer(server)) { > found = true; > break; > } > } > if (!found) { > defaultServers.add(server); > } > } > return defaultServers; > } > {code} > That is a logic of 2 nest loops. And for each server, listRSGroups() > allocates a new LinkedList and calls Map#values(), both of which are very > heavy operations. > Maybe the inner loop could be moved out, that is > # Build a list of servers of other groups than default group > # Iterate each online servers and check if it is in the list above. If it is > not, then it belongs to default group. -- This message was sent by Atlassian JIRA (v7.6.3#76005)