dsmiley commented on code in PR #4194:
URL: https://github.com/apache/solr/pull/4194#discussion_r2898899295


##########
solr/benchmark/src/java/org/apache/solr/bench/MiniClusterState.java:
##########
@@ -157,6 +158,11 @@ public void shutdownMiniCluster(BenchmarkParams 
benchmarkParams, BaseBenchState
       IOUtils.closeQuietly(client);
       cluster.shutdown();
       logClusterDirectorySize();
+
+      String orr = ObjectReleaseTracker.clearObjectTrackerAndCheckEmpty();
+      if (orr != null) {

Review Comment:
   this helped track down 2 issues!



##########
solr/benchmark/src/java/org/apache/solr/bench/search/QueryResponseWriters.java:
##########
@@ -93,6 +96,12 @@ public void setup(MiniClusterBenchState miniClusterState) 
throws Exception {
   public Object query(
       BenchState benchState, MiniClusterState.MiniClusterBenchState 
miniClusterState)
       throws SolrServerException, IOException {
-    return miniClusterState.client.request(benchState.q, collection);
+    NamedList<Object> response = miniClusterState.client.request(benchState.q, 
collection);
+    // consume the stream completely

Review Comment:
   this is necessary since otherwise the InputStreamResponseParser is left 
unconsumed and in fact it means the benchmark was mostly useless as nothing was 
transmitted since there was no consumer!  wow



##########
solr/benchmark/src/java/org/apache/solr/bench/search/QueryResponseWriters.java:
##########
@@ -57,7 +60,7 @@ public class QueryResponseWriters {
   @State(Scope.Benchmark)
   public static class BenchState {
 
-    @Param({CommonParams.JAVABIN, CommonParams.JSON, "cbor", "smile", "xml", 
"raw"})
+    @Param({CommonParams.JAVABIN, CommonParams.JSON, "cbor", "smile", "xml"})

Review Comment:
   "raw" is special; isn't working (NPE) in this scenario.  Don't think it 
makes sense to benchmark it.



##########
solr/benchmark/src/java/org/apache/solr/bench/generators/IntegersDSL.java:
##########
@@ -132,13 +132,18 @@ public IntegerMaxCardinalitySolrGen(int maxCardinality, 
Gen<Integer> integers) {
 
     @Override
     public Integer generate(SolrRandomnessSource in) {

Review Comment:
   can be called concurrently; needs to be thread-safe



##########
solr/benchmark/src/resources/configs/cloud-minimal/conf/schema.xml:
##########
@@ -36,7 +36,7 @@
     </fieldType>
 
     <!-- for versioning -->
-    <field name="_version_" type="long" indexed="true" stored="true"/>
+    <field name="_version_" type="long" indexed="true" stored="true" 
docValues="true"/>

Review Comment:
   we need docValues on `_version_`.  This ought to be a loud Solr error but it 
turned up here latently when a DBQ came in -- very weird.



-- 
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]

Reply via email to