[
https://issues.apache.org/jira/browse/HBASE-21247?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16658950#comment-16658950
]
Sean Busbey commented on HBASE-21247:
-------------------------------------
okay I've made a first pass. I like the added test and the correction on
IOTestProvider.
I agree that your current solution solves the immediate issue, but I think it's
confusing to follow and will be hard to maintain. In particular:
{code}
// 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.
- return conf.getClass(key, Providers.defaultProvider.clazz,
WALProvider.class);
+ // If meta WAL provider is not specified, use provider class of ordinary
WAL
+ return conf.getClass(key, key.equals(META_WAL_PROVIDER) &&
conf.get(META_WAL_PROVIDER)==null ?
+ conf.getClass(WAL_PROVIDER, Providers.defaultProvider.clazz,
WALProvider.class) :
+ Providers.defaultProvider.clazz,
+ WALProvider.class);
{code}
It's odd to suddenly read a bunch of stuff about the specific provider keys at
the end of a method that was generic and reusable before then. Additionally,
the comment says "we won't use this default because of XXX" and then we do a
bunch of logic to determine what that default ought to be.
I think the fundamental issue is that the way HBASE-20856 was implemented
invalidated the assumptions of this method; specifically that the default
passed to {{getProviderClass}} would be a member of the Providers enum. I think
it'll be easier to maintain this long term if we either restore that assumption
or make it no longer needed, rather than do a second round of configuration
lookups in this exception handling block.
What if we refactor {{getProviderClass}} to take a {{Class}} as the
defaultValue rather than a string version? We can get a String out of it for
the call to {{Providers.valueOf}} and then use it directly if needed when we
call {{Configuration.getClass}}.
> Custom WAL Provider cannot be specified by configuration whose value is
> outside the enums in Providers
> ------------------------------------------------------------------------------------------------------
>
> Key: HBASE-21247
> URL: https://issues.apache.org/jira/browse/HBASE-21247
> Project: HBase
> Issue Type: Bug
> Reporter: Ted Yu
> Assignee: Ted Yu
> Priority: Major
> Fix For: 3.0.0
>
> Attachments: 21247.v1.txt, 21247.v2.txt, 21247.v3.txt, 21247.v4.tst,
> 21247.v4.txt, 21247.v5.txt, 21247.v6.txt, 21247.v7.txt, 21247.v8.txt
>
>
> Currently all the WAL Providers acceptable to hbase are specified in
> Providers enum of WALFactory.
> This restricts the ability for additional WAL Providers to be supplied - by
> class name.
> This issue fixes the bug by allowing the specification of new WAL Provider
> class name using the config "hbase.wal.provider".
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
