bharathv commented on a change in pull request #2778:
URL: https://github.com/apache/hbase/pull/2778#discussion_r543078007
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationPeerConfigUtil.java
##########
@@ -64,6 +65,9 @@
public static final String HBASE_REPLICATION_PEER_BASE_CONFIG =
"hbase.replication.peer.base.config";
+ public static final String HBASE_REPLICATION_PEER_REMOVE_BASE_CONFIG =
Review comment:
I think this approach can be better, it seems confusing that one would
add a configuration to remove another configuration? (and the community is
generally against adding such esoteric configs)
What happens if someone forgets to undo this and that has some adverse
affect after a next restart..
Why not just consider "" config as a special case and then reset the configs
in the copiedConfigBuilder?
Something like
initially someone set it k1=v1;k2=v2....
then they can set it to "" and restart master
detect that "" has been set (which is different from what we already have)
and remove whatever we already have populated?
To avoid config divergence, we can disallow
hbase.replication.peer.base.config to be done via updateConfig RPC (throw some
sensible exception)..
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationPeerConfigUtil.java
##########
@@ -455,36 +458,45 @@ public static ReplicationPeerConfig
appendTableCFsToReplicationPeerConfig(
/**
* Helper method to add base peer configs from Configuration to
ReplicationPeerConfig
- * if not present in latter.
*
* This merges the user supplied peer configuration
* {@link org.apache.hadoop.hbase.replication.ReplicationPeerConfig} with
peer configs
* provided as property hbase.replication.peer.base.configs in hbase
configuration.
- * Expected format for this hbase configuration is "k1=v1;k2=v2,v2_1".
Original value
- * of conf is retained if already present in ReplicationPeerConfig.
+ * Expected format for this hbase configuration is "k1=v1;k2=v2,v2_1".
*
* @param conf Configuration
* @return ReplicationPeerConfig containing updated configs.
*/
- public static ReplicationPeerConfig
addBasePeerConfigsIfNotPresent(Configuration conf,
+ public static ReplicationPeerConfig
updateReplicationBasePeerConfigs(Configuration conf,
ReplicationPeerConfig receivedPeerConfig) {
- String basePeerConfigs = conf.get(HBASE_REPLICATION_PEER_BASE_CONFIG, "");
+ String removeBasePeerConfigs =
conf.get(HBASE_REPLICATION_PEER_REMOVE_BASE_CONFIG, "");
ReplicationPeerConfigBuilder copiedPeerConfigBuilder =
ReplicationPeerConfig.
newBuilder(receivedPeerConfig);
- Map<String,String> receivedPeerConfigMap =
receivedPeerConfig.getConfiguration();
+ // remove the peer configurations specified in the conf
Review comment:
Sorry just got to this PR. Thats right, master branch replication
coordination is different from branch-1. This was specifically taken into
consideration in the original PR that added this feature (see the commit
message for branch-1)..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
