[
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)
