SOLR-16699: Expose creationTimeMillis in COLSTATUS API #2226
Conversation
github-actions
bot
added
documentation
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java
Show resolved
Hide resolved
solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java
Outdated
Show resolved
Hide resolved
solr/core/src/java/org/apache/solr/cloud/overseer/ClusterStateMutator.java
Outdated
Show resolved
Hide resolved
| // set a default created date, we don't aim at reading actual zookeeper state. The restored | ||
| // collection | ||
| // will have a new creation date when persisted in zookeeper. |
| @@ -345,6 +346,16 @@ private void clusterStatusWithCollectionAndMultipleShards() | |||
| } | |||
| } | |||
|
|
|||
| private Instant getCreationTimeFromClusterStateOrFail( | |||
| CloudSolrClient client, String collectionName) throws IOException { | |||
| DocCollection docCollection = client.getClusterState().getCollectionOrNull(collectionName); | |||
There was a problem hiding this comment.
Why call the OrNull version; why not let the default impl throw its own exception? That's what its for.
| @@ -549,4 +523,9 @@ public void testSingleExternalCollectionCompressedState() throws Exception { | |||
| server.shutdown(); | |||
| } | |||
| } | |||
|
|
|||
| private DocCollection createDocCollection(String name, Map<String, Object> props) { | |||
There was a problem hiding this comment.
glad to see this; simplified the test
| * When this DocCollection is read from Zookeeper, it captures a collection persisted in | ||
| * Zookeeper. The date thus captures the date when the collection node was created, and thus the | ||
| * collection made usable by APIs as specified by {@code ZkStateReader#getCollectionPath(String)} |
There was a problem hiding this comment.
Should start with a simple sentence like "The creation time of this collection." Perhaps say it's never null. Then the proposed text.
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| public class BaseHttpClusterStateProviderTest extends SolrCloudTestCase { |
There was a problem hiding this comment.
This test does not need to exist or could be converted to something generic. A ClusterStateProvider is an abstraction with equivalent implementations -- and thus could be chosen arbitrarily in some other test that checks the time that otherwise doesn't really care about ZK or Http for that matter. Many things in Lucene and Solr are tested embracing this philosophy and I count myself a convert. "randomized testing". Maybe an existing test that checks creation time could be retrofitted to use the cluster state provider impl randomly? Or alternatively since there are 2, make a generic test that is parameterized for the two (example of the approach -- TestConfigSetService)
There was a problem hiding this comment.
Thanks for this comment, it was useful. I have recreated this test in ClusterStateProviderTest to implement the suggestion.
…Mutator.java Co-authored-by: David Smiley <dsmiley@apache.org>
|
ReindexCollection.testAbort fails sometimes; isn't caused by this change. -- http://fucit.org/solr-jenkins-reports/failure-report.html |
…US API responses (#2226) Using ZooKeeper "ctime" of the node. Co-authored-by: Julien Pilourdault Co-authored-by: Paul McArthur <pmcarthur-apache@proton.me> Co-authored-by: David Smiley <dsmiley@salesforce.com>
https://issues.apache.org/jira/browse/SOLR-16699
Description
The collection creation time is now exposed in the solrj DocCollection and the COLSTATUS API.
Solution
The creation time comes from the ctime value of the ZooKeeper node for the collection.
Tests
Added a test to verify the presence and value of the
creationTimeMillisattribute in theDocCollectionand COLSTATUS API response.Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.