mlbiscoc commented on code in PR #2916:
URL: https://github.com/apache/solr/pull/2916#discussion_r1892947219
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -368,4 +341,77 @@ public static Map<String, Object>
postProcessCollectionJSON(Map<String, Object>
collection.put("health", Health.combine(healthStates).toString());
return collection;
}
+
+ private class SolrCollectionProperiesIterator implements
Iterator<NamedList<Object>> {
+
+ final Iterator<DocCollection> it;
+ Map<String, List<String>> collectionVsAliases;
+ String routeKey;
+ List<String> liveNodes;
+ String shard;
+
+ public SolrCollectionProperiesIterator(
+ Iterator<DocCollection> it,
+ Map<String, List<String>> collectionVsAliases,
+ String routeKey,
+ List<String> liveNodes,
+ String shard) {
+ this.it = it;
+ this.collectionVsAliases = collectionVsAliases;
+ this.routeKey = routeKey;
+ this.liveNodes = liveNodes;
+ this.shard = shard;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return it.hasNext();
+ }
+
+ @Override
+ public NamedList<Object> next() {
+ NamedList<Object> collectionProps = new SimpleOrderedMap<>();
+ DocCollection clusterStateCollection = it.next();
+ Map<String, Object> collectionStatus;
+ String name = clusterStateCollection.getName();
+
+ Set<String> requestedShards = new HashSet<>();
+ if (routeKey != null) {
+ DocRouter router = clusterStateCollection.getRouter();
+ Collection<Slice> slices = router.getSearchSlices(routeKey, null,
clusterStateCollection);
+ for (Slice slice : slices) {
+ requestedShards.add(slice.getName());
+ }
+ }
+ if (shard != null) {
+ String[] paramShards = shard.split(",");
+ requestedShards.addAll(Arrays.asList(paramShards));
+ }
Review Comment:
Moved it out so its not per iteration.
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -368,4 +341,77 @@ public static Map<String, Object>
postProcessCollectionJSON(Map<String, Object>
collection.put("health", Health.combine(healthStates).toString());
return collection;
}
+
+ private class SolrCollectionProperiesIterator implements
Iterator<NamedList<Object>> {
+
+ final Iterator<DocCollection> it;
+ Map<String, List<String>> collectionVsAliases;
+ String routeKey;
+ List<String> liveNodes;
+ String shard;
+
+ public SolrCollectionProperiesIterator(
+ Iterator<DocCollection> it,
+ Map<String, List<String>> collectionVsAliases,
+ String routeKey,
+ List<String> liveNodes,
+ String shard) {
+ this.it = it;
+ this.collectionVsAliases = collectionVsAliases;
+ this.routeKey = routeKey;
+ this.liveNodes = liveNodes;
+ this.shard = shard;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return it.hasNext();
+ }
+
+ @Override
+ public NamedList<Object> next() {
+ NamedList<Object> collectionProps = new SimpleOrderedMap<>();
+ DocCollection clusterStateCollection = it.next();
+ Map<String, Object> collectionStatus;
+ String name = clusterStateCollection.getName();
+
+ Set<String> requestedShards = new HashSet<>();
+ if (routeKey != null) {
+ DocRouter router = clusterStateCollection.getRouter();
+ Collection<Slice> slices = router.getSearchSlices(routeKey, null,
clusterStateCollection);
+ for (Slice slice : slices) {
+ requestedShards.add(slice.getName());
+ }
+ }
+ if (shard != null) {
+ String[] paramShards = shard.split(",");
+ requestedShards.addAll(Arrays.asList(paramShards));
+ }
+
+ byte[] bytes = Utils.toJSON(clusterStateCollection);
+ @SuppressWarnings("unchecked")
+ Map<String, Object> docCollection = (Map<String, Object>)
Utils.fromJSON(bytes);
Review Comment:
So looked at this and I understand now why it was done this way. It wants to
just write this out to the `TextWriter` as a normal POJO but that doesn't seem
to be what the `DocCollection` class is. So instead what was done was to just
write it to JSON `byte[]` then back to a `Map` which was the "easiest" way.
Was looking to avoid this back and forth, but there were a few options. I
tried using an `ObjectMapper` to `Map.class` but there is an error for unable
to cast Instant due to a missing dependency for Jackson we need to introduce.
`Java 8 date/time type `java.time.Instant` not supported by default Issue`
Other way is to introduce a some kind of `toMap` method so that the
TextWriter can write this as a generic `Map`.
Another option which actually looks like the way we should go is I found
that the `DocCollection` class extends ZkNodeProps which implements
`MapWriter`. `DocCollection` already overrides
`[writeMap](https://github.com/apache/solr/blob/9cef6e390719cbd7b55085cfef98fcb053785f77/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java#L427)`
so we could just return this to the TextWriter! Unfortunately the
ClusterStatus class does a bunch of JSON post processing such as `Health` added
to the `Map` that the output is missing some things because of this
`[postProcessCollectionJSON()](https://github.com/apache/solr/blob/9cef6e390719cbd7b55085cfef98fcb053785f77/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java#L334)`
method.
I am thinking we should refactor DocCollection to so we can just return this
but the changes were much more drastic but may be worth it. Maybe in a
different JIRA? This scope continues to creep with me adding improvement to
`NamedList`. Would be happy to refactor this and pick it up if you agree. Would
clean up much more code and avoid this JSON processing for every collection
iteration.
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -368,4 +341,77 @@ public static Map<String, Object>
postProcessCollectionJSON(Map<String, Object>
collection.put("health", Health.combine(healthStates).toString());
return collection;
}
+
+ private class SolrCollectionProperiesIterator implements
Iterator<NamedList<Object>> {
Review Comment:
Ah yeah don't know why I didn't do this first. Changed it appropriately.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
