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]