dsmiley commented on code in PR #2571:
URL: https://github.com/apache/solr/pull/2571#discussion_r1690524467
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java:
##########
@@ -268,35 +258,90 @@ public void testAliasHandling() throws Exception {
}
@Test
- @LogLevel("org.apache.solr.servlet.HttpSolrCall=INFO")
+ @LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
public void testHttpCspPerf() throws Exception {
- String COLLECTION = "TEST_COLLECTION";
- CollectionAdminRequest.createCollection(COLLECTION, "conf", 2, 1)
- .process(cluster.getSolrClient());
- cluster.waitForActiveCollection(COLLECTION, 2, 2);
+ performTest(false, "HTTPCSPTEST",
+ 1,
+ 10,
+ //1 create collection, 2 /admin/core , 1 LISTALIASES, 1
CLUSTERSTATUS, 10 CLUSTERSTATUS with coll param
Review Comment:
I propose that collection creation be external to the logging observation.
Create it beforehand. The reason why is that it'll generate a number of calls
including internal distributed ones that are not what we're trying to observe
(we're only trying to observe requests *from* our client, not from the Overseer
or whatever Solr node.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java:
##########
@@ -268,35 +258,90 @@ public void testAliasHandling() throws Exception {
}
@Test
- @LogLevel("org.apache.solr.servlet.HttpSolrCall=INFO")
+ @LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
public void testHttpCspPerf() throws Exception {
- String COLLECTION = "TEST_COLLECTION";
- CollectionAdminRequest.createCollection(COLLECTION, "conf", 2, 1)
- .process(cluster.getSolrClient());
- cluster.waitForActiveCollection(COLLECTION, 2, 2);
+ performTest(false, "HTTPCSPTEST",
+ 1,
+ 10,
+ //1 create collection, 2 /admin/core , 1 LISTALIASES, 1
CLUSTERSTATUS, 10 CLUSTERSTATUS with coll param
+ 15,
+ 15);
- SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s", "my
doc");
- httpBasedCloudSolrClient.add(COLLECTION, doc);
- httpBasedCloudSolrClient.commit(COLLECTION);
+ }
+ @Test
+ @LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
+ public void testZkCspPerf() throws Exception {
- for (int i = 0; i < 3; i++) {
- assertEquals(
- 1,
- httpBasedCloudSolrClient
- .query(COLLECTION, params("q", "*:*"))
- .getResults()
- .getNumFound());
+ performTest(true, "ZKCSPTEST",
+ 0,
+ 0,
+ //1 create collection, 2 /admin/core
+ 3,
+ 15);
+
+ }
+
+ private CloudSolrClient createHttpCSPBasedCloudSolrClient() {
Review Comment:
"Csp" capitalization.
Why put "based" in it... The corresponding ZK one does not have that. I
suggest naming them both in harmony like createHttpCspCloudSolrClient and
createZkCspCloudSolrClient and which are both plenty long names as it is
##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -325,6 +325,11 @@ protected void init() throws Exception {
addCollectionParamIfNeeded(getCollectionsList());
action = PROCESS;
+
+ //this request would be processed by Solr
+ if(invalidStates ==null){
Review Comment:
Please don't add this; I don't think it's necessary. As I understand it,
you added this to observe "nonAdmin" requests? That goes to a different
logger, the SolrCore "Request" logger that already exists at info.
But even then, why listen for the non-admin calls? Those are only going to
be query and indexing which is outside of the scope of what we're focused on
right now. Eventually, future work items that look closely at stateVer
handling, I could see doing this, however.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java:
##########
@@ -268,35 +258,90 @@ public void testAliasHandling() throws Exception {
}
@Test
- @LogLevel("org.apache.solr.servlet.HttpSolrCall=INFO")
+ @LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
public void testHttpCspPerf() throws Exception {
- String COLLECTION = "TEST_COLLECTION";
- CollectionAdminRequest.createCollection(COLLECTION, "conf", 2, 1)
- .process(cluster.getSolrClient());
- cluster.waitForActiveCollection(COLLECTION, 2, 2);
+ performTest(false, "HTTPCSPTEST",
+ 1,
+ 10,
+ //1 create collection, 2 /admin/core , 1 LISTALIASES, 1
CLUSTERSTATUS, 10 CLUSTERSTATUS with coll param
+ 15,
+ 15);
- SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s", "my
doc");
- httpBasedCloudSolrClient.add(COLLECTION, doc);
- httpBasedCloudSolrClient.commit(COLLECTION);
+ }
+ @Test
+ @LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
+ public void testZkCspPerf() throws Exception {
Review Comment:
Why test with ZkCsp using an HTTP logging approach? ZkCsp does not make
HTTP calls. If we could monitor TCP ZK connections somehow, *that* would be
interesting. Such an approach would be doable with distributed tracing tech or
SolrZkClient metrics (even easier) but I don't want to suggest expanding the
scope/effort of this PR too much.
--
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
