Re: [PR] SOLR-17535 ClusterState.collectionStream() in lieu of getCollectionStates() [solr]
murblanc commented on code in PR #2834:
URL: https://github.com/apache/solr/pull/2834#discussion_r1827306479
##
solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java:
##
@@ -382,6 +383,7 @@ void setLiveNodes(Set liveNodes) {
* Be aware that this may return collections which may not exist now. You
can confirm that this
* collection exists after verifying CollectionRef.get() != null
*/
+ @Deprecated // see collectionStream()
Review Comment:
Use `@deprecated` tag in Javadoc instead of dangling comment
--
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
Re: [PR] SOLR-17535 ClusterState.collectionStream() in lieu of getCollectionStates() [solr]
murblanc commented on code in PR #2834:
URL: https://github.com/apache/solr/pull/2834#discussion_r1827307530
##
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java:
##
@@ -168,24 +167,12 @@ Map analyzeField(String configSet, String
fieldName, String fiel
}
List listCollectionsForConfig(String configSet) {
-final List collections = new ArrayList<>();
-Map states =
-zkStateReader().getClusterState().getCollectionStates();
-for (Map.Entry e : states.entrySet()) {
- final String coll = e.getKey();
- if (coll.startsWith(DESIGNER_PREFIX)) {
-continue; // ignore temp
- }
-
- try {
-if (configSet.equals(e.getValue().get().getConfigName()) &&
e.getValue().get() != null) {
- collections.add(coll);
-}
- } catch (Exception exc) {
-log.warn("Failed to get config name for {}", coll, exc);
- }
-}
-return collections;
+return zkStateReader()
+.getClusterState()
+.collectionStream()
Review Comment:
Performance regression: this forces reading all `state.json` of all
collections.
I think `getCollectionNames()` should be part of this PR and used here
instead.
--
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
Re: [PR] SOLR-17535 ClusterState.collectionStream() in lieu of getCollectionStates() [solr]
murblanc commented on code in PR #2834:
URL: https://github.com/apache/solr/pull/2834#discussion_r1827308183
##
solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java:
##
@@ -382,6 +383,7 @@ void setLiveNodes(Set liveNodes) {
* Be aware that this may return collections which may not exist now. You
can confirm that this
* collection exists after verifying CollectionRef.get() != null
*/
+ @Deprecated // see collectionStream()
Review Comment:
And IMO this can't be deprecated for performance reasons until
`getCollectionNames()` is added to replace it.
--
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
