From 2544df8f6d9a23e285d899c6a754611dd830f68f Mon Sep 17 00:00:00 2001 From: Andy Vuong Date: Thu, 3 Sep 2020 11:25:36 -0700 Subject: [PATCH] SOLR-14658: SolrJ collectionStatus(col) should only fetch one status (#1687) An optimization or a perf-bug depending on point of view --- solr/CHANGES.txt | 3 ++ .../handler/admin/CollectionsHandler.java | 5 +-- .../solr/cloud/CollectionsAPISolrJTest.java | 37 +++++++++++++++++++ .../solrj/request/CollectionAdminRequest.java | 22 +++++++++-- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 69a398d64a4..31c0616381a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -183,6 +183,9 @@ Optimizations * SOLR-14819: Fix inefficient iterator pattern in JsonSchemaValidator. (Thomas DuBuisson via Bruno Roustant) +* SOLR-14658: SolrJ's CollectionAdminRequest.collectionStatus(collection) would internally get + all collection statuses instead of just the specified collection. (Andy Vuong) + Bug Fixes --------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index 06226410df1..71380024839 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -517,10 +517,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission ColStatus.RAW_SIZE_DETAILS_PROP, ColStatus.RAW_SIZE_SAMPLING_PERCENT_PROP, ColStatus.SIZE_INFO_PROP); - // make sure we can get the name if there's "name" but not "collection" - if (props.containsKey(CoreAdminParams.NAME) && !props.containsKey(COLLECTION_PROP)) { - props.put(COLLECTION_PROP, props.get(CoreAdminParams.NAME)); - } + new ColStatus(h.coreContainer.getSolrClientCache(), h.coreContainer.getZkController().getZkStateReader().getClusterState(), new ZkNodeProps(props)) .getColStatus(rsp.getValues()); diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java index a6d3d811f25..ba5a9b64079 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java @@ -662,6 +662,43 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase { Number down = (Number) rsp.getResponse().findRecursive(collectionName, "shards", "shard1", "replicas", "down"); assertTrue("should be some down replicas, but there were none in shard1:" + rsp, down.intValue() > 0); } + + @Test + public void testColStatusCollectionName() throws Exception { + final String[] collectionNames = {"collectionStatusTest_1", "collectionStatusTest_2"}; + for (String collectionName : collectionNames) { + CollectionAdminRequest.createCollection(collectionName, "conf2", 1, 1) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(collectionName, 1, 1); + } + // assert only one collection is returned using the solrj colstatus interface + CollectionAdminRequest.ColStatus req = CollectionAdminRequest.collectionStatus(collectionNames[0]); + CollectionAdminResponse rsp = req.process(cluster.getSolrClient()); + assertNotNull(rsp.getResponse().get(collectionNames[0])); + assertNull(rsp.getResponse().get(collectionNames[1])); + + req = CollectionAdminRequest.collectionStatus(collectionNames[1]); + rsp = req.process(cluster.getSolrClient()); + assertNotNull(rsp.getResponse().get(collectionNames[1])); + assertNull(rsp.getResponse().get(collectionNames[0])); + + // assert passing null collection fails + expectThrows(NullPointerException.class, + "Passing null to collectionStatus should result in an NPE", + () -> CollectionAdminRequest.collectionStatus(null)); + + // assert passing non-existent collection returns no collections + req = CollectionAdminRequest.collectionStatus("doesNotExist"); + rsp = req.process(cluster.getSolrClient()); + assertNull(rsp.getResponse().get(collectionNames[0])); + assertNull(rsp.getResponse().get(collectionNames[1])); + + // assert collectionStatuses returns all collections + req = CollectionAdminRequest.collectionStatuses(); + rsp = req.process(cluster.getSolrClient()); + assertNotNull(rsp.getResponse().get(collectionNames[1])); + assertNotNull(rsp.getResponse().get(collectionNames[0])); + } private static final int NUM_DOCS = 10; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java index bffdc4e13f3..531f8f132e4 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java @@ -868,13 +868,23 @@ public abstract class CollectionAdminRequest } /** - * Return a SolrRequest for low-level detailed status of the collection. + * Return a SolrRequest for low-level detailed status of the specified collection. + * @param collection the collection to get the status of. */ public static ColStatus collectionStatus(String collection) { + checkNotNull(CoreAdminParams.COLLECTION, collection); return new ColStatus(collection); } + + /** + * Return a SolrRequest for low-level detailed status of all collections on the cluster. + */ + public static ColStatus collectionStatuses() { + return new ColStatus(); + } - public static class ColStatus extends AsyncCollectionSpecificAdminRequest { + public static class ColStatus extends AsyncCollectionAdminRequest { + protected String collection = null; protected Boolean withSegments = null; protected Boolean withFieldInfo = null; protected Boolean withCoreInfo = null; @@ -885,7 +895,12 @@ public abstract class CollectionAdminRequest protected Float rawSizeSamplingPercent = null; private ColStatus(String collection) { - super(CollectionAction.COLSTATUS, collection); + super(CollectionAction.COLSTATUS); + this.collection = collection; + } + + private ColStatus() { + super(CollectionAction.COLSTATUS); } public ColStatus setWithSegments(boolean withSegments) { @@ -931,6 +946,7 @@ public abstract class CollectionAdminRequest @Override public SolrParams getParams() { ModifiableSolrParams params = (ModifiableSolrParams)super.getParams(); + params.setNonNull(CoreAdminParams.COLLECTION, collection); params.setNonNull("segments", withSegments); params.setNonNull("fieldInfo", withFieldInfo); params.setNonNull("coreInfo", withCoreInfo);