dsmiley commented on code in PR #2853:
URL: https://github.com/apache/solr/pull/2853#discussion_r1874572327
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -128,58 +130,57 @@ public ClusterState.CollectionRef getState(String
collection) {
}
@SuppressWarnings("unchecked")
- private ClusterState fetchClusterState(
- SolrClient client, String collection, Map<String, Object>
clusterProperties)
+ private ClusterState fetchClusterState(SolrClient client)
throws SolrServerException, IOException, NotACollectionException {
SimpleOrderedMap<?> cluster =
- submitClusterStateRequest(client, collection,
ClusterStateRequestType.FETCH_COLLECTION);
+ submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_STATE);
- Map<String, Object> collectionsMap;
- if (collection != null) {
- collectionsMap =
- Collections.singletonMap(
- collection, ((NamedList<?>)
cluster.get("collections")).get(collection));
- } else {
- collectionsMap = ((NamedList<?>) cluster.get("collections")).asMap(10);
- }
- int znodeVersion;
- Map<String, Object> collFromStatus = (Map<String, Object>)
(collectionsMap).get(collection);
- if (collection != null && collFromStatus == null) {
- throw new NotACollectionException(); // probably an alias
- }
- if (collection != null) { // can be null if alias
- znodeVersion = (int) collFromStatus.get("znodeVersion");
- } else {
- znodeVersion = -1;
- }
-
- ClusterState cs = new ClusterState(this.liveNodes, new HashMap<>());
List<String> liveNodesList = (List<String>) cluster.get("live_nodes");
if (liveNodesList != null) {
- Set<String> liveNodes = new HashSet<>(liveNodesList);
- this.liveNodes = liveNodes;
+ this.liveNodes = Set.copyOf(liveNodesList);
liveNodesTimestamp = System.nanoTime();
- cs = new ClusterState(liveNodes, new HashMap<>());
}
- for (Map.Entry<String, Object> e : collectionsMap.entrySet()) {
- @SuppressWarnings("rawtypes")
- Map m = (Map) e.getValue();
- Long creationTimeMillisFromClusterStatus = (Long)
m.get("creationTimeMillis");
- Instant creationTime =
- creationTimeMillisFromClusterStatus == null
- ? Instant.EPOCH
- : Instant.ofEpochMilli(creationTimeMillisFromClusterStatus);
- cs = cs.copyWith(e.getKey(), fillPrs(znodeVersion, e, creationTime, m));
+ var collectionsNl = (NamedList<NamedList<?>>) cluster.get("collections");
+
+ Map<String, DocCollection> collStateByName = new
LinkedHashMap<>(collectionsNl.size());
+ for (Entry<String, NamedList<?>> entry : collectionsNl) {
+ final var collStateMap = entry.getValue().asMap(10);
+ final int zNodeVersion = (int) collStateMap.get("znodeVersion");
Review Comment:
fixed a critical bug in your last version. We must use the ZK version for
the DocCollection, not a bogus -1!
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -128,58 +130,57 @@ public ClusterState.CollectionRef getState(String
collection) {
}
@SuppressWarnings("unchecked")
- private ClusterState fetchClusterState(
- SolrClient client, String collection, Map<String, Object>
clusterProperties)
+ private ClusterState fetchClusterState(SolrClient client)
throws SolrServerException, IOException, NotACollectionException {
SimpleOrderedMap<?> cluster =
- submitClusterStateRequest(client, collection,
ClusterStateRequestType.FETCH_COLLECTION);
+ submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_STATE);
- Map<String, Object> collectionsMap;
- if (collection != null) {
- collectionsMap =
- Collections.singletonMap(
- collection, ((NamedList<?>)
cluster.get("collections")).get(collection));
- } else {
- collectionsMap = ((NamedList<?>) cluster.get("collections")).asMap(10);
- }
- int znodeVersion;
- Map<String, Object> collFromStatus = (Map<String, Object>)
(collectionsMap).get(collection);
- if (collection != null && collFromStatus == null) {
- throw new NotACollectionException(); // probably an alias
- }
- if (collection != null) { // can be null if alias
- znodeVersion = (int) collFromStatus.get("znodeVersion");
- } else {
- znodeVersion = -1;
- }
-
- ClusterState cs = new ClusterState(this.liveNodes, new HashMap<>());
List<String> liveNodesList = (List<String>) cluster.get("live_nodes");
if (liveNodesList != null) {
- Set<String> liveNodes = new HashSet<>(liveNodesList);
- this.liveNodes = liveNodes;
+ this.liveNodes = Set.copyOf(liveNodesList);
liveNodesTimestamp = System.nanoTime();
- cs = new ClusterState(liveNodes, new HashMap<>());
}
- for (Map.Entry<String, Object> e : collectionsMap.entrySet()) {
- @SuppressWarnings("rawtypes")
- Map m = (Map) e.getValue();
- Long creationTimeMillisFromClusterStatus = (Long)
m.get("creationTimeMillis");
- Instant creationTime =
- creationTimeMillisFromClusterStatus == null
- ? Instant.EPOCH
- : Instant.ofEpochMilli(creationTimeMillisFromClusterStatus);
- cs = cs.copyWith(e.getKey(), fillPrs(znodeVersion, e, creationTime, m));
+ var collectionsNl = (NamedList<NamedList<?>>) cluster.get("collections");
+
+ Map<String, DocCollection> collStateByName = new
LinkedHashMap<>(collectionsNl.size());
+ for (Entry<String, NamedList<?>> entry : collectionsNl) {
+ final var collStateMap = entry.getValue().asMap(10);
+ final int zNodeVersion = (int) collStateMap.get("znodeVersion");
+ collStateByName.put(
+ entry.getKey(), getDocCollectionFromObjects(entry.getKey(),
collStateMap, zNodeVersion));
}
- if (clusterProperties != null) {
- Map<String, Object> properties = (Map<String, Object>)
cluster.get("properties");
- if (properties != null) {
- clusterProperties.putAll(properties);
- }
Review Comment:
this was dead code; nobody calls this method with clusterProperties anymore
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -128,58 +130,57 @@ public ClusterState.CollectionRef getState(String
collection) {
}
@SuppressWarnings("unchecked")
- private ClusterState fetchClusterState(
- SolrClient client, String collection, Map<String, Object>
clusterProperties)
+ private ClusterState fetchClusterState(SolrClient client)
throws SolrServerException, IOException, NotACollectionException {
SimpleOrderedMap<?> cluster =
- submitClusterStateRequest(client, collection,
ClusterStateRequestType.FETCH_COLLECTION);
+ submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_STATE);
- Map<String, Object> collectionsMap;
- if (collection != null) {
- collectionsMap =
- Collections.singletonMap(
- collection, ((NamedList<?>)
cluster.get("collections")).get(collection));
- } else {
- collectionsMap = ((NamedList<?>) cluster.get("collections")).asMap(10);
- }
- int znodeVersion;
- Map<String, Object> collFromStatus = (Map<String, Object>)
(collectionsMap).get(collection);
- if (collection != null && collFromStatus == null) {
- throw new NotACollectionException(); // probably an alias
- }
- if (collection != null) { // can be null if alias
- znodeVersion = (int) collFromStatus.get("znodeVersion");
- } else {
- znodeVersion = -1;
- }
-
- ClusterState cs = new ClusterState(this.liveNodes, new HashMap<>());
List<String> liveNodesList = (List<String>) cluster.get("live_nodes");
if (liveNodesList != null) {
- Set<String> liveNodes = new HashSet<>(liveNodesList);
- this.liveNodes = liveNodes;
+ this.liveNodes = Set.copyOf(liveNodesList);
Review Comment:
small optimization
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -128,58 +130,57 @@ public ClusterState.CollectionRef getState(String
collection) {
}
@SuppressWarnings("unchecked")
- private ClusterState fetchClusterState(
- SolrClient client, String collection, Map<String, Object>
clusterProperties)
+ private ClusterState fetchClusterState(SolrClient client)
throws SolrServerException, IOException, NotACollectionException {
SimpleOrderedMap<?> cluster =
- submitClusterStateRequest(client, collection,
ClusterStateRequestType.FETCH_COLLECTION);
+ submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_STATE);
- Map<String, Object> collectionsMap;
- if (collection != null) {
- collectionsMap =
- Collections.singletonMap(
- collection, ((NamedList<?>)
cluster.get("collections")).get(collection));
- } else {
- collectionsMap = ((NamedList<?>) cluster.get("collections")).asMap(10);
- }
- int znodeVersion;
- Map<String, Object> collFromStatus = (Map<String, Object>)
(collectionsMap).get(collection);
- if (collection != null && collFromStatus == null) {
- throw new NotACollectionException(); // probably an alias
- }
- if (collection != null) { // can be null if alias
- znodeVersion = (int) collFromStatus.get("znodeVersion");
- } else {
- znodeVersion = -1;
- }
-
- ClusterState cs = new ClusterState(this.liveNodes, new HashMap<>());
List<String> liveNodesList = (List<String>) cluster.get("live_nodes");
if (liveNodesList != null) {
- Set<String> liveNodes = new HashSet<>(liveNodesList);
- this.liveNodes = liveNodes;
+ this.liveNodes = Set.copyOf(liveNodesList);
liveNodesTimestamp = System.nanoTime();
- cs = new ClusterState(liveNodes, new HashMap<>());
}
- for (Map.Entry<String, Object> e : collectionsMap.entrySet()) {
- @SuppressWarnings("rawtypes")
- Map m = (Map) e.getValue();
- Long creationTimeMillisFromClusterStatus = (Long)
m.get("creationTimeMillis");
- Instant creationTime =
- creationTimeMillisFromClusterStatus == null
- ? Instant.EPOCH
- : Instant.ofEpochMilli(creationTimeMillisFromClusterStatus);
- cs = cs.copyWith(e.getKey(), fillPrs(znodeVersion, e, creationTime, m));
+ var collectionsNl = (NamedList<NamedList<?>>) cluster.get("collections");
+
+ Map<String, DocCollection> collStateByName = new
LinkedHashMap<>(collectionsNl.size());
Review Comment:
No longer using ClusterState.copyWith, solving a perf problem
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
