From cafa449769fb131830ede129910287670625aa0d Mon Sep 17 00:00:00 2001
From: noblepaul <noble.paul@gmail.com>
Date: Thu, 24 Sep 2020 17:26:41 +1000
Subject: [PATCH] SOLR-14613: Avoid multiple ZK write

---
 .../org/apache/solr/handler/ClusterAPI.java    | 18 +++++++-----------
 .../solr/common/cloud/ClusterProperties.java   | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java b/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
index ad3e796068c..8667a87f4ad 100644
--- a/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
@@ -17,7 +17,6 @@
 
 package org.apache.solr.handler;
 
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -28,6 +27,7 @@ import org.apache.solr.client.solrj.request.beans.ClusterPropInfo;
 import org.apache.solr.client.solrj.request.beans.CreateConfigInfo;
 import org.apache.solr.cloud.OverseerConfigSetMessageHandler;
 import org.apache.solr.cluster.placement.impl.PlacementPluginConfigImpl;
+import org.apache.solr.common.MapWriterMap;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.cloud.ClusterProperties;
@@ -212,22 +212,18 @@ public class ClusterAPI {
     @Command(name = "set-placement-plugin")
     public void setPlacementPlugin(PayloadObj<Map<String, Object>> obj) {
       Map<String, Object> placementPluginConfig = obj.getDataMap();
+      if(placementPluginConfig.isEmpty()) placementPluginConfig = null;
       ClusterProperties clusterProperties = new ClusterProperties(getCoreContainer().getZkController().getZkClient());
       // When the json contains { "set-placement-plugin" : null }, the map is empty, not null.
-      final boolean unset = placementPluginConfig.isEmpty();
       // Very basic sanity check. Real validation will be done when the config is used...
-      if (!unset && !placementPluginConfig.containsKey(PlacementPluginConfigImpl.CONFIG_CLASS)) {
+      if (!(placementPluginConfig == null) && !placementPluginConfig.containsKey(PlacementPluginConfigImpl.CONFIG_CLASS)) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Must contain " + PlacementPluginConfigImpl.CONFIG_CLASS + " attribute (or be null)");
       }
       try {
-        // Need to reset to null first otherwise the mappings in placementPluginConfig are added to existing ones
-        // in /clusterprops.json rather than replacing them. If removing the config, that's all we do.
-        clusterProperties.setClusterProperties(
-                Collections.singletonMap(PlacementPluginConfigImpl.PLACEMENT_PLUGIN_CONFIG_KEY, null));
-        if (!unset) {
-          clusterProperties.setClusterProperties(
-                  Collections.singletonMap(PlacementPluginConfigImpl.PLACEMENT_PLUGIN_CONFIG_KEY, placementPluginConfig));
-        }
+        clusterProperties.update(placementPluginConfig == null?
+            null:
+            new MapWriterMap(placementPluginConfig),
+            PlacementPluginConfigImpl.PLACEMENT_PLUGIN_CONFIG_KEY);
       } catch (Exception e) {
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error in API", e);
       }
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterProperties.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterProperties.java
index 2f0779cbd06..0460c4637e0 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterProperties.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterProperties.java
@@ -19,11 +19,13 @@ package org.apache.solr.common.cloud;
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.util.Utils;
@@ -116,6 +118,21 @@ public class ClusterProperties {
     });
   }
 
+
+  /**Set this object at the json path
+   *
+   * @param obj the Object to be set
+   * @param path the json path
+   */
+  @SuppressWarnings("unchecked")
+  public void update(MapWriter obj, String... path) throws KeeperException, InterruptedException{
+    client.atomicUpdate(ZkStateReader.CLUSTER_PROPS, bytes -> {
+      Map<String, Object> zkJson = (Map<String, Object>) Utils.fromJSON(bytes);
+      Utils.setObjectByPath(zkJson, Arrays.asList(path), obj);
+      return Utils.toJSON(zkJson);
+    });
+  }
+
   /**
    * See SOLR-12827 for background. We auto convert any "collectionDefaults" keys to "defaults/collection" format.
    * This method will modify the given map and return the same object. Remove this method in Solr 9.