gjacoby126 commented on a change in pull request #2778:
URL: https://github.com/apache/hbase/pull/2778#discussion_r542727432
##########
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
+ if (removeBasePeerConfigs.length() != 0) {
+ List<String> removeBasePeerConfigList = Splitter.on(';').trimResults()
+ .omitEmptyStrings().splitToList(removeBasePeerConfigs);
+ for (String peerConfigToRemove : removeBasePeerConfigList) {
+ copiedPeerConfigBuilder.removeConfiguration(peerConfigToRemove);
+ }
+ }
+
+ Map<String, String> receivedPeerConfigMap =
receivedPeerConfig.getConfiguration();
+ String basePeerConfigs = conf.get(HBASE_REPLICATION_PEER_BASE_CONFIG, "");
Review comment:
What happens if I try to add a peer config setting and remove the same
peer config setting in the same Configuration object? Looks like the add just
silently wins? I'd think that should produce an exception (or at the very least
a log).
##########
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:
Because configuration settings are often from hbase-site.xml, and
hbase-site.xml can be different on different region servers (e.g when upgrading
a cluster), it would seem like you could get into some unwelcome race
conditions with this approach.
Let's say that you're doing a rolling restart from Version 1 to Version 2.
Version 1 has a config C == true, Version 2 tries to remove C.
1. Server 1 is upgraded to Version 2 and restarts, and removes C from the
peer config
2. Server 2 is still on Version 1 and is restarted for an unrelated reason
(maybe it crashes and is restarted). It sees that C is not in the peer config,
and recreates it.
Assuming that you upgraded hbase-site.xml everywhere and restarted, the
cluster would _eventually_ converge on the right answer, but there's a lot of
opportunity for the config to flicker back and forth in the meantime. Depending
on the config, that could have incorrect or unpredictable results
----------------------------------------------------------------
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]
