[jira] [Commented] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16807823#comment-16807823
]
Xiang Li commented on HBASE-22119:
--
[~xucang] the patches for master and branch-1 are ready. Please help to review
them at your convenience. Thanks!
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16806174#comment-16806174
]
Xiang Li edited comment on HBASE-22119 at 3/31/19 3:34 PM:
---
All UT get passed on my local machine against the patch for branch-1.
was (Author: water):
All UT get passed on my local machine
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16806174#comment-16806174
]
Xiang Li commented on HBASE-22119:
--
All UT get passed on my local machine
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16805183#comment-16805183
]
Xiang Li commented on HBASE-22119:
--
[~xucang] patch for branch-1 is uploaded. Please help to review, thanks!
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Attachment: HBASE-22119.branch-1.000.patch
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Status: Patch Available (was: Open)
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Status: Open (was: Patch Available)
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.branch-1.000.patch,
> HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22031) Provide RSGroupInfo with a new constructor using shallow copy
[
https://issues.apache.org/jira/browse/HBASE-22031?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803006#comment-16803006
]
Xiang Li edited comment on HBASE-22031 at 3/29/19 8:25 AM:
---
Hi [~xucang] yes, it is a good question (for me) that if it worths here, or
necessary.
The shallow copy trades safety for a better performance: It is safe if we deep
copy (allocate new TreeSets and then make a copy) for tables and servers
inputted. The update on the inputs will not affect RSGroupInfo instance. But as
a trade-off, the allocation of TreeSet is heavy.
By using shallow copy, RSGroupInfo instance points the the input ones, rather
than making a copy within itself. It is fast but the subsequent update on the
inputs will also affect that RSGroupInfo instance.
If the code followed does not change the input servers and tables, it is safe
to do the shallow copy. I will review the code in the context to make sure it
is safe to do that.
was (Author: water):
Hi [~xucang] yes, it is a good question (for me) that if it worths here, or
necessary.
The shallow copy trades safety for a better performance: It is safe if we deep
copy (allocate new TreeSets and then make a copy) for tables and servers
inputted. The update on the inputs will not affect RSGroupInfo instance. But as
a trace-off, the allocation of TreeSet is heavy.
By using shallow copy, RSGroupInfo instance points the the input ones, rather
than making a copy within itself. It is fast but the subsequent update on the
inputs will also affect that RSGroupInfo instance.
If the code followed does not change the input servers and tables, it is safe
to do the shallow copy. I will review the code in the context to make sure it
is safe to do that.
> Provide RSGroupInfo with a new constructor using shallow copy
> -
>
> Key: HBASE-22031
> URL: https://issues.apache.org/jira/browse/HBASE-22031
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22031.master.000.patch,
> HBASE-22031.master.001.patch, HBASE-22031.master.002.patch
>
>
> As for org.apache.hadoop.hbase.rsgroup.RSGroupInfo, the following constructor
> performs deep copies on both servers and tables inputed.
> {code:title=hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java.java|borderStyle=solid}
> RSGroupInfo(String name, SortedSet servers, SortedSet
> tables) {
> this.name = name;
> this.servers = (servers == null) ? new TreeSet<>() : new TreeSet<>(servers);
> this.tables = (tables == null) ? new TreeSet<>() : new TreeSet<>(tables);
> }
> {code}
> The constructor of TreeSet is heavy and I think it is better to have a new
> constructor with shallow copy and it could be used at least in
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java|borderStyle=solid}
> private synchronized void refresh(boolean forceOnline) throws IOException {
> ...
> groupList.add(new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP,
> getDefaultServers(), orphanTables));
> ...
> {code}
> It is not needed to allocate new TreeSet to deep copy the output of
> getDefaultServers() and orphanTables, both of which are allocated in the near
> context and not updated in the code followed. So it is safe to make a shadow
> copy here.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22031) Provide RSGroupInfo with a new constructor using shallow copy
[
https://issues.apache.org/jira/browse/HBASE-22031?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803006#comment-16803006
]
Xiang Li edited comment on HBASE-22031 at 3/27/19 4:22 PM:
---
Hi [~xucang] yes, it is a good question (for me) that if it worths here, or
necessary.
The shallow copy trades safety for a better performance: It is safe if we deep
copy (allocate new TreeSets and then make a copy) for tables and servers
inputted. The update on the inputs will not affect RSGroupInfo instance. But as
a trace-off, the allocation of TreeSet is heavy.
By using shallow copy, RSGroupInfo instance points the the input ones, rather
than making a copy within itself. It is fast but the subsequent update on the
inputs will also affect that RSGroupInfo instance.
If the code followed does not change the input servers and tables, it is safe
to do the shallow copy. I will review the code in the context to make sure it
is safe to do that.
was (Author: water):
Hi [~xucang] yes, it is a good question (for me) that if it worths here.
The shallow copy trades safety for a better performance: It is safe if we deep
copy (allocate new TreeSets and then make a copy) for tables and servers
inputted. The update on the inputs will not affect RSGroupInfo instance. But as
a trace-off, the allocation of TreeSet is heavy.
By using shallow copy, RSGroupInfo instance points the the input ones, rather
than making a copy within itself. It is fast but the subsequent update on the
inputs will also affect that RSGroupInfo instance.
If the code followed does not change the input servers and tables, it is safe
to do the shallow copy. I will review the code in the context to make sure it
is safe to do that.
> Provide RSGroupInfo with a new constructor using shallow copy
> -
>
> Key: HBASE-22031
> URL: https://issues.apache.org/jira/browse/HBASE-22031
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22031.master.000.patch,
> HBASE-22031.master.001.patch, HBASE-22031.master.002.patch
>
>
> As for org.apache.hadoop.hbase.rsgroup.RSGroupInfo, the following constructor
> performs deep copies on both servers and tables inputed.
> {code:title=hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java.java|borderStyle=solid}
> RSGroupInfo(String name, SortedSet servers, SortedSet
> tables) {
> this.name = name;
> this.servers = (servers == null) ? new TreeSet<>() : new TreeSet<>(servers);
> this.tables = (tables == null) ? new TreeSet<>() : new TreeSet<>(tables);
> }
> {code}
> The constructor of TreeSet is heavy and I think it is better to have a new
> constructor with shallow copy and it could be used at least in
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java|borderStyle=solid}
> private synchronized void refresh(boolean forceOnline) throws IOException {
> ...
> groupList.add(new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP,
> getDefaultServers(), orphanTables));
> ...
> {code}
> It is not needed to allocate new TreeSet to deep copy the output of
> getDefaultServers() and orphanTables, both of which are allocated in the near
> context and not updated in the code followed. So it is safe to make a shadow
> copy here.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22031) Provide RSGroupInfo with a new constructor using shallow copy
[
https://issues.apache.org/jira/browse/HBASE-22031?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803006#comment-16803006
]
Xiang Li commented on HBASE-22031:
--
Hi [~xucang] yes, it is a good question (for me) that if it worths here.
The shallow copy trades safety for a better performance: It is safe if we deep
copy (allocate new TreeSets and then make a copy) for tables and servers
inputted. The update on the inputs will not affect RSGroupInfo instance. But as
a trace-off, the allocation of TreeSet is heavy.
By using shallow copy, RSGroupInfo instance points the the input ones, rather
than making a copy within itself. It is fast but the subsequent update on the
inputs will also affect that RSGroupInfo instance.
If the code followed does not change the input servers and tables, it is safe
to do the shallow copy. I will review the code in the context to make sure it
is safe to do that.
> Provide RSGroupInfo with a new constructor using shallow copy
> -
>
> Key: HBASE-22031
> URL: https://issues.apache.org/jira/browse/HBASE-22031
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22031.master.000.patch,
> HBASE-22031.master.001.patch, HBASE-22031.master.002.patch
>
>
> As for org.apache.hadoop.hbase.rsgroup.RSGroupInfo, the following constructor
> performs deep copies on both servers and tables inputed.
> {code:title=hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java.java|borderStyle=solid}
> RSGroupInfo(String name, SortedSet servers, SortedSet
> tables) {
> this.name = name;
> this.servers = (servers == null) ? new TreeSet<>() : new TreeSet<>(servers);
> this.tables = (tables == null) ? new TreeSet<>() : new TreeSet<>(tables);
> }
> {code}
> The constructor of TreeSet is heavy and I think it is better to have a new
> constructor with shallow copy and it could be used at least in
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java|borderStyle=solid}
> private synchronized void refresh(boolean forceOnline) throws IOException {
> ...
> groupList.add(new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP,
> getDefaultServers(), orphanTables));
> ...
> {code}
> It is not needed to allocate new TreeSet to deep copy the output of
> getDefaultServers() and orphanTables, both of which are allocated in the near
> context and not updated in the code followed. So it is safe to make a shadow
> copy here.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Description:
There are several places to check if a group is the "default" group, where the
input could be a String or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it more safe. It is not safe as there is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
It is more safe to check like:
{code}
RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
{code}
was:
There are several places to check if a group is the "default" group, where the
input could be a String or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it more safe. It is not safe as there is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
It is more safe to check like:
{code}
RSGroupInfo.DEFAULT_GROUP.equals(group.getName)
{code}
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16802974#comment-16802974
]
Xiang Li commented on HBASE-22119:
--
[~xucang] Would you please review this simple improvements at your convenience?
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName())
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Description:
There are several places to check if a group is the "default" group, where the
input could be a String or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it more safe. It is not safe as there is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
It is more safe to check like:
{code}
RSGroupInfo.DEFAULT_GROUP.equals(group.getName)
{code}
was:
There are several places to check if a group is the "default" group, where the
input could be a String or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it safe. There is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it more safe. It is not safe as there is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
> It is more safe to check like:
> {code}
> RSGroupInfo.DEFAULT_GROUP.equals(group.getName)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Status: Patch Available (was: Open)
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it safe. There is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Attachment: HBASE-22119.master.000.patch
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22119.master.000.patch
>
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it safe. There is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Description:
There are several places to check if a group is the "default" group, where the
input could be a String or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it safe. There is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
was:
There are several places to check if a group is the default group, where the
input could be a string or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it safe. There is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> There are several places to check if a group is the "default" group, where
> the input could be a String or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it safe. There is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Description:
There are several places to check if a group is the default group, where the
input could be a string or a RSGroupInfo.
It is better to provide official functions in RSGroupInfo to tell if a group is
the default group, so as to
* Simply the code
* Make it safe. There is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
was:
There are several places to check if a group is the default group or not, the
input could be a string or a RSGroupInfo. It is better to provide official
functions in RSGroupInfo to tell if a group is the default group, so as to
* Simply the code
* Make it safe. There is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> There are several places to check if a group is the default group, where the
> input could be a string or a RSGroupInfo.
> It is better to provide official functions in RSGroupInfo to tell if a group
> is the default group, so as to
> * Simply the code
> * Make it safe. There is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[
https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22119:
-
Description:
There are several places to check if a group is the default group or not, the
input could be a string or a RSGroupInfo. It is better to provide official
functions in RSGroupInfo to tell if a group is the default group, so as to
* Simply the code
* Make it safe. There is no null check like
{code}
if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
{code}
was:There are several places to check if a group is the default group or not,
the input could be a string or a RSGroupInfo. It is better to provide official
functions in RSGroupInfo to tell if a group is
> Provide functions in RSGroupInfo to check if a group is the default group
> -
>
> Key: HBASE-22119
> URL: https://issues.apache.org/jira/browse/HBASE-22119
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> There are several places to check if a group is the default group or not, the
> input could be a string or a RSGroupInfo. It is better to provide official
> functions in RSGroupInfo to tell if a group is the default group, so as to
> * Simply the code
> * Make it safe. There is no null check like
> {code}
> if (!group.getName().equals(RSGroupInfo.DEFAULT_GROUP))
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is default group
[ https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Xiang Li updated HBASE-22119: - Description: There are several places to check if a group is the default group or not, the input could be a string or a RSGroupInfo. It is better to provide official functions in RSGroupInfo to tell if a group is > Provide functions in RSGroupInfo to check if a group is default group > - > > Key: HBASE-22119 > URL: https://issues.apache.org/jira/browse/HBASE-22119 > Project: HBase > Issue Type: Improvement > Components: rsgroup >Reporter: Xiang Li >Assignee: Xiang Li >Priority: Minor > > There are several places to check if a group is the default group or not, the > input could be a string or a RSGroupInfo. It is better to provide official > functions in RSGroupInfo to tell if a group is -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is the default group
[ https://issues.apache.org/jira/browse/HBASE-22119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Xiang Li updated HBASE-22119: - Summary: Provide functions in RSGroupInfo to check if a group is the default group (was: Provide functions in RSGroupInfo to check if a group is default group) > Provide functions in RSGroupInfo to check if a group is the default group > - > > Key: HBASE-22119 > URL: https://issues.apache.org/jira/browse/HBASE-22119 > Project: HBase > Issue Type: Improvement > Components: rsgroup >Reporter: Xiang Li >Assignee: Xiang Li >Priority: Minor > > There are several places to check if a group is the default group or not, the > input could be a string or a RSGroupInfo. It is better to provide official > functions in RSGroupInfo to tell if a group is -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (HBASE-22119) Provide functions in RSGroupInfo to check if a group is default group
Xiang Li created HBASE-22119: Summary: Provide functions in RSGroupInfo to check if a group is default group Key: HBASE-22119 URL: https://issues.apache.org/jira/browse/HBASE-22119 Project: HBase Issue Type: Improvement Components: rsgroup Reporter: Xiang Li Assignee: Xiang Li -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-22064) Remove Admin.deleteSnapshot(byte[])
[ https://issues.apache.org/jira/browse/HBASE-22064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799978#comment-16799978 ] Xiang Li commented on HBASE-22064: -- [~Apache9] [~pingsutw] It seems that the description of this JIRA is still empty. Do you mind putting some words into it? > Remove Admin.deleteSnapshot(byte[]) > --- > > Key: HBASE-22064 > URL: https://issues.apache.org/jira/browse/HBASE-22064 > Project: HBase > Issue Type: Task > Components: Admin >Affects Versions: 3.0.0 >Reporter: Duo Zhang >Assignee: kevin su >Priority: Major > Labels: beginner > Fix For: 3.0.0 > > Attachments: HBASE-22064.v0.patch > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799454#comment-16799454
]
Xiang Li edited comment on HBASE-22009 at 3/24/19 9:56 AM:
---
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095].
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
was (Author: water):
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095).
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799454#comment-16799454
]
Xiang Li edited comment on HBASE-22009 at 3/24/19 9:56 AM:
---
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095]).
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
was (Author: water):
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095])
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799454#comment-16799454
]
Xiang Li edited comment on HBASE-22009 at 3/24/19 9:56 AM:
---
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see [this
comment|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095]).
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
was (Author: water):
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095]).
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799454#comment-16799454
]
Xiang Li edited comment on HBASE-22009 at 3/24/19 9:56 AM:
---
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095])
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
was (Author: water):
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095].
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799454#comment-16799454
]
Xiang Li edited comment on HBASE-22009 at 3/23/19 5:04 AM:
---
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failure (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095).
[^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all UT
could get passed;)
was (Author: water):
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failures (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095).
So [^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all
UT could get passed;)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Comment Edited] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799506#comment-16799506
]
Xiang Li edited comment on HBASE-22089 at 3/23/19 2:54 AM:
---
[~pingsutw] Maybe we could have a discussion on this proposal.
was (Author: water):
[~pingsutw] Maybe we could have a discussion in this proposal.
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Assignee: kevin su
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
> finished. See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.clusterSchemaService is not null.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799506#comment-16799506
]
Xiang Li commented on HBASE-22089:
--
[~pingsutw] Maybe we could have a discussion in this proposal.
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Assignee: kevin su
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
> finished. See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.clusterSchemaService is not null.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799454#comment-16799454
]
Xiang Li commented on HBASE-22009:
--
[~xucang] There is no "previous" branch-1 patch:P. Due to the difference
between master and branch-1, the patch v000 for master does not fit branch-1,
and causes the UT failures (see
[this|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095).
So [^HBASE-22009.branch-1.000.patch] is the one dedicated for branch-1 and all
UT could get passed;)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
finished. See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
was:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
finished. See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
> finished. See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.clusterSchemaService is not null.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet. Actually, it won't be ready until it is not ready
HMaster#initClusterSchemaService() is finished. See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
was:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet (it is one of the steps to start HMaster). See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet. Actually, it won't be ready until it is not ready
> HMaster#initClusterSchemaService() is finished. See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.clusterSchemaService is not null.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
finished. See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
was:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet. Actually, it won't be ready until it is not ready
HMaster#initClusterSchemaService() is finished. See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet. Actually, it won't be ready until HMaster#initClusterSchemaService() is
> finished. See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.clusterSchemaService is not null.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet (it is one of the steps to start HMaster). See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.clusterSchemaService is not null.
was:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet (it is one of the steps to start HMaster). See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet (it is one of the steps to start HMaster). See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.clusterSchemaService is not null.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet (it is one of the steps to start HMaster). See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add protections, like adding null check before calling
getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
wait until this.
was:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet (it is one of the steps to start HMaster). See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add a protection here (like adding null check before calling
getClusterSchema().getXXX()).
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet (it is one of the steps to start HMaster). See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add protections, like adding null check before calling
> getClusterSchema().getXXX(), or in HMaster#getClusterSchema(), add a loop to
> wait until this.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
getClusterSchema() might return null when cluster schema service is not ready
yet (it is one of the steps to start HMaster). See [this
comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
for details.
We need to add a protection here (like adding null check before calling
getClusterSchema().getXXX()).
was:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
> getClusterSchema() might return null when cluster schema service is not ready
> yet (it is one of the steps to start HMaster). See [this
> comment|https://issues.apache.org/jira/browse/HBASE-20690?focusedCommentId=16784549=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16784549]
> for details.
> We need to add a protection here (like adding null check before calling
> getClusterSchema().getXXX()).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
[
https://issues.apache.org/jira/browse/HBASE-22089?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22089:
-
Description:
HMaster#getClusterSchema() has a lot of usage, for example:
{code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
void assignTableToGroup(TableDescriptor desc) throws IOException {
String groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
...
}
{code}
> HMaster#getClusterSchema() could return null and cause NPE
> --
>
> Key: HBASE-22089
> URL: https://issues.apache.org/jira/browse/HBASE-22089
> Project: HBase
> Issue Type: Improvement
> Components: master
>Reporter: Xiang Li
>Priority: Minor
>
> HMaster#getClusterSchema() has a lot of usage, for example:
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java|borderStyle=solid}
> void assignTableToGroup(TableDescriptor desc) throws IOException {
> String groupName =
>
> master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
> .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
> ...
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (HBASE-22089) HMaster#getClusterSchema() could return null and cause NPE
Xiang Li created HBASE-22089: Summary: HMaster#getClusterSchema() could return null and cause NPE Key: HBASE-22089 URL: https://issues.apache.org/jira/browse/HBASE-22089 Project: HBase Issue Type: Improvement Components: master Reporter: Xiang Li -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799140#comment-16799140
]
Xiang Li edited comment on HBASE-22009 at 3/22/19 4:03 PM:
---
[~xucang] It seems that the patch is not re-applied to branch-1 yet after
revert. Would you please give a check? The analysis on the UT failure is
[here|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095]
and a dedicated patch for branch-1 is available in the attachment.
was (Author: water):
[~xucang] It seems that the patch is not re-applied to branch-1 yet after
revert. Would you please give a check? The analysis on the UT failure is
[#https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095]
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799140#comment-16799140
]
Xiang Li commented on HBASE-22009:
--
[~xucang] It seems that the patch is not re-applied to branch-1 yet after
revert. Would you please give a check? The analysis on the UT failure is
[#https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16797095=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16797095]
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Description:
{code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
private SortedSet getDefaultServers() throws IOException {
SortedSet 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 nested 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.
was:
{code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
private SortedSet getDefaultServers() throws IOException {
SortedSet 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.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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 nested 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799055#comment-16799055
]
Xiang Li edited comment on HBASE-22009 at 3/22/19 2:19 PM:
---
Xu, agree, I am ok with it.
was (Author: water):
Xu, agree, I am ok with it ^_^
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16799055#comment-16799055
]
Xiang Li commented on HBASE-22009:
--
Xu, agree, I am ok with it ^_^
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16798674#comment-16798674
]
Xiang Li commented on HBASE-22051:
--
[~xucang], please help to review the patch v000 for branch-2 when you have
time. Thanks!
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.branch-2.000.patch, HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Attachment: HBASE-22051.branch-2.000.patch
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.branch-2.000.patch, HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Status: Patch Available (was: Open)
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.branch-2.000.patch, HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Status: Open (was: Patch Available)
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.branch-2.000.patch, HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16798088#comment-16798088
]
Xiang Li edited comment on HBASE-22009 at 3/21/19 1:23 PM:
---
Hi [~xucang] I upload the addendum for master branch. All UT get passed (no
logic change 8-))
Please help to review at your convenience. Please let me know if you need a
complete patch to contain both v000 and the addendum.
was (Author: water):
Hi [~xucang] I upload the addendum for master branch. All UT get passed (no
logic change ^_^)
Please help to review at your convenience. Please let me know if you need a
complete patch to contain both v000 and the addendum.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16798088#comment-16798088
]
Xiang Li edited comment on HBASE-22009 at 3/21/19 1:23 PM:
---
Hi [~xucang] I upload the addendum for master branch. All UT get passed (no
logic change ^_^)
Please help to review at your convenience. Please let me know if you need a
complete patch to contain both v000 and the addendum.
was (Author: water):
Hi [~xucang] I upload the addendum for master branch. All UT get passed.
Please help to review at your convenience. Please let me know if you need a
complete patch to contain both v000 and the addendum.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16798088#comment-16798088
]
Xiang Li edited comment on HBASE-22009 at 3/21/19 1:22 PM:
---
Hi [~xucang] I upload the addendum for master branch. All UT get passed.
Please help to review at your convenience. Please let me know if you need a
complete patch to contain both v000 and the addendum.
was (Author: water):
Hi [~xucang] I upload the addendum for master branch. Please help to review at
your convenience. Please let me know if you need a complete patch to contain
both v000 and the addendum.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Status: Patch Available (was: Open)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16798088#comment-16798088
]
Xiang Li commented on HBASE-22009:
--
Hi [~xucang] I upload the addendum for master branch. Please help to review at
your convenience. Please let me know if you need a complete patch to contain
both v000 and the addendum.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Attachment: HBASE-22009.master.000.addendum.patch
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.addendum.patch, HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Status: Open (was: Patch Available)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.patch, call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16797301#comment-16797301
]
Xiang Li commented on HBASE-22051:
--
[~xucang] I upload patch v000 for branch-1. Please review when you have time.
Thanks!
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Status: Patch Available (was: Open)
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Attachment: HBASE-22051.branch-1.000.patch
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Status: Open (was: Patch Available)
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.branch-1.000.patch,
> HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16797213#comment-16797213
]
Xiang Li commented on HBASE-22009:
--
Hi Xu Cang, I might need to upload an addendum for master branch. The patch
v000 for master(which has been merged) is already a correct one itself, but I
would like to use that addendum to rename some variables to avoid confusion,
also make it to be the same as the patch for branch-1 (master_v000 + addendum =
branch_1_v000, logically and literally). The addendum contains no logic change.
I will upload that addendum for master branch after being verified by full UT.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.patch, call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16797095#comment-16797095
]
Xiang Li edited comment on HBASE-22009 at 3/20/19 2:14 PM:
---
Hi [~xucang], I uploaded the patch v000 for branch-1, which is dedicated for
that branch. The reason why the previous patch fails branch-1 is due to the
following statement:
{code:java}
if (!serversInOtherGroup.contains(server))
{code}
"serversInOtherGroup" is a Set of Address, but "server" is an instance of
ServerName. Set#contains() could accepts an "object" as the input, so no
class/type check here... and serversInOtherGroup.contains(server) always
returns false.
The patch for master branch has the same statement, but its "server" is an
instance of Address, which is correct...The differences between master and
branch-1 causes the confusion here... Sorry, I should have noticed those and
uploaded the patch for branch-1 at the very beginning...
I also make some changes on the variable names, trying to avoid confusion in
the future.
Please review the patch v000 for branch-1 at your convenience. It passes all
UT on my local machine.
was (Author: water):
Hi [~xucang], I uploaded the patch v000 for branch-1, which is dedicated for
that branch. The reason why the previous patch fails branch-1 is due to the
following statement:
{code:java}
if (!serversInOtherGroup.contains(server))
{code}
"serversInOtherGroup" is a Set of Address, but "server" is an instance of
ServerName. Set#contains() could accepts an "object" as the input, so no
class/type check here... and serversInOtherGroup.contains(server) always
returns false.
The patch for master branch has the same statement, but its "server" is an
instance of Address, which is correct...The differences between master and
branch-1 causes the confusion here... Sorry, I should have noticed those and
uploaded the patch for branch-1 at the very beginning...
I also make some changes on the variable names, trying to avoid confusion in
the future.
Please review the patch v000 for branch-1 at your convenience. It passes all
UT on my local machines.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.patch, call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Status: Patch Available (was: Reopened)
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.patch, call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Attachment: HBASE-22009.branch-1.000.patch
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.branch-1.000.patch,
> HBASE-22009.master.000.patch, call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16797095#comment-16797095
]
Xiang Li commented on HBASE-22009:
--
Hi [~xucang], I uploaded the patch v000 for branch-1, which is dedicated for
that branch. The reason why the previous patch fails branch-1 is due to the
following statement:
{code:java}
if (!serversInOtherGroup.contains(server))
{code}
"serversInOtherGroup" is a Set of Address, but "server" is an instance of
ServerName. Set#contains() could accepts an "object" as the input, so no
class/type check here... and serversInOtherGroup.contains(server) always
returns false.
The patch for master branch has the same statement, but its "server" is an
instance of Address, which is correct...The differences between master and
branch-1 causes the confusion here... Sorry, I should have noticed those and
uploaded the patch for branch-1 at the very beginning...
I also make some changes on the variable names, trying to avoid confusion in
the future.
Please review the patch v000 for branch-1 at your convenience. It passes all
UT on my local machines.
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16796293#comment-16796293
]
Xiang Li commented on HBASE-22009:
--
Hi [~xucang], I found this patch seems not workable in branch-1. It makes
TestRSGroupsBasics#testClearDeadServers() failed, not at the verification
points but at afterMethold(), with the following error:
{code:java}
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 15.97 s
<<< FAILURE! - in org.apache.hadoop.hbase.rsgroup.TestRSGroupsBasics
[ERROR]
testClearDeadServers(org.apache.hadoop.hbase.rsgroup.TestRSGroupsBasics) Time
elapsed: 3.59 s <<< ERROR!
org.apache.hadoop.hbase.constraint.ConstraintException:
org.apache.hadoop.hbase.constraint.ConstraintException: Target group is the
same as source group: default
at
org.apache.hadoop.hbase.rsgroup.RSGroupAdminServer.moveServers(RSGroupAdminServer.java:169)
at
org.apache.hadoop.hbase.rsgroup.RSGroupAdminEndpoint.moveServers(RSGroupAdminEndpoint.java:216)
at
org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos$RSGroupAdminService.callMethod(RSGroupAdminProtos.java:13870)
at
org.apache.hadoop.hbase.master.MasterRpcServices.execMasterService(MasterRpcServices.java:711)
at
org.apache.hadoop.hbase.protobuf.generated.MasterProtos$MasterService$2.callBlockingMethod(MasterProtos.java:63436)
at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:2380)
at org.apache.hadoop.hbase.ipc.CallRunner.run(CallRunner.java:124)
at
org.apache.hadoop.hbase.ipc.RpcExecutor$Handler.run(RpcExecutor.java:297)
at
org.apache.hadoop.hbase.ipc.RpcExecutor$Handler.run(RpcExecutor.java:277)
at
org.apache.hadoop.hbase.rsgroup.TestRSGroupsBasics.afterMethod(TestRSGroupsBasics.java:77)
Caused by: org.apache.hadoop.hbase.ipc.RemoteWithExtrasException:
org.apache.hadoop.hbase.constraint.ConstraintException: Target group is the
same as source group: default
at
org.apache.hadoop.hbase.rsgroup.RSGroupAdminServer.moveServers(RSGroupAdminServer.java:169)
at
org.apache.hadoop.hbase.rsgroup.RSGroupAdminEndpoint.moveServers(RSGroupAdminEndpoint.java:216)
at
org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos$RSGroupAdminService.callMethod(RSGroupAdminProtos.java:13870)
at
org.apache.hadoop.hbase.master.MasterRpcServices.execMasterService(MasterRpcServices.java:711)
at
org.apache.hadoop.hbase.protobuf.generated.MasterProtos$MasterService$2.callBlockingMethod(MasterProtos.java:63436)
at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:2380)
at org.apache.hadoop.hbase.ipc.CallRunner.run(CallRunner.java:124)
at
org.apache.hadoop.hbase.ipc.RpcExecutor$Handler.run(RpcExecutor.java:297)
at
org.apache.hadoop.hbase.ipc.RpcExecutor$Handler.run(RpcExecutor.java:277)
{code}
I do not figure out the reason yet...
Shall we revert this change from branch-1 as well as 1.5.1 ? Sorry for the
inconvenience it brings
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16795482#comment-16795482
]
Xiang Li commented on HBASE-22051:
--
[~xucang] Could you please help to review the patch when you are available to?
This JIRA addresses UT while no change made on the logic of HBase itself:P
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16795473#comment-16795473
]
Xiang Li commented on HBASE-22009:
--
Thanks Xu Cang!
> 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
> Fix For: 3.0.0, 2.2.0, 1.5.1, 2.2.1
>
> Attachments: HBASE-22009.master.000.patch,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16795171#comment-16795171
]
Xiang Li edited comment on HBASE-22009 at 3/18/19 4:27 PM:
---
[~xucang] Would you please review [this
comment|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16793491=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16793491]
at your convenience? Thanks!
was (Author: water):
[~xucang] Would you please review [this
comment|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16793491=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16793491]
at your convenience?
> 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,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16795171#comment-16795171
]
Xiang Li commented on HBASE-22009:
--
[~xucang] Would you please review [this
comment|https://issues.apache.org/jira/browse/HBASE-22009?focusedCommentId=16793491=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16793491]
at your convenience?
> 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,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22031) Provide RSGroupInfo with a new constructor using shallow copy
[
https://issues.apache.org/jira/browse/HBASE-22031?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794648#comment-16794648
]
Xiang Li commented on HBASE-22031:
--
[~xucang], would you please help to review patch v002 at your convenience?
> Provide RSGroupInfo with a new constructor using shallow copy
> -
>
> Key: HBASE-22031
> URL: https://issues.apache.org/jira/browse/HBASE-22031
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22031.master.000.patch,
> HBASE-22031.master.001.patch, HBASE-22031.master.002.patch
>
>
> As for org.apache.hadoop.hbase.rsgroup.RSGroupInfo, the following constructor
> performs deep copies on both servers and tables inputed.
> {code:title=hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java.java|borderStyle=solid}
> RSGroupInfo(String name, SortedSet servers, SortedSet
> tables) {
> this.name = name;
> this.servers = (servers == null) ? new TreeSet<>() : new TreeSet<>(servers);
> this.tables = (tables == null) ? new TreeSet<>() : new TreeSet<>(tables);
> }
> {code}
> The constructor of TreeSet is heavy and I think it is better to have a new
> constructor with shallow copy and it could be used at least in
> {code:title=hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java|borderStyle=solid}
> private synchronized void refresh(boolean forceOnline) throws IOException {
> ...
> groupList.add(new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP,
> getDefaultServers(), orphanTables));
> ...
> {code}
> It is not needed to allocate new TreeSet to deep copy the output of
> getDefaultServers() and orphanTables, both of which are allocated in the near
> context and not updated in the code followed. So it is safe to make a shadow
> copy here.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794112#comment-16794112
]
Xiang Li edited comment on HBASE-22051 at 3/16/19 3:50 AM:
---
Uploaded patch v000. The logic change of it includes
* 3 places mentioned in the "description", to remove the hard-coded
verification and use variables instead.
* More strict verifications at the end of testClearNotProcessedDeadServer().
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments.
[~xucang], would you please review it at your convenience?
was (Author: water):
Uploaded patch v000. The logic change of it includes
* 3 places mentioned in the "description", to remove the hard-coded
verification and use variables instead.
* More strict verifications at the end of testClearNotProcessedDeadServer().
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments.
[~xucang], would you please review it at your convenience?
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794112#comment-16794112
]
Xiang Li edited comment on HBASE-22051 at 3/16/19 3:58 AM:
---
Uploaded patch v000. The logic change of it includes
* 3 places mentioned in the "description", to remove the hard-coded
verification and use variables instead.
* More strict verifications at the end of testClearNotProcessedDeadServer().
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup".
* Add or correct some comments.
[~xucang], would you please review it at your convenience?
was (Author: water):
Uploaded patch v000. The logic change of it includes
* 3 places mentioned in the "description", to remove the hard-coded
verification and use variables instead.
* More strict verifications at the end of testClearNotProcessedDeadServer().
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments.
[~xucang], would you please review it at your convenience?
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Status: Patch Available (was: Open)
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Attachment: HBASE-22051.master.000.patch
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
> Attachments: HBASE-22051.master.000.patch
>
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in the verifications of TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Summary: Expect values are hard-coded in the verifications of
TestRSGroupsBasics (was: Expect values are hard-coded in TestRSGroupsBasics)
> Expect values are hard-coded in the verifications of TestRSGroupsBasics
> ---
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect values are hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Summary: Expect values are hard-coded in TestRSGroupsBasics (was: Expect
value is hard-coded in TestRSGroupsBasics)
> Expect values are hard-coded in TestRSGroupsBasics
> --
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794112#comment-16794112
]
Xiang Li edited comment on HBASE-22051 at 3/16/19 3:50 AM:
---
Uploaded patch v000. The logic change of it includes
* 3 places mentioned in the "description", to remove the hard-coded
verification and use variables instead.
* More strict verifications at the end of testClearNotProcessedDeadServer().
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments.
[~xucang], would you please review it at your convenience?
was (Author: water):
Uploaded patch v000. The logic change of it includes the 3 places mentioned in
the "description", to remove the hard-coded verification and use variables
instead.
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments
[~xucang], would you please review it at your convenience?
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794112#comment-16794112
]
Xiang Li edited comment on HBASE-22051 at 3/16/19 3:36 AM:
---
Uploaded patch v000. The logic change of it includes the 3 places mentioned in
the "description", to remove the hard-coded verification and use variables
instead.
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments
[~xucang], would you please review it at your convenience?
was (Author: water):
Uploaded patch v000. The logic change of it includes the 3 places mentioned in
the "description", to remove the hard-coded verification and use the variables
instead.
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments
[~xucang], would you please review it at your convenience?
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794112#comment-16794112
]
Xiang Li commented on HBASE-22051:
--
Uploaded patch v000. The logic change of it includes the 3 places mentioned in
the "description", to remove the hard-coded verification and use the variables
instead.
The patches also make some changes to improve the readability:
* Rename some variables, like "targetServer" --> "serverToStop", "appInfo" -->
"deadServerGroup"
* Add or correct some comments
[~xucang], would you please review it at your convenience?
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Description:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
We could also some other places which have hard-coded verifications, as follow:
{code:java}
public void testClearDeadServers() throws Exception {
...
final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 3);
...
assertEquals(2, newGroupServers.size());
{code}
and
{code:java}
public void testClearNotProcessedDeadServer() throws Exception {
...
RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
...
assertEquals(1, notClearedServers.size());
{code}
was:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
We could also see some hard-coded verifications, as follow:
{code:java}
public void testClearDeadServers() throws Exception {
...
final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 3);
...
assertEquals(2, newGroupServers.size());
{code}
and
{code:java}
public void testClearNotProcessedDeadServer() throws Exception {
...
RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
...
assertEquals(1, notClearedServers.size());
{code}
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also some other places which have hard-coded verifications, as
> follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Description:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
We could also see some hard-coded verifications, as follow:
{code:java}
public void testClearDeadServers() throws Exception {
...
final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 3);
...
assertEquals(2, newGroupServers.size());
{code}
and
{code:java}
public void testClearNotProcessedDeadServer() throws Exception {
...
RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
...
assertEquals(1, notClearedServers.size());
{code}
was:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
and
{code:java}
public void testClearDeadServers() throws Exception {
...
final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 3);
...
assertEquals(2, newGroupServers.size());
{code}
and
{code:java}
public void testClearNotProcessedDeadServer() throws Exception {
...
RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
...
assertEquals(1, notClearedServers.size());
{code}
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> We could also see some hard-coded verifications, as follow:
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Description:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
and
{code:java}
public void testClearDeadServers() throws Exception {
...
final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 3);
...
assertEquals(2, newGroupServers.size());
{code}
and
{code:java}
public void testClearNotProcessedDeadServer() throws Exception {
...
RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
...
assertEquals(1, notClearedServers.size());
{code}
was:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
> and
> {code:java}
> public void testClearDeadServers() throws Exception {
> ...
> final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()),
> 3);
> ...
> assertEquals(2, newGroupServers.size());
> {code}
> and
> {code:java}
> public void testClearNotProcessedDeadServer() throws Exception {
> ...
> RSGroupInfo appInfo = addGroup("deadServerGroup", 1);
> ...
> assertEquals(1, notClearedServers.size());
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[
https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22051:
-
Description:
In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
mini cluster. But in some verifications of TestGroupsBasics, such as in
testBasicStartUp(), the expected value is hard-coded as 4, like:
{code:java}
public void testBasicStartUp() throws IOException {
...
assertEquals(4, defaultInfo.getServers().size());
..
}
{code}
> Expect value is hard-coded in TestRSGroupsBasics
>
>
> Key: HBASE-22051
> URL: https://issues.apache.org/jira/browse/HBASE-22051
> Project: HBase
> Issue Type: Improvement
> Components: rsgroup, test
>Reporter: Xiang Li
>Assignee: Xiang Li
>Priority: Minor
>
> In TestRSGroupsBase, we have NUM_SLAVES_BASE = 4, which is used to launch the
> mini cluster. But in some verifications of TestGroupsBasics, such as in
> testBasicStartUp(), the expected value is hard-coded as 4, like:
> {code:java}
> public void testBasicStartUp() throws IOException {
> ...
> assertEquals(4, defaultInfo.getServers().size());
> ..
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Expect value is hard-coded in TestRSGroupsBasics
[ https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Xiang Li updated HBASE-22051: - Summary: Expect value is hard-coded in TestRSGroupsBasics (was: Remove the hardcoded expect value in TestRSGroupsBasics) > Expect value is hard-coded in TestRSGroupsBasics > > > Key: HBASE-22051 > URL: https://issues.apache.org/jira/browse/HBASE-22051 > Project: HBase > Issue Type: Improvement > Components: rsgroup, test >Reporter: Xiang Li >Assignee: Xiang Li >Priority: Minor > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (HBASE-22051) Remove the hardcoded expect value in TestRSGroupsBasics
[ https://issues.apache.org/jira/browse/HBASE-22051?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Xiang Li updated HBASE-22051: - Priority: Minor (was: Critical) Component/s: (was: master) test rsgroup Issue Type: Improvement (was: Bug) Summary: Remove the hardcoded expect value in TestRSGroupsBasics (was: Prohibit HMaster#getClusterSchema from returning null) > Remove the hardcoded expect value in TestRSGroupsBasics > --- > > Key: HBASE-22051 > URL: https://issues.apache.org/jira/browse/HBASE-22051 > Project: HBase > Issue Type: Improvement > Components: rsgroup, test >Reporter: Xiang Li >Assignee: Xiang Li >Priority: Minor > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 2:41 PM:
---
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 think it
might not be easy to verify it directly, but it is verified indirectly by
(1) {color:#59afe1}TestRSGroupsBasics#testBasicStartUp(){color} for the
condition where there is no servers in other groups (all are in default group)
{code:java}
assertEquals(4, defaultInfo.getServers().size());
/* TestRSGroupsBase#setUpTestBeforeClass() puts NUM_SLAVES_BASE=4 region
servers into default rsgroup, as verified */
{code}
and
(2) {color:#59afe1}TestRSGroupsBasics#testClearDeadServers() {color}for the
condition where there are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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 think it
might not be easy to verify it directly, but it is verified indirectly by
(1) {color:#59afe1}TestRSGroupBasics#testBasicStartUp(){color} 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, as verified */
{code}
and
(2) {color:#59afe1}TestRSGroupBasics#testClearDeadServers() {color}for the
condition where there are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:52 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 think it
might not be easy to verify it directly, but it is verified indirectly by
(1) {color:#59afe1}TestRSGroupBasics#testBasicStartUp(){color} 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, as verified */
{code}
and
(2) {color:#59afe1}TestRSGroupBasics#testClearDeadServers() {color}for the
condition where there are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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
(1) {color:#59afe1}TestRSGroupBasics#testBasicStartUp(){color} 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, as verified */
{code}
and
(2) {color:#59afe1}TestRSGroupBasics#testClearDeadServers() {color}for the
condition where there are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Updated] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Xiang Li updated HBASE-22009:
-
Attachment: call_stack_getDefaultServers.png
> 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,
> call_stack_getDefaultServers.png
>
>
> {code:title=RSGroupInfoManagerImpl.java|borderStyle=solid}
> private SortedSet getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:43 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
(1) 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, as verified */
{code}
and
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then 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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:50 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
(1) {color:#59afe1}TestRSGroupBasics#testBasicStartUp(){color} 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, as verified */
{code}
and
(2) {color:#59afe1}TestRSGroupBasics#testClearDeadServers() {color}for the
condition where there are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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
(1) +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, as verified */
{code}
and
(2) +TestRSGroupBasics#testClearDeadServers()+ for the condition where there
are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:49 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
(1) +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, as verified */
{code}
and
(2) +TestRSGroupBasics#testClearDeadServers()+ for the condition where there
are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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
(1) *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, as verified */
{code}
and
(2) *TestRSGroupBasics#testClearDeadServers() *for the condition where there
are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:49 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
(1) *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, as verified */
{code}
and
(2) *TestRSGroupBasics#testClearDeadServers() *for the condition where there
are some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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
(1) 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, as verified */
{code}
and
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then stop 1
region server, making 2 left, as verified*/
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:41 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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeps 1 server in default rsgroup) and then 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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:40 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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:36 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() 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}
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 he function modified.
getDefaultServers() 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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:42 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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeping 1 server in default rsgroup) and then 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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{code}
assertEquals(2, newGroupServers.size());
/* testClearDeadServers() moves 3 region servers from default rsgroup into a
specific rsgroup (while keeps 1 server in default rsgroup) and then 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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:39 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
(1) 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
(2) TestRSGroupBasics#testClearDeadServers() for the condition where there are
some servers in other groups as well as default group
{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
1. 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() for the condition where there are some
servers in other groups as well as default group
{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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:39 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
1. 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() for the condition where there are some
servers in other groups as well as default group
{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() 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}
> 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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Comment Edited] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li edited comment on HBASE-22009 at 3/15/19 9:37 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()
{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() 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 getDefaultServers() throws IOException {
> SortedSet 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)
[jira] [Commented] (HBASE-22009) Improve RSGroupInfoManagerImpl#getDefaultServers()
[
https://issues.apache.org/jira/browse/HBASE-22009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16793491#comment-16793491
]
Xiang Li commented on HBASE-22009:
--
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 he function modified.
getDefaultServers() 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 getDefaultServers() throws IOException {
> SortedSet 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)
