You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2020/09/03 18:29:45 UTC
[lucene-solr] branch branch_8x updated: SOLR-14658: SolrJ
collectionStatus(col) should only fetch one status (#1687)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 61ea26a SOLR-14658: SolrJ collectionStatus(col) should only fetch one status (#1687)
61ea26a is described below
commit 61ea26ac758ea8048177b197d2fdeddbe7cc5df0
Author: Andy Vuong <an...@users.noreply.github.com>
AuthorDate: Thu Sep 3 11:25:36 2020 -0700
SOLR-14658: SolrJ collectionStatus(col) should only fetch one status (#1687)
An optimization or a perf-bug depending on point of view
(cherry picked from commit 2544df8f6d9a23e285d899c6a754611dd830f68f)
---
solr/CHANGES.txt | 3 ++
.../solr/handler/admin/CollectionsHandler.java | 5 +--
.../apache/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 11e025d..610195b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -64,6 +64,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 3e55fc0..42df2e5 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
@@ -543,10 +543,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 162f250..f6ade9e 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
@@ -677,6 +677,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 6f6ac83..432c901 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
@@ -912,13 +912,23 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
}
/**
- * 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;
@@ -929,7 +939,12 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
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) {
@@ -975,6 +990,7 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
@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);