hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gary Helmling (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-14866) VerifyReplication should use peer configuration in peer connection
Date Thu, 03 Dec 2015 19:37:11 GMT

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

Gary Helmling commented on HBASE-14866:

bq. Should transformClusterKey just use standardizeZKQuorumServerString rather than having
two different cases?

Sure, I can clean that up along the way.

bq. Seems like the entry for shouldbemissing will have two prefixes. Is that intended?

Thanks, nice catch! I'll fix that.

bq. why not move buildZKQuorumServerString and standardizeZKQuorumServerString into ZKUtil?

ZKUtil deals mostly with ZK related operations (adding, watching znodes).  Building a quorum
string is how we handle multiple ZK configs in a Configuration.  So it seems more configuration
related than ZK operation related to me.  In addition ZKUtil is in hbase-client so we can't
use anything in it from hbase-common.  We could move everything, including createClusterConf(),
to ZKUtil, but that seems a weird home for it to me, since the original problem was a failure
to handle additional configuration properties (beyond ZK quorum config) for target/destination

bq. Most of these changes to HBaseConfiguration seem to be very replication specific. Should
we have a different class for replication based configuration, so that HBaseConfiguration
doesn't get too unwieldy?

bq. Agreed. Maybe we need something like ReplicationUtils ?

These changes go beyond replication usage.  This is a common problem wherever a program needs
to handle talking to two clusters with a single Configuration.  This applies to CopyTable,
SyncTable and TableOutputFormat, none of which assumes any replication configuration.  By
comparison, the replication code actually abstracts the usage of these ZK configuration utilities
pretty well, except for the couple of problems in ReplicationAdmin and VerifyReplication.
 In the non-replication cases, we need to be able to handle: applying different ZK quorum
configurations for the different clusters, and overriding other configuration properties (for
example security-related config) for the other clusters.  {{HBaseConfiguration.createClusterConf()}}
is the cleanest way I can see of abstracting this, especially for all of the non-replication
usage.  This also seems like clearly a configuration problem to me, so HBaseConfiguration
seems like the right home.  That is how we handle creating new HBase configurations everywhere
(via {{HBaseConfiguration.create()}}), so this seems analogous.

If we're worried about bloating HBaseConfiguration with the additions moved from ZKUtil, then
I could create a new util class in hbase-common to hold them, but I think we already have
a proliferation of config related methods spread across multiple utility classes:
* ConfigurationUtils in hbase-server -- I would put the methods there, but we need access
to them from hbase-client and hbase-server, so hbase-common seems like the right home.  ConfigurationUtils
is annotated public, so we can't just move it without compatibility concerns.
* ZKUtil in hbase-client -- this class deals mostly with operations on ZooKeeper (adding,
watching znodes), so I think removing all the config methods actually made for a cleaner separation
of ZK operations vs. configuration related manipulations.  Since ZKUtil is in hbase-client
we also can't depend on it from hbase-common.  We could move it to hbase-common, but that
would introduce a new dependency on ZooKeeper in hbase-common that is not currently there.
* ZKConfig in hbase-client -- this currently deals with creating a ZK properties based configuration
for HQuorumPeer.  So again moving the methods there would be expanding what it currently handles
and has the additional problem of being in hbase-client, so createClusterConf() would have
to move there as well.

It seems to me like we have two best options:
* move the ZK related config options to a new private util class in hbase-common.  This could
even be ZKConfig, moved from hbase-client, since it's private.  It would be an expansion of
it's current reponsibilities, but doesn't seem too bad.
* go back to the original targeted fixes to ReplicationAdmin and VerifyReplication, since
those are the actual problems I'm trying to solve.

What do you guys think? I'll hold off on further changes to this until we get some consensus.

> VerifyReplication should use peer configuration in peer connection
> ------------------------------------------------------------------
>                 Key: HBASE-14866
>                 URL: https://issues.apache.org/jira/browse/HBASE-14866
>             Project: HBase
>          Issue Type: Improvement
>          Components: Replication
>            Reporter: Gary Helmling
>            Assignee: Gary Helmling
>             Fix For: 2.0.0, 1.2.0, 1.3.0
>         Attachments: HBASE-14866.patch, HBASE-14866_v1.patch, hbase-14866-v4.patch, hbase-14866_v2.patch,
> VerifyReplication uses the replication peer's configuration to construct the ZooKeeper
quorum address for the peer connection.  However, other configuration properties in the peer's
configuration are dropped.  It should merge all configuration properties from the {{ReplicationPeerConfig}}
when creating the peer connection and obtaining a credentials for the peer cluster.

This message was sent by Atlassian JIRA

View raw message