You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Andrew Purtell (JIRA)" <ji...@apache.org> on 2015/05/27 03:39:17 UTC

[jira] [Commented] (HBASE-13771) Replication clients should not access zookeeper directly

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

Andrew Purtell commented on HBASE-13771:
----------------------------------------

Some random notes:

Replication queue and peer management is almost pluggable in ReplicationAdmin. ReplicationFactory is hardcoded to always create the xxxZKImpl instances. This must be changed to instantiate classes specified in site configuration, using reflection.

When we can plug in alternative implementations, we can switch the default to one that uses a new Master admin API.

ReplicationAdmin unconditionally creates a ZooKeeperWatcher. It's an anonymous inner class that overrides abort().  This is passed through to ReplicationFactory#getReplicationPeers and ReplicationFactory#getReplicationQueuesClient. ReplicationFactory should deal with this, keep it out of ReplicationAdmin. Fortunately ReplicationFactory is private.

Add new API for replication admin as ReplicationAdminService, implement it in MasterRpcServices:
{code}
AddPeerResponse replicationAddPeer(AddPeerRequest req)
RemovePeerResponse replicationRemovePeer(RemovePeerRequest req)
EnablePeerResponse replicationEnablePeer(EnablePeerRequest req)
DisablePeerResponse replicationDisablePeer(DisablePeerRequest req)
ListPeersResponse replicationListPeers(ListPeersRequest req)
ListPeerConfigsResponse replicationListPeerConfigs(ListPeerConfigsRequest req)
GetPeerConfigResponse replicationGetPeerConfig(GetPeerConfigRequest req)
GetPeerConfigResponse replicationGetPeerConfig(GetPeerConfigRequest req)
GetPeerTableCFsResponse replicationGetPeerTableCFs(GetPeerTableCFsRequest req)
SetPeerTableCFsResponse replicationSetPeerTableCFs(SetPeerTableCFsRequest req)
AppendPeerTableCFsResponse replicationAppendPeerTableCFs(AppendPeerTableCFsRequest req)
RemovePeerTableCFsResponse replicationRemovePeerTableCFs(RemovePeerTableCFsRequest req)
GetPeerStateResponse replicationGetPeerState(GetPeerStateRequest req)
{code}

Add protobuf message definitions for above new xxxResponse and xxxRequest types. Could prefix them with "Replication" like the interface methods. See about consolidating the message types for xxxPeerTableCFs methods.

The Master can simply create ReplicationQueuesClientZKImpl and ReplicationPeersZKImpl instances and delegate to them from the ReplicationAdminService implementation, until we move away from managing state in ZK (HBASE-13773). 

ReplicationAdmin#listValidReplicationPeers will not need to check state in ZK or call the new replicationGetPeerState API repeatedly if ListPeersResponse includes the information it needs.

Other ReplicationAdmin methods use Admin APIs already.

> Replication clients should not access zookeeper directly
> --------------------------------------------------------
>
>                 Key: HBASE-13771
>                 URL: https://issues.apache.org/jira/browse/HBASE-13771
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 2.0.0, 1.0.1, 1.1.0, 0.98.12, 1.2.0
>            Reporter: Andrew Purtell
>            Priority: Critical
>
> Replication client actions set and modify znodes directly. This is legacy from an era before we had the AccessController available and missing coverage of admin action in our security model. 
> All replication client actions should be mediated by the master, and hooked up to the coprocessor framework for use by the AccessController. After adding this functionality we should should restrict access to replication znodes to only the HBase service principal, but provide a configuration option to relax those permissions for as long as older admin clients are in use (with a stern suggestion to upgrade ASAP). This type of functional change, with optional backwards compatibility, should be fine for all branches.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)