dsmiley commented on code in PR #3742:
URL: https://github.com/apache/solr/pull/3742#discussion_r2414792451
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java:
##########
@@ -256,47 +257,58 @@ public void testAliasHandling() throws Exception {
@Test
@LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
public void testHttpCspPerf() throws Exception {
+ // This ensures CH2SC is caching cluster status by counting the number of
logged calls to the
+ // admin endpoint. Too many calls to CLUSTERSTATUS might mean insufficient
caching and
+ // performance regressions!
+ try {
+ // BaseHttpClusterStateProvider has a background job that pre-fetches
data from CLUSTERSTATUS
+ // on timed intervals. This can pollute this test, so we set the
interval very high to
+ // prevent it from running.
+ System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, "" +
Integer.MAX_VALUE);
- String collectionName = "HTTPCSPTEST";
- CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1)
- .process(cluster.getSolrClient());
- cluster.waitForActiveCollection(collectionName, 2, 2);
+ String collectionName = "HTTPCSPTEST";
+ CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1)
+ .process(cluster.getSolrClient());
+ cluster.waitForActiveCollection(collectionName, 2, 2);
- try (LogListener adminLogs =
LogListener.info(HttpSolrCall.class).substring("[admin]");
- CloudSolrClient solrClient = createHttpCSPBasedCloudSolrClient(); ) {
- solrClient.getClusterStateProvider().getLiveNodes(); // talks to Solr
+ try (LogListener adminLogs =
LogListener.info(HttpSolrCall.class).substring("[admin]");
+ CloudSolrClient solrClient = createHttpCSPBasedCloudSolrClient(); ) {
+ solrClient.getClusterStateProvider().getLiveNodes(); // talks to Solr
- assertEquals(1, adminLogs.getCount());
- assertTrue(
- adminLogs
- .pollMessage()
- .contains(
- "path=/admin/collections
params={prs=true&liveNodes=true&action"
- + "=CLUSTERSTATUS&includeAll=false"));
-
- SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s", "my
doc");
- solrClient.add(collectionName, doc);
-
- // getCount seems to return a cumulative count, but add() results in
only 1 additional admin
- // request to fetch CLUSTERSTATUS for the collection
- assertEquals(2, adminLogs.getCount());
- assertTrue(
- adminLogs
- .pollMessage()
- .contains(
- "path=/admin/collections "
- +
"params={prs=true&action=CLUSTERSTATUS&includeAll=false"));
+ assertEquals(1, adminLogs.getCount());
+ assertTrue(
+ adminLogs
+ .pollMessage()
+ .contains(
+ "path=/admin/collections
params={prs=true&liveNodes=true&action"
+ + "=CLUSTERSTATUS&includeAll=false"));
- solrClient.commit(collectionName);
- // No additional admin requests sent
- assertEquals(2, adminLogs.getCount());
+ SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s",
"my doc");
+ solrClient.add(collectionName, doc);
- for (int i = 0; i < 3; i++) {
- assertEquals(
- 1, solrClient.query(collectionName, params("q",
"*:*")).getResults().getNumFound());
+ // getCount seems to return a cumulative count, but add() results in
only 1 additional admin
+ // request to fetch CLUSTERSTATUS for the collection
+ assertEquals(2, adminLogs.getCount());
+ assertTrue(
+ adminLogs
+ .pollMessage()
+ .contains(
+ "path=/admin/collections "
+ +
"params={prs=true&action=CLUSTERSTATUS&includeAll=false"));
+
+ solrClient.commit(collectionName);
// No additional admin requests sent
assertEquals(2, adminLogs.getCount());
+
+ for (int i = 0; i < 3; i++) {
+ assertEquals(
+ 1, solrClient.query(collectionName, params("q",
"*:*")).getResults().getNumFound());
+ // No additional admin requests sent
+ assertEquals(2, adminLogs.getCount());
+ }
}
+ } finally {
+ System.clearProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS);
Review Comment:
Please don't bother clearing system properties in tests. We've got [some
nice test
infrastructure](https://github.com/apache/solr/blob/b74b77c212cb4574210445a3075d03cc7258cc6f/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java#L92)
that makes doing so needless, and here causing you to do a needless
try-finally.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
