You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2023/07/17 20:49:25 UTC

[solr] branch branch_9_3 updated: SOLR-16891: Fix blank Cloud graph screen in UI (#1783)

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch branch_9_3
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9_3 by this push:
     new de33f50ce79 SOLR-16891: Fix blank Cloud graph screen in UI (#1783)
de33f50ce79 is described below

commit de33f50ce79ec1d156faf204553012037e2bc1cb
Author: Houston Putman <ho...@apache.org>
AuthorDate: Mon Jul 17 16:45:33 2023 -0400

    SOLR-16891: Fix blank Cloud graph screen in UI (#1783)
    
    * MapWriter and IteratorWriter should not be special cases for JSON writing
    * DocCollection, Slice and Replica now write to Json and write to Maps identically.
    
    (cherry picked from commit 952a7be422b1173683c1b70fc407b93b13cfb6f1)
---
 solr/CHANGES.txt                                   |  2 +
 .../solr/handler/admin/ZookeeperInfoHandler.java   |  8 +-
 .../org/apache/solr/common/IteratorWriter.java     | 29 ++++++-
 .../src/java/org/apache/solr/common/MapWriter.java | 31 ++++++-
 .../org/apache/solr/common/cloud/ClusterState.java | 10 +--
 .../apache/solr/common/cloud/DocCollection.java    | 14 ++--
 .../java/org/apache/solr/common/cloud/Replica.java | 41 ++++-----
 .../java/org/apache/solr/common/cloud/Slice.java   |  6 --
 .../org/apache/solr/common/cloud/ZkNodeProps.java  |  8 +-
 .../solr/common/util/MapWriterJSONWriter.java      | 97 ----------------------
 .../java/org/apache/solr/common/util/Utils.java    |  3 +-
 11 files changed, 89 insertions(+), 160 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 8705ad1d083..4223b613c29 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -265,6 +265,8 @@ Bug Fixes
 
 * SOLR-16753: PRS state is now always updated at the end of a splitShard. (Houston Putman, hossman)
 
+* SOLR-16891: DocCollection, Slice and Replica now write to Json and write to Maps identically. (Houston Putman, Tomás Fernández Löbbe)
+
 Dependency Upgrades
 ---------------------
 * PR#1744: Update software.amazon.awssdk:* to v2.20.97 (solrbot)
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java
index e484a8a0722..161676fd696 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java
@@ -31,6 +31,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.Date;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -55,7 +56,6 @@ import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.ContentStream;
-import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.handler.RequestHandlerBase;
 import org.apache.solr.request.SolrQueryRequest;
@@ -529,11 +529,7 @@ public final class ZookeeperInfoHandler extends RequestHandlerBase {
           if (dc != null) {
             // TODO: for collections with perReplicaState, a ser/deser to JSON was needed to get the
             // state to render correctly for the UI?
-            @SuppressWarnings("unchecked")
-            Map<String, Object> collectionState =
-                dc.isPerReplicaState()
-                    ? (Map<String, Object>) Utils.fromJSONString(Utils.toJSONString(dc))
-                    : dc.getProperties();
+            Map<String, Object> collectionState = dc.toMap(new LinkedHashMap<>());
             if (applyStatusFilter) {
               // verify this collection matches the filtered state
               if (page.matchesStatusFilter(collectionState, liveNodes)) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java b/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java
index bc29536ef4e..f321646a5c6 100644
--- a/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/IteratorWriter.java
@@ -21,9 +21,10 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
+import org.noggit.JSONWriter;
 
 /** Interface to help do push writing to an array */
-public interface IteratorWriter {
+public interface IteratorWriter extends JSONWriter.Writable {
   /**
    * @param iw after this method returns , the ItemWriter Object is invalid Do not hold a reference
    *     to this object
@@ -86,4 +87,30 @@ public interface IteratorWriter {
     }
     return l;
   }
+
+  @Override
+  default void write(JSONWriter writer) {
+    writer.startArray();
+    try {
+      writeIter(
+          new IteratorWriter.ItemWriter() {
+            boolean first = true;
+
+            @Override
+            public IteratorWriter.ItemWriter add(Object o) {
+              if (first) {
+                first = false;
+              } else {
+                writer.writeValueSeparator();
+              }
+              writer.indent();
+              write(writer);
+              return this;
+            }
+          });
+    } catch (IOException e) {
+      throw new RuntimeException("this should never happen", e);
+    }
+    writer.endArray();
+  }
 }
diff --git a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
index fb05d422a34..0bbe2dc50a0 100644
--- a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
@@ -24,13 +24,14 @@ import java.util.function.BiConsumer;
 import java.util.function.BiPredicate;
 import java.util.function.Supplier;
 import org.apache.solr.common.util.Utils;
+import org.noggit.JSONWriter;
 
 /**
  * Use this class to push all entries of a Map into an output. This avoids creating map instances
  * and is supposed to be memory efficient. If the entries are primitives, unnecessary boxing is also
  * avoided.
  */
-public interface MapWriter extends MapSerializable, NavigableObject {
+public interface MapWriter extends MapSerializable, NavigableObject, JSONWriter.Writable {
 
   default String jsonStr() {
     return Utils.toJSONString(this);
@@ -41,6 +42,34 @@ public interface MapWriter extends MapSerializable, NavigableObject {
     return Utils.convertToMap(this, map);
   }
 
+  @Override
+  default void write(JSONWriter writer) {
+    writer.startObject();
+    try {
+      writeMap(
+          new MapWriter.EntryWriter() {
+            boolean first = true;
+
+            @Override
+            public MapWriter.EntryWriter put(CharSequence k, Object v) {
+              if (first) {
+                first = false;
+              } else {
+                writer.writeValueSeparator();
+              }
+              writer.indent();
+              writer.writeString(k.toString());
+              writer.writeNameSeparator();
+              writer.write(v);
+              return this;
+            }
+          });
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+    writer.endObject();
+  }
+
   void writeMap(EntryWriter ew) throws IOException;
 
   default MapWriter append(MapWriter another) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
index 6d74dd49015..4418d1edaa9 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
@@ -18,6 +18,7 @@ package org.apache.solr.common.cloud;
 
 import static org.apache.solr.common.util.Utils.STANDARDOBJBUILDER;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.Collection;
 import java.util.Collections;
@@ -30,6 +31,7 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.DocCollection.CollectionStateProps;
@@ -48,7 +50,7 @@ import org.slf4j.LoggerFactory;
  *
  * @lucene.experimental
  */
-public class ClusterState implements JSONWriter.Writable {
+public class ClusterState implements MapWriter {
 
   /** Cluster Prop that is http or https. */
   public static final String URL_SCHEME = "urlScheme";
@@ -306,15 +308,13 @@ public class ClusterState implements JSONWriter.Writable {
   }
 
   @Override
-  public void write(JSONWriter jsonWriter) {
-    LinkedHashMap<String, DocCollection> map = new LinkedHashMap<>();
+  public void writeMap(EntryWriter ew) throws IOException {
     for (Entry<String, CollectionRef> e : collectionStates.entrySet()) {
       if (e.getValue().getClass() == CollectionRef.class) {
         DocCollection coll = e.getValue().get();
-        map.put(coll.getName(), coll);
+        ew.put(coll.getName(), coll);
       }
     }
-    jsonWriter.write(map);
   }
 
   @Override
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java b/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java
index 40b41ba2171..f70a5dc3e46 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java
@@ -18,13 +18,13 @@ package org.apache.solr.common.cloud;
 
 import static org.apache.solr.common.util.Utils.toJSONString;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -34,8 +34,6 @@ import java.util.function.BiConsumer;
 import java.util.function.BiPredicate;
 import java.util.function.Supplier;
 import org.apache.solr.common.cloud.Replica.ReplicaStateProps;
-import org.apache.solr.common.util.CollectionUtil;
-import org.noggit.JSONWriter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -125,7 +123,7 @@ public class DocCollection extends ZkNodeProps implements Iterable<Slice> {
       if (perReplicaStatesRef == null || perReplicaStatesRef.get() == null) {
         throw new RuntimeException(
             CollectionStateProps.PER_REPLICA_STATE
-                + " = true , but perReplicatStates param is not provided");
+                + " = true , but perReplicaStates param is not provided");
       }
       this.perReplicaStatesRef = perReplicaStatesRef;
       for (Slice s : this.slices.values()) { // set the same reference to all slices too
@@ -398,11 +396,9 @@ public class DocCollection extends ZkNodeProps implements Iterable<Slice> {
   }
 
   @Override
-  public void write(JSONWriter jsonWriter) {
-    LinkedHashMap<String, Object> all = CollectionUtil.newLinkedHashMap(slices.size() + 1);
-    all.putAll(propMap);
-    all.put(CollectionStateProps.SHARDS, slices);
-    jsonWriter.write(all);
+  public void writeMap(EntryWriter ew) throws IOException {
+    propMap.forEach(ew.getBiConsumer());
+    ew.put(CollectionStateProps.SHARDS, slices);
   }
 
   public Replica getReplica(String coreNodeName) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
index aae2d250c1f..72bb2879662 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
@@ -27,7 +27,6 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.util.Utils;
-import org.noggit.JSONWriter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -372,11 +371,6 @@ public class Replica extends ZkNodeProps implements MapWriter {
     return new Replica(name, node, collection, shard, core, getState(), type, propMap);
   }
 
-  @Override
-  public void writeMap(MapWriter.EntryWriter ew) throws IOException {
-    _allPropsWriter().writeMap(ew);
-  }
-
   private static final Map<String, State> STATES = new HashMap<>();
 
   static {
@@ -390,28 +384,21 @@ public class Replica extends ZkNodeProps implements MapWriter {
     return STATES.get(shortName);
   }
 
-  private MapWriter _allPropsWriter() {
-    return w -> {
-      w.putIfNotNull(ReplicaStateProps.CORE_NAME, core)
-          .putIfNotNull(ReplicaStateProps.NODE_NAME, node)
-          .putIfNotNull(ReplicaStateProps.TYPE, type.toString())
-          .putIfNotNull(ReplicaStateProps.STATE, getState().toString())
-          .putIfNotNull(ReplicaStateProps.LEADER, () -> isLeader() ? "true" : null)
-          .putIfNotNull(
-              ReplicaStateProps.FORCE_SET_STATE, propMap.get(ReplicaStateProps.FORCE_SET_STATE))
-          .putIfNotNull(ReplicaStateProps.BASE_URL, propMap.get(ReplicaStateProps.BASE_URL));
-      for (Map.Entry<String, Object> e : propMap.entrySet()) {
-        if (!ReplicaStateProps.WELL_KNOWN_PROPS.contains(e.getKey())) {
-          w.putIfNotNull(e.getKey(), e.getValue());
-        }
-      }
-    };
-  }
-
   @Override
-  public void write(JSONWriter jsonWriter) {
-    // this serializes also our declared properties
-    jsonWriter.write(_allPropsWriter());
+  public void writeMap(MapWriter.EntryWriter ew) throws IOException {
+    ew.putIfNotNull(ReplicaStateProps.CORE_NAME, core)
+        .putIfNotNull(ReplicaStateProps.NODE_NAME, node)
+        .putIfNotNull(ReplicaStateProps.TYPE, type.toString())
+        .putIfNotNull(ReplicaStateProps.STATE, getState().toString())
+        .putIfNotNull(ReplicaStateProps.LEADER, () -> isLeader() ? "true" : null)
+        .putIfNotNull(
+            ReplicaStateProps.FORCE_SET_STATE, propMap.get(ReplicaStateProps.FORCE_SET_STATE))
+        .putIfNotNull(ReplicaStateProps.BASE_URL, propMap.get(ReplicaStateProps.BASE_URL));
+    for (Map.Entry<String, Object> e : propMap.entrySet()) {
+      if (!ReplicaStateProps.WELL_KNOWN_PROPS.contains(e.getKey())) {
+        ew.putIfNotNull(e.getKey(), e.getValue());
+      }
+    }
   }
 
   @Override
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Slice.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Slice.java
index 0007c679690..7ca05b279e3 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Slice.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Slice.java
@@ -34,7 +34,6 @@ import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import org.apache.solr.common.cloud.Replica.Type;
 import org.apache.solr.common.util.CollectionUtil;
-import org.noggit.JSONWriter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -327,11 +326,6 @@ public class Slice extends ZkNodeProps implements Iterable<Replica> {
     return name + ':' + toJSONString(propMap);
   }
 
-  @Override
-  public void write(JSONWriter jsonWriter) {
-    jsonWriter.write(propMap);
-  }
-
   /** JSON properties related to a slice's state. */
   public interface SliceStateProps {
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
index 0e6dcef7108..b187e475fe8 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
@@ -28,10 +28,9 @@ import java.util.Set;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.util.JavaBinCodec;
 import org.apache.solr.common.util.Utils;
-import org.noggit.JSONWriter;
 
 /** ZkNodeProps contains generic immutable properties. */
-public class ZkNodeProps implements JSONWriter.Writable, MapWriter {
+public class ZkNodeProps implements MapWriter {
 
   protected final Map<String, Object> propMap;
 
@@ -105,11 +104,6 @@ public class ZkNodeProps implements JSONWriter.Writable, MapWriter {
     return new ZkNodeProps(props);
   }
 
-  @Override
-  public void write(JSONWriter jsonWriter) {
-    jsonWriter.write(propMap);
-  }
-
   /** Get a string property value. */
   public String getStr(String key) {
     return getStr(key, null);
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/MapWriterJSONWriter.java b/solr/solrj/src/java/org/apache/solr/common/util/MapWriterJSONWriter.java
deleted file mode 100644
index 7e9ab8ef842..00000000000
--- a/solr/solrj/src/java/org/apache/solr/common/util/MapWriterJSONWriter.java
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.solr.common.util;
-
-import java.io.IOException;
-import org.apache.solr.common.IteratorWriter;
-import org.apache.solr.common.MapWriter;
-import org.noggit.CharArr;
-import org.noggit.JSONWriter;
-
-class MapWriterJSONWriter extends JSONWriter {
-
-  public MapWriterJSONWriter(CharArr out, int indentSize) {
-    super(out, indentSize);
-  }
-
-  @Override
-  public void handleUnknownClass(Object o) {
-    // avoid materializing MapWriter / IteratorWriter to Map / List
-    // instead serialize them directly
-    if (o instanceof MapWriter) {
-      writeMapWriter((MapWriter) o);
-    } else if (o instanceof IteratorWriter) {
-      IteratorWriter iteratorWriter = (IteratorWriter) o;
-      writeIter(iteratorWriter);
-    } else {
-      super.handleUnknownClass(o);
-    }
-  }
-
-  private void writeIter(IteratorWriter iteratorWriter) {
-    startArray();
-    try {
-      iteratorWriter.writeIter(
-          new IteratorWriter.ItemWriter() {
-            boolean first = true;
-
-            @Override
-            public IteratorWriter.ItemWriter add(Object o) {
-              if (first) {
-                first = false;
-              } else {
-                writeValueSeparator();
-              }
-              indent();
-              write(o);
-              return this;
-            }
-          });
-    } catch (IOException e) {
-      throw new RuntimeException("this should never happen", e);
-    }
-    endArray();
-  }
-
-  private void writeMapWriter(MapWriter mapWriter) {
-    startObject();
-    try {
-      mapWriter.writeMap(
-          new MapWriter.EntryWriter() {
-            boolean first = true;
-
-            @Override
-            public MapWriter.EntryWriter put(CharSequence k, Object v) {
-              if (first) {
-                first = false;
-              } else {
-                writeValueSeparator();
-              }
-              indent();
-              writeString(k.toString());
-              writeNameSeparator();
-              write(v);
-              return this;
-            }
-          });
-    } catch (IOException e) {
-      throw new RuntimeException(e);
-    }
-    endObject();
-  }
-}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 6cd2dae47e9..1158eaae608 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -81,6 +81,7 @@ import org.apache.solr.common.annotation.JsonProperty;
 import org.apache.solr.common.params.CommonParams;
 import org.noggit.CharArr;
 import org.noggit.JSONParser;
+import org.noggit.JSONWriter;
 import org.noggit.ObjectBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -216,7 +217,7 @@ public class Utils {
   public static byte[] toJSON(Object o) {
     if (o == null) return new byte[0];
     CharArr out = new CharArr();
-    new MapWriterJSONWriter(out, 2).write(o); // indentation by default
+    new JSONWriter(out, 2).write(o); // indentation by default
     return toUTF8(out);
   }