[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16804375#comment-16804375 ] Ankit Singhal commented on HBASE-21456: --- bq, I see a lot of changes where we just pass down null, true as the third and fourth arguments to createReader. Would it make sense to overload the createReader method, creating a 2-arg and a 4-arg version? Yep, As these are WalProvider interface method so overloading may not be possible until we do it through WalProviderFactory or specific implementation. But anyways, these methods argument list is gonna change in the next patch for WAL API. bq. Overall, I think this looks good. I think it helps clear up some of the ambiguity we had before. Have you investigated if the test values are related to your patch? I have not seen these failures with other pre-commit jobs(for other patches) so could be related to this, but when I ran them locally, they are passing without any issue. so I'll keep an eye on them specifically when working through my another patch. Thanks [~elserj] for review the patch, If everything looks ok for now , can we commit this? > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Affects Versions: HBASE-20952 >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Fix For: HBASE-20952 > > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.002.patch, HBASE-21456.HBASE-20952.003.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803280#comment-16803280
]
Josh Elser commented on HBASE-21456:
{noformat}
- reader = WALFactory.createReader(fs, edits, conf);
+ reader = WALProviderFactory.getInstance(conf).createReader(fs, edits,
null, true);{noformat}
I see a lot of changes where we just pass down {{null, true}} as the third and
fourth arguments to {{createReader}}. Would it make sense to overload the
{{createReader}} method, creating a 2-arg and a 4-arg version?
Overall, I think this looks good. I think it helps clear up some of the
ambiguity we had before. Have you investigated if the test values are related
to your patch?
> Make WALFactory only used for creating WALProviders
> ---
>
> Key: HBASE-21456
> URL: https://issues.apache.org/jira/browse/HBASE-21456
> Project: HBase
> Issue Type: Sub-task
> Components: wal
>Affects Versions: HBASE-20952
>Reporter: Josh Elser
>Assignee: Ankit Singhal
>Priority: Major
> Fix For: HBASE-20952
>
> Attachments: HBASE-21456.HBASE-20952.001.patch,
> HBASE-21456.HBASE-20952.002.patch, HBASE-21456.HBASE-20952.003.patch,
> HBASE-21456.HBASE-20952.wip.patch
>
>
> As a Factory, WALFactory should only have one job: creating instances of
> WALProvider.
> However, over the years, it has been a landing place for lots of wal-related
> methods (e.g. constructing readers, WALEntryStream, and more). We want all of
> this to live in the WALProvider.
> We can do this in two steps: have the WALFactory methods invoke the method on
> the WALProvider (handled elsewhere), then here we can replace usage of the
> WALFactory "wrapper methods" with the WALProvider itself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16802451#comment-16802451
]
Hadoop QA commented on HBASE-21456:
---
| (x) *{color:red}-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 64 new or modified test
files. {color} |
|| || || || {color:brown} HBASE-20952 Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
25s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 5m
30s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
25s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
8s{color} | {color:green} HBASE-20952 passed {color} |
| {color:blue}0{color} | {color:blue} refguide {color} | {color:blue} 5m
29s{color} | {color:blue} branch has no errors when building the reference
guide. See footer for rendered docs, which you should manually inspect. {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} 4m
11s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
13s{color} | {color:green} HBASE-20952 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
1s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
13s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 1m 48s{color}
| {color:red} hbase-server generated 1 new + 187 unchanged - 1 fixed = 188
total (was 188) {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m
13s{color} | {color:red} hbase-server: The patch generated 2 new + 931
unchanged - 5 fixed = 933 total (was 936) {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} xml {color} | {color:green} 0m
2s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:blue}0{color} | {color:blue} refguide {color} | {color:blue} 5m
10s{color} | {color:blue} patch has no errors when building the reference
guide. See footer for rendered docs, which you should manually inspect. {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}
8m 34s{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
24s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
30s{color} | {color:red} hbase-server generated 1 new + 0 unchanged - 0 fixed =
1 total (was 0) {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
40s{color} | {color:green} hbase-common in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red}271m 48s{color}
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 24m
21s{color} | {color:green} hbase-mapreduce in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 19m
6s{color} | {color:green} hbase-backup in the patch passed. {color} |
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16802378#comment-16802378
]
Hadoop QA commented on HBASE-21456:
---
| (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 65 new or modified test
files. {color} |
|| || || || {color:brown} HBASE-20952 Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
22s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
2s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
53s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
40s{color} | {color:green} HBASE-20952 passed {color} |
| {color:blue}0{color} | {color:blue} refguide {color} | {color:blue} 6m
8s{color} | {color:blue} branch has no errors when building the reference
guide. See footer for rendered docs, which you should manually inspect. {color}
|
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 4m
20s{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
34s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
27s{color} | {color:green} HBASE-20952 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
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
42s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 1m 56s{color}
| {color:red} hbase-server generated 1 new + 187 unchanged - 1 fixed = 188
total (was 188) {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m
12s{color} | {color:red} hbase-zookeeper: The patch generated 5 new + 0
unchanged - 0 fixed = 5 total (was 0) {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m
32s{color} | {color:red} hbase-server: The patch generated 2 new + 931
unchanged - 5 fixed = 933 total (was 936) {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} xml {color} | {color:green} 0m
2s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:blue}0{color} | {color:blue} refguide {color} | {color:blue} 5m
39s{color} | {color:blue} patch has no errors when building the reference
guide. See footer for rendered docs, which you should manually inspect. {color}
|
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 4m
8s{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 21s{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
59s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
31s{color} | {color:red} hbase-server generated 1 new + 0 unchanged - 0 fixed =
1 total (was 0) {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
28s{color} | {color:green} hbase-common in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 1m 23s{color}
| {color:red} hbase-zookeeper in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red}149m 5s{color}
| {color:red}
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16802240#comment-16802240 ] Ankit Singhal commented on HBASE-21456: --- .003 should fix the test failures and checkstyle errors. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Affects Versions: HBASE-20952 >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Fix For: HBASE-20952 > > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.002.patch, HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16788524#comment-16788524
]
Hadoop QA commented on HBASE-21456:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
23s{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 63 new or modified test
files. {color} |
|| || || || {color:brown} HBASE-20952 Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
28s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
30s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
7s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
5s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 4m
41s{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
32s{color} | {color:green} HBASE-20952 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
3s{color} | {color:green} HBASE-20952 passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
19s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:red}-1{color} | {color:red} mvninstall {color} | {color:red} 4m
6s{color} | {color:red} root in the patch failed. {color} |
| {color:red}-1{color} | {color:red} compile {color} | {color:red} 0m
30s{color} | {color:red} hbase-mapreduce in the patch failed. {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 2m 18s{color}
| {color:red} hbase-server generated 1 new + 187 unchanged - 1 fixed = 188
total (was 188) {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 0m 30s{color}
| {color:red} hbase-mapreduce in the patch failed. {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m
34s{color} | {color:red} hbase-server: The patch generated 72 new + 934
unchanged - 2 fixed = 1006 total (was 936) {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m
21s{color} | {color:red} hbase-mapreduce: The patch generated 1 new + 2
unchanged - 0 fixed = 3 total (was 2) {color} |
| {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m
0s{color} | {color:red} The patch has 9 line(s) that end in whitespace. Use git
apply --whitespace=fix <>. Refer https://git-scm.com/docs/git-apply
{color} |
| {color:red}-1{color} | {color:red} shadedjars {color} | {color:red} 4m
5s{color} | {color:red} patch has 14 errors when building our shaded downstream
artifacts. {color} |
| {color:red}-1{color} | {color:red} hadoopcheck {color} | {color:red} 2m
30s{color} | {color:red} The patch causes 14 errors with Hadoop v2.7.4. {color}
|
| {color:red}-1{color} | {color:red} hadoopcheck {color} | {color:red} 5m
10s{color} | {color:red} The patch causes 14 errors with Hadoop v3.0.0. {color}
|
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 0m
23s{color} | {color:red} hbase-mapreduce in the patch failed. {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
37s{color} | {color:red} hbase-server generated 1 new + 0 unchanged - 0 fixed =
1 total (was 0) {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}271m 41s{color}
| {color:red} hbase-server in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 0m 38s{color}
| {color:red} hbase-mapreduce in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
53s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}319m 34s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16788294#comment-16788294 ] Josh Elser commented on HBASE-21456: [~an...@apache.org], actually, this doesn't apply against HBASE-20952 for me. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16788293#comment-16788293 ] Josh Elser commented on HBASE-21456: Fine by me. Let me land this one quick to unblock you :) > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16788279#comment-16788279
]
Ankit Singhal commented on HBASE-21456:
---
{quote}Why have both a public constructor and the static {{getInstance()}}
method? Can we consolidate in one direction?\{quote}
Yes, agreed, we don't need both, working on refactoring the same along with the
current work (expecting to have public constructor instead of getInstance() and
move same factory object all around the code).
{quote}Trying to think about how to help folks like Stack and Reid: what about
an issue to start updating the book? A (short) chapter on how we see WALs
working that briefly illustrate the design would help (and we'd probably write
such things later, anyways). This would be helpful to cover things like:\{quote}
Good point, will be glad to work on this , I'll pick this up immediately after
completing the current required API work for reader/writer.
How about committig this one in branch as will be producing the another patch
on the top of it for another Jira which will take care of above review comment
as well?
> Make WALFactory only used for creating WALProviders
> ---
>
> Key: HBASE-21456
> URL: https://issues.apache.org/jira/browse/HBASE-21456
> Project: HBase
> Issue Type: Sub-task
> Components: wal
>Reporter: Josh Elser
>Assignee: Ankit Singhal
>Priority: Major
> Attachments: HBASE-21456.HBASE-20952.001.patch,
> HBASE-21456.HBASE-20952.wip.patch
>
>
> As a Factory, WALFactory should only have one job: creating instances of
> WALProvider.
> However, over the years, it has been a landing place for lots of wal-related
> methods (e.g. constructing readers, WALEntryStream, and more). We want all of
> this to live in the WALProvider.
> We can do this in two steps: have the WALFactory methods invoke the method on
> the WALProvider (handled elsewhere), then here we can replace usage of the
> WALFactory "wrapper methods" with the WALProvider itself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16786364#comment-16786364 ] Reid Chan commented on HBASE-21456: --- +1, let's make some progress. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16784778#comment-16784778
]
Josh Elser commented on HBASE-21456:
Looking back at the 001 patch,
* Why have both a public constructor and the static {{getInstance()}} method?
Can we consolidate in one direction?
Trying to think about how to help folks like Stack and Reid: what about an
issue to start updating the book? A (short) chapter on how we see WALs working
that briefly illustrate the design would help (and we'd probably write such
things later, anyways). This would be helpful to cover things like:
* What does a WALProvider do?
* How do I read to a WAL?
* How do I write to a WAL?
* How do I "find"/get my WAL?
* What happened to WALFactory?
> Make WALFactory only used for creating WALProviders
> ---
>
> Key: HBASE-21456
> URL: https://issues.apache.org/jira/browse/HBASE-21456
> Project: HBase
> Issue Type: Sub-task
> Components: wal
>Reporter: Josh Elser
>Assignee: Ankit Singhal
>Priority: Major
> Attachments: HBASE-21456.HBASE-20952.001.patch,
> HBASE-21456.HBASE-20952.wip.patch
>
>
> As a Factory, WALFactory should only have one job: creating instances of
> WALProvider.
> However, over the years, it has been a landing place for lots of wal-related
> methods (e.g. constructing readers, WALEntryStream, and more). We want all of
> this to live in the WALProvider.
> We can do this in two steps: have the WALFactory methods invoke the method on
> the WALProvider (handled elsewhere), then here we can replace usage of the
> WALFactory "wrapper methods" with the WALProvider itself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16745470#comment-16745470
]
Josh Elser commented on HBASE-21456:
{quote}Just wondering if a write-up/plan/spec to consult to help navigate the
changes. Would address comments like mine and [~reidchan]'s where our first
blush is to ask why the change at all? Thanks.
{quote}
I think this came up in the doc that Ted was putting together a few months
back. The high level goals are still the same there: all WAL related operations
go through WALProvider, and get pulled out of other places.
To this specific patch, I see this as a natural progression. I think keeping
WALFactory around with its current methods just adds more indirection without
much value. The final state would be WAL(Provider)Factory instantiates a
WALProvider for you, given the configuration. WALProvider gives you the
interface to do "all things WAL" (read, write, create), returning you a WAL (or
Reader or Writer) when relevant.
Does that help, or would you rather see more than just this?
> Make WALFactory only used for creating WALProviders
> ---
>
> Key: HBASE-21456
> URL: https://issues.apache.org/jira/browse/HBASE-21456
> Project: HBase
> Issue Type: Sub-task
> Components: wal
>Reporter: Josh Elser
>Assignee: Ankit Singhal
>Priority: Major
> Attachments: HBASE-21456.HBASE-20952.001.patch,
> HBASE-21456.HBASE-20952.wip.patch
>
>
> As a Factory, WALFactory should only have one job: creating instances of
> WALProvider.
> However, over the years, it has been a landing place for lots of wal-related
> methods (e.g. constructing readers, WALEntryStream, and more). We want all of
> this to live in the WALProvider.
> We can do this in two steps: have the WALFactory methods invoke the method on
> the WALProvider (handled elsewhere), then here we can replace usage of the
> WALFactory "wrapper methods" with the WALProvider itself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16741397#comment-16741397 ] stack commented on HBASE-21456: --- I'm good w/ it all. Just wondering if a write-up/plan/spec to consult to help navigate the changes. Would address comments like mine and [~reidchan]'s where our first blush is to ask why the change at all? Thanks. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16740883#comment-16740883
]
Ankit Singhal commented on HBASE-21456:
---
{quote}Do we reach a consensus, or any info i needed to be synced?
{quote}
{quote}I have similar sort of question: whats wrong w/ WALFactory being the
place you go to to manufacture anything to do w/ WALs...? Obviously you have
some larger plan in mind. Is there a pointer? Thanks.
{quote}
Sorry [~reidchan], I thought consensus was reached after your comment, but it
seems not ,that's my bad.
[~stack], I think(and this probably was also the same thought when Josh
created this Jira) that currently there is a confusion in responsibility of
walFactory vs walProvider. As with new API's added in walProvider, it is better
to access walProvider directly instead creating a shim for them in the factory
and on the other hand , we can redefine WalFactory (or WalProviderFactory) to
just help in initializing the configured WAL provider and let other WAL related
stuff to provider itself.
But that's all said, would like to hear your thought on the need of having
WalFactory intercepting all the request made to the provider.
> Make WALFactory only used for creating WALProviders
> ---
>
> Key: HBASE-21456
> URL: https://issues.apache.org/jira/browse/HBASE-21456
> Project: HBase
> Issue Type: Sub-task
> Components: wal
>Reporter: Josh Elser
>Assignee: Ankit Singhal
>Priority: Major
> Attachments: HBASE-21456.HBASE-20952.001.patch,
> HBASE-21456.HBASE-20952.wip.patch
>
>
> As a Factory, WALFactory should only have one job: creating instances of
> WALProvider.
> However, over the years, it has been a landing place for lots of wal-related
> methods (e.g. constructing readers, WALEntryStream, and more). We want all of
> this to live in the WALProvider.
> We can do this in two steps: have the WALFactory methods invoke the method on
> the WALProvider (handled elsewhere), then here we can replace usage of the
> WALFactory "wrapper methods" with the WALProvider itself.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16739017#comment-16739017 ] stack commented on HBASE-21456: --- [~an...@apache.org] See [~reidchan]'s note above sir. I have similar sort of question: whats wrong w/ WALFactory being the place you go to to manufacture anything to do w/ WALs...? Obviously you have some larger plan in mind. Is there a pointer? Thanks. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16738752#comment-16738752 ] Ankit Singhal commented on HBASE-21456: --- Thanks [~elserj] for taking a look. bq. what if we rename WALFactory to WALProviderFactory Done in the latest patch, have not yet removed the dependency of filesystem from reader and writer, will do it when updating the write and read path. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.001.patch, > HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16737437#comment-16737437 ] Josh Elser commented on HBASE-21456: Glanced at the WIP patch briefly: only thing that jumped out at me so far – what if we rename WALFactory to WALProviderFactory. I'm not usually one for renaming quickly, but I think that will make things more semantically correct :) > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16733680#comment-16733680 ] Ankit Singhal commented on HBASE-21456: --- Uploading wip patch, as there are some test failures which needs to be taken care and better handling of delegate WAL providers. > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Assignee: Ankit Singhal >Priority: Major > Attachments: HBASE-21456.HBASE-20952.wip.patch > > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21456) Make WALFactory only used for creating WALProviders
[ https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16683874#comment-16683874 ] Reid Chan commented on HBASE-21456: --- Looks like Ted is working on it, but still in HBASE-21246. Do we reach a consensus, or any info i needed to be synced? > Make WALFactory only used for creating WALProviders > --- > > Key: HBASE-21456 > URL: https://issues.apache.org/jira/browse/HBASE-21456 > Project: HBase > Issue Type: Sub-task > Components: wal >Reporter: Josh Elser >Priority: Major > > As a Factory, WALFactory should only have one job: creating instances of > WALProvider. > However, over the years, it has been a landing place for lots of wal-related > methods (e.g. constructing readers, WALEntryStream, and more). We want all of > this to live in the WALProvider. > We can do this in two steps: have the WALFactory methods invoke the method on > the WALProvider (handled elsewhere), then here we can replace usage of the > WALFactory "wrapper methods" with the WALProvider itself. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
