subject:"\[jira\] \[Commented\] \(HBASE\-21071\) HBaseTestingUtility\:\:startMiniCluster\(\) to use builder pattern"

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-28 Thread Hudson (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16594583#comment-16594583
 ] 

Hudson commented on HBASE-21071:


Results for branch master
[build #461 on 
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/master/461/]: (x) 
*{color:red}-1 overall{color}*

details (if available):

(x) {color:red}-1 general checks{color}
-- For more information [see general 
report|https://builds.apache.org/job/HBase%20Nightly/job/master/461//General_Nightly_Build_Report/]




(/) {color:green}+1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2) 
report|https://builds.apache.org/job/HBase%20Nightly/job/master/461//JDK8_Nightly_Build_Report_(Hadoop2)/]


(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3) 
report|https://builds.apache.org/job/HBase%20Nightly/job/master/461//JDK8_Nightly_Build_Report_(Hadoop3)/]


(/) {color:green}+1 source release artifact{color}
-- See build output for details.


(/) {color:green}+1 client integration test{color}


> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Improvement
>  Components: test
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>  Labels: reviewed
> Fix For: 3.0.0, 2.2.0
>
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-27 Thread Hudson (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16594317#comment-16594317
 ] 

Hudson commented on HBASE-21071:


Results for branch branch-2
[build #1173 on 
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1173/]: 
(x) *{color:red}-1 overall{color}*

details (if available):

(/) {color:green}+1 general checks{color}
-- For more information [see general 
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1173//General_Nightly_Build_Report/]




(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2) 
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1173//JDK8_Nightly_Build_Report_(Hadoop2)/]


(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3) 
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1173//JDK8_Nightly_Build_Report_(Hadoop3)/]


(/) {color:green}+1 source release artifact{color}
-- See build output for details.


(/) {color:green}+1 client integration test{color}


> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Improvement
>  Components: test
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>  Labels: reviewed
> Fix For: 3.0.0, 2.2.0
>
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-27 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16593998#comment-16593998
 ] 

Mingliang Liu commented on HBASE-21071:
---

Thank you very much [~stack], [~yuzhih...@gmail.com] and [~Apache9] for review 
and insightful discussion.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Improvement
>  Components: test
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>  Labels: reviewed
> Fix For: 3.0.0, 2.2.0
>
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16591063#comment-16591063
 ] 

Mingliang Liu commented on HBASE-21071:
---

More than happy. Thanks [~stack].

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Improvement
>  Components: test
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16591049#comment-16591049
 ] 

Hadoop QA commented on HBASE-21071:
---

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
14s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 73 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
12s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
42s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
28s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  2m 
41s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
25s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  5m 
17s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m  
3s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
14s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  5m 
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
18s{color} | {color:green} hbase-server: The patch generated 0 new + 406 
unchanged - 48 fixed = 406 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
19s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
10s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
16s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
13s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
14s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 35s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  6m  
2s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
17s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}117m  
8s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 14m 
11s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} |

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590877#comment-16590877
 ] 

Mingliang Liu commented on HBASE-21071:
---

Interesting. {{TestFlushWithThroughputController}} is not happy consistently. 
Well I'd like to resubmit as [HBASE-21097] has been committed after our last 
Jenkins run.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Improvement
>  Components: test
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590870#comment-16590870
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
16s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 73 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
15s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
33s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
32s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  2m 
38s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
13s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  5m 
35s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m  
4s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
15s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  5m 
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
17s{color} | {color:green} hbase-server: The patch generated 0 new + 406 
unchanged - 48 fixed = 406 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
20s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
13s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
10s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
14s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
23s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 36s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  6m  
5s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m  
5s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}131m 12s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m 
53s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590636#comment-16590636
 ] 

stack commented on HBASE-21071:
---

Ok. Retrying this patch and +1'd over in HBASE-21097

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Improvement
>  Components: test
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590626#comment-16590626
 ] 

Mingliang Liu commented on HBASE-21071:
---

Oh, I see [~yuzhih...@gmail.com] provided a patch specially aiming to fix this 
in [HBASE-21097]. 

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590623#comment-16590623
 ] 

Mingliang Liu commented on HBASE-21071:
---

Thanks [~stack]. I wish I changed the configs (ideally by mistake) for the 
failing test {{TestFlushWithThroughputController::testFlushThroughputTuning}}, 
so the fix can be trivial. Unfortunately I didn't :(

The only HTU method it uses to start a mini cluster is 
{{hbtu.startMiniCluster(1);}} where 1 is the slave number. This is used by many 
other tests and they all seem happy. I run it locally multiple times and it 
always pass.

I had a look at the test log and found the delta for double assertion, 
{{EPSILON}}, seems too tight. Not sure if we can relax that a little bit.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590600#comment-16590600
 ] 

stack commented on HBASE-21071:
---

Yeah, same tests fail. Maybe we changed the configs for the tests by mistake 
[~liuml07] ? Thanks sir.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590586#comment-16590586
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
16s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 73 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
13s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
59s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
32s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  2m 
53s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
24s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  5m 
33s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
10s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
14s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
50s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  5m 
50s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
19s{color} | {color:green} hbase-server: The patch generated 0 new + 406 
unchanged - 48 fixed = 406 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
13s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
11s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
16s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
14s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
22s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 55s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  6m 
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
28s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}134m 23s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m 
17s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590231#comment-16590231
 ] 

stack commented on HBASE-21071:
---

Although, this is second failure of TestFlushWithThroughputController. Perhaps 
configs in this test were changed? Mind taking a look [~liuml07] ? Thank you.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16590228#comment-16590228
 ] 

stack commented on HBASE-21071:
---

Random failures. One more try then will commit.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-23 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589866#comment-16589866
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
15s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 73 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
31s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  4m 
41s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  6m 
33s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  3m 
10s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
58s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  6m 
38s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
25s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
15s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  4m 
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  6m 
14s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  6m 
14s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
32s{color} | {color:green} hbase-server: The patch generated 0 new + 406 
unchanged - 48 fixed = 406 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
22s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
14s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
15s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
46s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
9m 29s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  7m  
9s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
30s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}119m 53s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 15m 
29s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-22 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589713#comment-16589713
 ] 

Mingliang Liu commented on HBASE-21071:
---

The failing tests can pass locally, and seems not related. One is tracked by 
[HBASE-21097] and the other [HBASE-19907]. 

Thanks [~stack]. I will watch and retry it for a clean run. 

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-22 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589674#comment-16589674
 ] 

stack commented on HBASE-21071:
---

Failures seem unrelated. Retry.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-22 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589638#comment-16589638
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
14s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 73 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  1m 
16s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  6m 
 8s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
34s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  2m 
46s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
24s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  6m 
11s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
34s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
20s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  4m 
30s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  6m 
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  6m 
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
25s{color} | {color:green} hbase-server: The patch generated 0 new + 406 
unchanged - 48 fixed = 406 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
27s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
15s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
16s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
13s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
49s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
9m  4s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  7m 
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m 
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}128m 12s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 14m 
11s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-22 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589402#comment-16589402
 ] 

stack commented on HBASE-21071:
---

I +1'd it up on rb [~liuml07] (There is a small question there too if you have 
a moment). Let me rerun hadoopqa. Will commit if all good. Nice cleanup. Thanks.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.006.patch, 
> HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-22 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589201#comment-16589201
 ] 

Mingliang Liu commented on HBASE-21071:
---

The failing test for {{branch-2}} is intermittent and can pass locally. It 
fails w/o this patch. I filed [HBASE-21096] to track it.

Ready for review. [~yuzhih...@gmail.com] and [~stack] mind taking a look? 
Thanks!

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch, HBASE-21071.006.patch, HBASE-21071.branch-2.006.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-22 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16588576#comment-16588576
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
14s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 71 new or modified test 
files. {color} |
|| || || || {color:brown} branch-2 Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  1m  
6s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  5m 
58s{color} | {color:green} branch-2 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m  
1s{color} | {color:green} branch-2 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  3m 
 0s{color} | {color:green} branch-2 passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  3m 
58s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  5m 
24s{color} | {color:green} branch-2 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  2m  
3s{color} | {color:green} branch-2 passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
17s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  2m 
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  4m 
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  4m 
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
19s{color} | {color:green} hbase-server: The patch generated 0 new + 410 
unchanged - 48 fixed = 410 total (was 458) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
19s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
11s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
16s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
14s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  3m 
39s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
5m 46s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  4m 
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m 
42s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}119m 12s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 17m  
4s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  5m  
8s{color} | {color:green} hbase-rsgroup in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-21 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16588391#comment-16588391
 ] 

Mingliang Liu commented on HBASE-21071:
---

The only failing test {{TestSyncReplicationMoreLogsInLocalCopyToRemote}} pass 
locally, and happens a few times w/o this patch in other JIRAs. Maybe related 
to HBASE-20634.

V6 adds the IA.Public annotation. Also attach a branch-2 patch.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-21 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16588378#comment-16588378
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
27s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 73 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
29s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  6m 
37s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 11m 
28s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  6m 
21s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  6m 
47s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 12m 
10s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  4m 
37s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
16s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  5m 
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
27s{color} | {color:green} hbase-server: The patch generated 0 new + 406 
unchanged - 48 fixed = 406 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
20s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
32s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
10s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
14s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
13s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
11s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  5m 
58s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green} 
14m 19s{color} | {color:green} Patch does not cause any errors with Hadoop 
2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 14m 
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  5m 
56s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}160m  6s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 14m 
33s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-21 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16588358#comment-16588358
 ] 

Mingliang Liu commented on HBASE-21071:
---

{quote}
Shouldn't StartMiniClusterOption have the same annotation ?
{quote}
Yes, this option should use IA.Public. Thanks,

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-21 Thread Ted Yu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16588190#comment-16588190
 ] 

Ted Yu commented on HBASE-21071:


HBaseTestingUtility is annotated @InterfaceAudience.Public .

Shouldn't StartMiniClusterOption have the same annotation ?

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-21 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16588161#comment-16588161
 ] 

Mingliang Liu commented on HBASE-21071:
---

The v4 patch seems good to Hadoop QA (except a trivial checkstyle warning).

The v5 patch, as promised, merges the v4 patch with code changes in v3 patch 
which updates the existing usages of newly deprecated methods. This version is 
ready for review, thanks.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch, 
> HBASE-21071.005.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-21 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16587096#comment-16587096
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
11s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 2 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
40s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  1m 
50s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
17s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
21s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  2m  
7s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
32s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
48s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  1m 
51s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  1m 
51s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  1m 
15s{color} | {color:red} hbase-server: The patch generated 2 new + 228 
unchanged - 47 fixed = 230 total (was 275) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
17s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 46s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  2m  
5s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
30s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}120m 
28s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
23s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}156m 48s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b |
| JIRA Issue | HBASE-21071 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12936378/HBASE-21071.004.patch 
|
| Optional Tests |  asflicense  javac  javadoc  unit  findbugs  shadedjars  
hadoopcheck  hbaseanti  checkstyle  compile  |
| uname | Linux 5771a75c3856 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 
10:45:36 UTC 2018 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | 
/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
 |
| git revision | master / bb3494134e |
| maven | version: Apache Maven 3.5.4 
(1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC3 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-HBASE-Build/14103/artifact/patchprocess/diff-checkstyle-hbase-server.txt
 |
|  Test Results | 
https://builds.apache.org/job/PreCommit-HBASE-Build/14103/testReport/ |
| Max. process+thread count | 4867 (vs. ulimit of 1) |
| modules | C: hbase-server U: hbase-server |
| Console output |

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-20 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16586968#comment-16586968
 ] 

stack commented on HBASE-21071:
---

Excellent.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-20 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16586935#comment-16586935
 ] 

Mingliang Liu commented on HBASE-21071:
---

Thanks [~stack]. That makes perfect sense to me now.

The v4 patch brings back the overload methods, deprecates them, updates 
(correct) the javadoc, and implements them using the option builder (so default 
values are clearer).

This v4 patch is partial and is mainly for Hadoop QA: I did not change all the 
use cases deliberately in this version patch. The reason is that, as we don't 
want to break those public API contract, let's see if the change makes our 
tests happy using the deprecated old APIs. Hopefully they are fine.

After this, I'll prepare the v5 patch, and the difference will be us eating our 
own dog food: update all existing test cases to use the new option builder (the 
change is already in v3 patch, I only need to merge them to v4 patch).

Sounds like a plan? Thanks!

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch, HBASE-21071.004.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-20 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16586914#comment-16586914
 ] 

Hadoop QA commented on HBASE-21071:
---

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
21s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 72 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
32s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  4m 
21s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  8m 
32s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  6m 
14s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  6m 
50s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 11m 
51s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  5m 
52s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
38s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  6m 
48s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  7m  
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  7m  
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
17s{color} | {color:green} hbase-server: The patch generated 0 new + 405 
unchanged - 49 fixed = 405 total (was 454) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} The patch hbase-mapreduce passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
12s{color} | {color:green} The patch hbase-rsgroup passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
11s{color} | {color:green} The patch hbase-shell passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
15s{color} | {color:green} The patch hbase-rest passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
13s{color} | {color:green} The patch hbase-examples passed checkstyle {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
11s{color} | {color:green} The patch hbase-spark passed checkstyle {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
12s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 35s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  5m 
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m 
59s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}118m 
11s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m 
10s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} |

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-20 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16586713#comment-16586713
 ] 

stack commented on HBASE-21071:
---

bq. However, I have no idea about HBase's commitment to IA.Public class across 
major releases.
[~liuml07] 

We are allowed break compat in a major release (as per semver) but we try to 
minimize breakage if only to avoid pissing off our users too much. Can remove 
after deprecating for the life of a major release is allowed.

Yeah, [~Apache9] has a point (unfortunately. Public is a bit of a pain. Would 
be sweet if could deprecate in master branch too. And yeah, this change even 
though in old HBTU has value. Thanks.


> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch, HBASE-21071.003.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-20 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16586689#comment-16586689
 ] 

Mingliang Liu commented on HBASE-21071:
---

[~Apache9] Thanks for the input. I understand the concern of 
{{@InterfaceAudience.Public}}, though I don't like its being Public either. I 
was thinking in master branch it's OK to remove those overload methods as it's 
for a new major release. While in branch-2 we deprecate those methods instead 
of phasing out. However, I have no idea about HBase's commitment to IA.Public 
class across major releases.

For starting a brand new and better test utility tool, I like the idea. Ideally 
as [~stack] suggested, we can use the option class in this patch and write a 
builder {{MiniHaseClusterBuilder}} to create {{MiniHaseCluster}} object on 
which we can call start/stop. That work is bigger than the scope here. I can 
help with that later.

This patch is most orthogonal to that effort and can add value to existing HTU. 
If we strictly respect the Public annotation, I think perhaps we can deprecate 
those methods instead of removing in both {{master}} and {{branch-2}}. 
Downstream applications using old overload methods will still compile but they 
will get checkstyle and compile warnings. If this makes sense, I can bring back 
the deleted 10+ method with deprecation annotation and implement them using 
option builder.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-19 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585146#comment-16585146
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
12s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 72 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
26s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
45s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m  
8s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  2m 
40s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
12s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  4m 
55s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m 
58s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
13s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  5m 
11s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  1m 
19s{color} | {color:red} hbase-server: The patch generated 1 new + 407 
unchanged - 49 fixed = 408 total (was 456) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
15s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 31s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  5m 
37s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m 
59s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}122m 40s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 12m 
18s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m 
13s{color} | {color:green} hbase-rsgroup in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  7m  
1s{color} | {color:green} hbase-shell in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  3m  
4s{color} | {color:green} hbase-rest in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  1m 
42s{color} | {color:green} hbase-examples in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m 
52s{color} | {color:green} hbase-spark in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  2m 
37s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}214m 33s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-19 Thread Duo Zhang (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585110#comment-16585110
 ] 

Duo Zhang commented on HBASE-21071:
---

And I like the approach here, Builder pattern is good. My concern is that, 
maybe we could start with a new one instead of patching on a corrupt one? I do 
not think HBTU should be marked as IA.Public directly, we should give user an 
interface, or just some actual utility classes without any internal states.

Thanks.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-19 Thread Duo Zhang (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585109#comment-16585109
 ] 

Duo Zhang commented on HBASE-21071:
---

Our testing framework is a bit messy, IIRC, the HBTU is marked as IA.Public, so 
I'm afraid you can not remove the public methods directly...

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-19 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585101#comment-16585101
 ] 

Mingliang Liu commented on HBASE-21071:
---

Thanks [~stack] for prompt review and helpful comments!

Yes it's necessary to make javadoc clear. In the v1 patch I put comments for 
each {{StartMiniClusterOption}} field so I did not put anything for the 
Builder. I agree that the field name should be meaningful. I also got confused 
about the {{create}} and {{withWALDir}} options, so I read through and rename 
them to {{createRootDir}} and {{createWALDir}} respectively. The v2 patch also 
refined the comments for those two options in  {{StartMiniClusterOption}}  
javadoc.

I leave the extra short cut method to start a mini cluster: 
{{startMiniCluster(int numSlaves)}}. There are >300 usages in the whole project 
and this method needs special attention. The other start method 
{{startMiniCluster(int numMasters, int numSlaves)}} was deleted as there are 
only ~20 effective use cases and I changed them manually from v1 patch to v2.

If the v2 patch looks good, I'll prepare for a branch-2 one.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch, 
> HBASE-21071.002.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-18 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585031#comment-16585031
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
12s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 31 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
30s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
48s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  3m 
17s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
47s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
15s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  3m 
17s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m  
1s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
15s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  3m 
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  3m 
17s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  1m 
16s{color} | {color:red} hbase-server: The patch generated 3 new + 353 
unchanged - 43 fixed = 356 total (was 396) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
12s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
7m 35s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  3m 
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
58s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}131m  5s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m 
56s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m 
53s{color} | {color:green} hbase-spark in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  1m 
10s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}195m 24s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.hbase.regionserver.TestRegionFavoredNodes |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b |
| JIRA Issue | HBASE-21071 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12936174/HBASE-21071.001.patch 
|
| Optional Tests |  asflicense  javac  javadoc  unit  findbugs  shadedjars  
hadoopcheck  hbaseanti  checkstyle  compile  |
| uname | Linux 5f66ea32baa3 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 
10:45:36 UTC 2018 x86_64 GNU/Linux |
| Build tool | maven |
|

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-18 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585018#comment-16585018
 ] 

stack commented on HBASE-21071:
---

Man. This stuff is a mess. Your patch helps a load.

I saw 'create' as a builder method and didn't know what it was for. Your 
builder methods could do with a bit of javadoc if you don't mind. You might 
also take the opportunity to fix our mess... the create method should be 
createRootDir or some such if it was called that, you might even be able to 
do w/o out javadoc. The method name alone would do.

No need of the final here... 

final StartMiniClusterOption option

... it does nothing IIRC.

This is a good change:

MiniHBaseCluster hbm = htu.startMiniHBaseCluster(1, 1); 104 
final MiniHBaseCluster hbm = htu.startMiniHBaseCluster();

This stuff below is great too...

StartMiniClusterOption.builder()
72  .masterClass(TestMasterMetrics.MyMaster.class).build();
73  TEST_UTIL.startMiniCluster(option);

Is withWALDir(true) same as create?

Patch looks great so far.

Agree, that leaving startMiniCluster(int numMasters, int numSlaves) in place as 
a short cut especially given it is used in so many places.

Nice cleanup.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-18 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16585017#comment-16585017
 ] 

stack commented on HBASE-21071:
---

bq. ... so I think HBase cluster (option) builder also makes sense here.

Makes sense. In class javadoc make it clear that it a compound of options from 
all cluster types.

Agree w/ your other conclusions. Let me look at patch.


> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
> Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-18 Thread Hadoop QA (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584753#comment-16584753
 ] 

Hadoop QA commented on HBASE-21071:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
15s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green}  0m  
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 31 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
21s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  4m 
45s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  3m 
55s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  2m 
 5s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
33s{color} | {color:green} branch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  4m  
8s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m  
5s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
16s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  4m 
 5s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  4m 
12s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  4m 
12s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  1m 
26s{color} | {color:red} hbase-server: The patch generated 3 new + 353 
unchanged - 43 fixed = 356 total (was 396) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
51s{color} | {color:green} patch has no errors when building our shaded 
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}  
8m 23s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4 
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  4m  
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  1m  
1s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 22m 35s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 13m 
32s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  4m 
43s{color} | {color:green} hbase-spark in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
43s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 92m 36s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.hbase.TestClientClusterMetrics |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b |
| JIRA Issue | HBASE-21071 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12936145/HBASE-21071.000.patch 
|
| Optional Tests |  asflicense  javac  javadoc  unit  findbugs  shadedjars  
hadoopcheck  hbaseanti  checkstyle  compile  |
| uname | Linux 2ba282319beb 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 
10:45:36 UTC 2018 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-18 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584741#comment-16584741
 ] 

Mingliang Liu commented on HBASE-21071:
---

{quote}
It seems the Builder can be a class within MiniClusterOptions.
{quote}
Thanks [~yuzhih...@gmail.com]. Yes that's a good suggestion.

{quote}
Should it be StartMiniClusterOptions to tie the new Builder tighter to the 
startMiniCluster method? (Will other methods in MiniCluster want to take 
options?)
{quote}
Yes this should be {{StartMiniClusterOptions}}. I don't find other methods that 
will be using this option class.

{quote}
Does this mean the options should be MiniHBaseClusterOptions and we should 
rename this start method to be startMiniHBaseCluster. Would a 
MiniHBaseClusterBuilder make sense returning a MiniHBaseCluster instance on 
which you called start.
{quote}
I checked this and tried in code for something that only focuses on the 
{{MiniHBaseCluster}}. Also followed the idea of creating a 
{{MiniHBaseClusterBuilder}} class to build a {{MiniHBaseCluster}} directly. 
However, I got two obstacles after a short code walk:
# Option combinations supported by {{startMiniCluster}} also include multiple 
HDFS options. To simplify those polymorphic helper methods, it seems easier to 
consolidate the hbase/hdfs/zk options in one place {{StartMiniClusterOptions}}. 
Specially {{startMiniHBaseCluster}} also accepts this option.
# If we replace {{startMiniCluster()}, the current build-and-start combo 
methods, with creating {{MiniHBaseCluster}} from builder first and calling 
start(), all call sites (hundreds) will have to update. Meanwhile, 
{{MiniHBaseCluster}} constructor currently initializes the cluster. We will 
have to split it to builder phase and start() phase. Some of the methods are 
using non-static {{HBaseTestingUtility}} methods to prepare directories. As 
[~stack] expects, it will be a very large refactoring patch.

*TL;DR* I take the trade off of being perfect and being affordable change. To 
start a {{MiniCluster}} or {{MiniHBaseCluster}} cluster, we first build an 
immutable option and then pass it to {{startMiniCluster(option)}} or 
{{startMiniHBaseCluster(option)}} methods. If using default option values, we 
can avoid build an option and use the other two methods {{startMiniCluster()}} 
or {{startMiniHBaseCluster()}}. These four methods sever all use cases we are 
targeting in a (hopefully) clear, simple and flexible way.

The v1 patch almost implements this idea, with one exception: 
{{startMiniCluster(int numSlaves)}} and {{startMiniCluster(int numMasters, int 
numSlaves)}} are not yet removed and replaced with simple builder calls. The 
reason is that, there are hundreds of calls of those two methods, and changing 
them manually can be error-prone. I'd like to post the v1 patch first for high 
level review and Jenkins. If it looks good overall, in the next patch I can 
update all other places. I'm fine if we keep one of them as another shortcut by 
the way.

Thanks,

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-17 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584586#comment-16584586
 ] 

Mingliang Liu commented on HBASE-21071:
---

Thanks [~yuzhih...@gmail.com] and [~stack] for positive feedback.

Yes, the existing structure is not very well organized as we have multiple 
{{MiniHBaseCluster}} constructors accepting different combination of arguments, 
13 {{startMiniCluster()}} methods, and 3 {{startMiniHBaseCluster()}}; plus 
{{startMiniCluster()}} ultimately calls {{startMiniHBaseCluster()}} after 
building {{MiniDFSCluster}} and {{MiniZKCluster}}.

{quote}
Does this mean the options should be MiniHBaseClusterOptions and we should 
rename this start method to be startMiniHBaseCluster.
{quote}
My previous idea was to have both DFS cluster options ({{numDataNodes}} and 
{{dataNodeHosts}}) and HBase cluster options as most tests create them together 
via {{startMiniCluster()}}. I also see usages where only DFSCluster was created 
so I think HBase cluster (option) builder also makes sense here.

{quote}
Would a MiniHBaseClusterBuilder make sense returning a MiniHBaseCluster 
instance on which you called start.
{quote}
I like this idea. Let me provide an early patch to show how the structure can 
be clearer. I will work on {{master}} branch first as I don't worry about 
pruning old stuff.


> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-17 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584529#comment-16584529
 ] 

stack commented on HBASE-21071:
---

Or, I just took a look... startMiniCluster method starts a MiniHBaseCluster 
Does this mean the options should be MiniHBaseClusterOptions and we should 
rename this start method to be startMiniHBaseCluster. 

These test classes are messy. They grew over time. They intentionally allow 
user many options but yes, as you note, the plethora tend to overwhelm making 
it unreadable at a certain point. Would a MiniHBaseClusterBuilder make sense 
returning a MiniHBaseCluster instance on which you called start.

Sorry for the messy feedback. You seem good at 'design' -- though limited 
interaction -- so I feel ok pushing back offering a larger refactor (smile).

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-17 Thread stack (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584522#comment-16584522
 ] 

stack commented on HBASE-21071:
---

Thank you for taking this up [~liuml07].

Should it be StartMiniClusterOptions to tie the new Builder tighter to the 
startMiniCluster method? (Will other methods in MiniCluster want to take 
options?)

Otherwise, looks great.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-17 Thread Ted Yu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584511#comment-16584511
 ] 

Ted Yu commented on HBASE-21071:


Overall, I like this initiative.

It seems the Builder can be a class within MiniClusterOptions.
A pattern in current base would look something like this:

MiniClusterOptions.newBuilder().setX().setY().build();

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (HBASE-21071) HBaseTestingUtility::startMiniCluster() to use builder pattern

2018-08-17 Thread Mingliang Liu (JIRA)



[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16584507#comment-16584507
 ] 

Mingliang Liu commented on HBASE-21071:
---

ping [~te...@apache.org] and [~stack]. Thanks,

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --
>
> Key: HBASE-21071
> URL: https://issues.apache.org/jira/browse/HBASE-21071
> Project: HBase
>  Issue Type: Bug
>  Components: test
>Affects Versions: 3.0.0
>Reporter: Mingliang Liu
>Assignee: Mingliang Liu
>Priority: Major
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

46 matches

Mail list logo