cpoerschke commented on a change in pull request #78:
URL: https://github.com/apache/solr/pull/78#discussion_r618591504



##########
File path: 
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
##########
@@ -236,6 +240,21 @@ public void prepare(ResponseBuilder rb) throws IOException
     if (rb.getSortSpec().getOffset() < 0) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "'start' 
parameter cannot be negative");
     }
+    if(rb.getSortSpec().getCount() > ROWS_WARN_THRESHOLD && 
shouldLogPeriodically("rowsWarn", 60)) {
+      log.warn("Very high 'rows' parameter detected. This may lead to 
performance- and memory problems. " +
+          "Consider pagination, see 
https://solr.apache.org/guide/pagination-of-results.html. " +
+          "This warning will mute for 60s.");

Review comment:
       How about making the logging interval (and the threshold) configurable? 
I've put up a possible approach in 
https://github.com/cominvent/solr/compare/solr15252-log-high-rows...cpoerschke:solr15252-log-high-rows
 and the idea would be that both could be made default (or invariant) 
parameters on the request handler. Admittedly 
`rows.warnThresholdLogIntervalSeconds` is a bit of a long name though.




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

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

Reply via email to