[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-17 Thread Hudson (JIRA)


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

Hudson commented on HBASE-21062:


Results for branch branch-2.0
[build #692 on 
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.0/692/]: 
(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.0/692//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.0/692//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.0/692//JDK8_Nightly_Build_Report_(Hadoop3)/]


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


> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-17 Thread Hudson (JIRA)


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

Hudson commented on HBASE-21062:


Results for branch branch-2.1
[build #204 on 
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.1/204/]: 
(/) *{color:green}+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.1/204//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/branch-2.1/204//JDK8_Nightly_Build_Report_(Hadoop2)/]


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


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


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


> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-17 Thread Hudson (JIRA)


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

Hudson commented on HBASE-21062:


Results for branch master
[build #438 on 
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/master/438/]: (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/master/438//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/master/438//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/438//JDK8_Nightly_Build_Report_(Hadoop3)/]


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


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


> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-17 Thread Duo Zhang (JIRA)


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

Duo Zhang commented on HBASE-21062:
---

+1.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-16 Thread stack (JIRA)


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

stack commented on HBASE-21062:
---

+1 for branch-2.0. thanks

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-16 Thread Hudson (JIRA)


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

Hudson commented on HBASE-21062:


Results for branch branch-2
[build #1120 on 
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1120/]: 
(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/1120//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/branch-2/1120//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/1120//JDK8_Nightly_Build_Report_(Hadoop3)/]


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


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


> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-16 Thread Josh Elser (JIRA)


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

Josh Elser commented on HBASE-21062:


{quote}Sorry I missed this point previously. It is right to keep the original 
behavior.
{quote}
No worries! You and I, both :)
{quote}'us it' -> "use it"
{quote}
Thanks, Ted. Will fix on commit along with checkstyle.

Unit test failure seems unrelated based on error, but will rerun locally.

Ping [~Apache9], [~stack], for your branches.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on HBASE-21062:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
13s{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 1 new or modified test 
files. {color} |
|| || || || {color:brown} branch-2.0 Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  3m 
13s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  1m 
59s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
25s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  4m 
36s{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 
19s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
30s{color} | {color:green} branch-2.0 passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  2m 
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  1m 
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  1m 
40s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  1m  
8s{color} | {color:red} hbase-server: The patch generated 5 new + 19 unchanged 
- 0 fixed = 24 total (was 19) {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 
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}  
7m 59s{color} | {color:green} Patch does not cause any errors with Hadoop 2.6.5 
2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  2m 
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
27s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}139m 41s{color} 
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
24s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}174m 39s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | 
hadoop.hbase.master.balancer.TestStochasticLoadBalancerRegionReplicaHighReplication
 |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:6f01af0 |
| JIRA Issue | HBASE-21062 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12935758/HBASE-21062.002.branch-2.0.patch
 |
| Optional Tests |  asflicense  javac  javadoc  unit  findbugs  shadedjars  
hadoopcheck  hbaseanti  checkstyle  compile  |
| uname | Linux b26e215d5cf5 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 
14:43:09 UTC 2018 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | 
/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh
 |
| git revision | branch-2.0 / 5e06b80e75 |
| maven | version: Apache Maven 3.5.4 
(1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) |
| Default Java | 1.8.0_171 |
| findbugs | v3.1.0-RC3 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-HBASE-Build/14052/artifact/patchprocess/diff-checkstyle-hbase-server.txt
 |
| unit | 

[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Ted Yu (JIRA)


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

Ted Yu commented on HBASE-21062:


lgtm
{code}
+  // we can't us it, we want to fall back to FSHLog which we know works on 
all versions.
{code}
'us it' -> "use it"

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on HBASE-21062:
---

| (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 1 new or modified test 
files. {color} |
|| || || || {color:brown} branch-2.0 Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  2m 
31s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  1m 
40s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  1m 
 8s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green}  3m 
49s{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 
14s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
28s{color} | {color:green} branch-2.0 passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  2m 
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  1m 
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  1m 
41s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  1m  
9s{color} | {color:red} hbase-server: The patch generated 2 new + 19 unchanged 
- 0 fixed = 21 total (was 19) {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 
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 16s{color} | {color:green} Patch does not cause any errors with Hadoop 2.6.5 
2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  2m 
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
31s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}109m 
44s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
21s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}143m  3s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:6f01af0 |
| JIRA Issue | HBASE-21062 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12935742/HBASE-21062.001.branch-2.0.patch
 |
| Optional Tests |  asflicense  javac  javadoc  unit  findbugs  shadedjars  
hadoopcheck  hbaseanti  checkstyle  compile  |
| uname | Linux 6d75256da050 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 
14:43:09 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 | branch-2.0 / 5e06b80e75 |
| maven | version: Apache Maven 3.5.4 
(1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) |
| Default Java | 1.8.0_171 |
| findbugs | v3.1.0-RC3 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-HBASE-Build/14051/artifact/patchprocess/diff-checkstyle-hbase-server.txt
 |
|  Test Results | 
https://builds.apache.org/job/PreCommit-HBASE-Build/14051/testReport/ |
| Max. process+thread count | 3566 (vs. ulimit of 1) |
| modules | C: hbase-server U: hbase-server |

[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Mingliang Liu (JIRA)


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

Mingliang Liu commented on HBASE-21062:
---

+1 (non-binding)

{quote}
if AsyncFSWAL is specified and we can't load it, we should fail.
{quote}
Sorry I missed this point previously. It is right to keep the original behavior.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Josh Elser (JIRA)


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

Josh Elser commented on HBASE-21062:


[~liuml07], thanks for taking a look at v1. What's your take on v2?

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch, 
> HBASE-21062.002.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Josh Elser (JIRA)


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

Josh Elser commented on HBASE-21062:


{quote}I had this working locally and must have botched this before making the 
patch.
{quote}
Oh, I see. When I had that method working earlier, it was only doing this check 
when AsyncFSWALProvider when it was the defaultProvider. I think I accidentally 
undid that work – if AsyncFSWAL is specified and we can't load it, we should 
fail.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Josh Elser (JIRA)


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

Josh Elser commented on HBASE-21062:


{quote}if I revert the not used {{getDefaultProvider}} class (and the override 
in test)
{quote}
Oh! My bad. Let me re-look at this. I had this working locally and must have 
botched this before making the patch.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Mingliang Liu (JIRA)


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

Mingliang Liu commented on HBASE-21062:
---

Thanks [~elserj].

It makes sense to me to keep the minimum change in this patch, so fixing the 
try-catch can be a separate effort (do you mind if I file one?). I like the 
mindset of making the "default" the real "default" being used and not worrying 
about future change. I'm +1 (overall) on the fix in {{getProviderClass}}.

However, as to the test - I think it's hard to test - I found if I revert the 
change in {{getProviderClass}} in you patch, the test will pass; if I revert 
the not used {{getDefaultProvider}} class (and the override in test), it passes 
as well. Again I'm fine if we don't add a new test here.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Josh Elser (JIRA)


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

Josh Elser commented on HBASE-21062:


{quote}It makes sense as the enum {{defaultProvider}} can not have a different 
value at runtime unless we change code
{quote}
My mindset is that the defaultProvider _will_ change at some point in time – 
inevitably. I think the change I've already put forward makes this much more 
clear for someone new to come along and change what the default is without 
having to know what about the impls of a different WALProvider.
{quote}So the test will pass anyway: w/ or w/o the fix in {{getProviderClass}}, 
w/ or w/o the {{getDefaultProvider}}. Correct me if I'm wrong.
{quote}
The test fails without the fix to WALFactory.
{quote}Another (orthogonal?) problem in {{getProviderClass}} is that, the 
try-catch scope is not implemented well
{quote}
I noticed that as well. Without having a clear reason to know why this happened 
(nor, it likely being something that we'd be OK with hitting all the time), I 
ignored that.
{quote}Last, for any error case, it merits to have logging message.
{quote}
That's a good idea.

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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


[jira] [Commented] (HBASE-21062) WALFactory has misleading notion of "default"

2018-08-15 Thread Mingliang Liu (JIRA)


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

Mingliang Liu commented on HBASE-21062:
---

The existing code implicitly employs the fact that {{defaultProvider}} is 
having {{AsyncFSWALProvider}} class value. It makes sense as the enum 
{{defaultProvider}} can not have a different value at runtime unless we change 
code. This implementation pattern is not ideal to me either - because when we 
change the {{defaultProvider}} we may forget to change {{getProviderClass}} - 
but it's tolerable. Meanwhile, the newly added test in the patch does not 
actually override the {{defaultProvider}} value and {{getDefaultProvider}} is 
not used anywhere. So the test will pass anyway: w/ or w/o the fix in 
{{getProviderClass}}, w/ or w/o the {{getDefaultProvider}}. Correct me if I'm 
wrong.

Another (orthogonal?) problem in {{getProviderClass}} is that, the try-catch 
scope is not implemented well. The reason is that, in the 
{{catch(IllegalArgumentException exception)\{...\}}} clause, we end up with 
returning the {{AsyncFSWALProvider}}. However, this time we would skip the 
process that load {{AsyncFSWALProvider}} and then falls back to 
{{FSHLogProvider}}.

Last, for any error case, it merits to have logging message.

Overall, a fix in my mind is like - NOT TESTED:
{code:java}
  @VisibleForTesting
  public Class getProviderClass(String key, String 
defaultValue) {
Providers provider;
try {
   provider = Providers.valueOf(conf.get(key, defaultValue));
} catch (IllegalArgumentException exception) {
  // Fall back to them specifying a class name
  // Note that the passed default class shouldn't actually be used, since 
the above only fails
  // when there is a config value present.
  LOG.warn("Unrecognized WALProvider class. Falling back to default 
AsyncFSWALProvider.");
  provider = Providers.defaultProviderdr;
}
if (provider.clazz == AsyncFSWALProvider.class && 
!AsyncFSWALProvider.load()) {
  // AsyncFSWAL has better performance in most cases, and also uses less 
resources, we will try
  // to use it if possible. But it deeply hacks into the internal of 
DFSClient so will be easily
  // broken when upgrading hadoop. If it is broken, then we fall back to 
use FSHLog.
  LOG.warn("Failed to load AsyncFSWALProvider. Falling back to 
FSHLogProvider.");
  return FSHLogProvider.class;
}
return provider.clazz;
  }
{code}
Thanks!

> WALFactory has misleading notion of "default"
> -
>
> Key: HBASE-21062
> URL: https://issues.apache.org/jira/browse/HBASE-21062
> Project: HBase
>  Issue Type: Bug
>  Components: wal
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Major
> Fix For: 3.0.0, 2.0.2, 2.2.0, 2.1.1
>
> Attachments: HBASE-21062.001.branch-2.0.patch
>
>
> In WALFactory, there is an enum {{Providers}} which has a list of supported 
> WALProvider implementations. In addition to list this, there is also a 
> {{defaultProvider}} (which the Configuration defaults to), that is meant to 
> be our "advertised" default WALProvider.
> However, the implementation of {{getProviderClass}} in WALFactory doesn't 
> actually adhere to the value of this enum, instead *always* returning 
> AsyncFSWal if it can be loaded.
> Having the default value in the enum but then overriding it in the 
> implementation of {{getProviderClass}} is silly and misleading.



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