[ 
https://issues.apache.org/jira/browse/HBASE-21456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)

Reply via email to