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

["Mingliang Liu (JIRA)" <ji...@apache.org>]

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

Mingliang Liu edited comment on HBASE-21062 at 8/15/18 9:24 PM:
----------------------------------------------------------------

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 (non-binding) 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.


was (Author: liuml07):
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)