[
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 clusters.
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, hbase-14866_v3.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
(v6.3.4#6332)
