sigram commented on code in PR #2237:
URL: https://github.com/apache/solr/pull/2237#discussion_r1478749236
##########
solr/core/src/java/org/apache/solr/request/SolrRequestInfo.java:
##########
@@ -210,6 +217,12 @@ public void addCloseHook(Closeable hook) {
}
}
+ public QueryLimits getLimits() {
+ return req == null
+ ? QueryLimits.NONE
+ : (QueryLimits) req.getContext().computeIfAbsent(LIMITS_KEY, (k) ->
new QueryLimits(req));
Review Comment:
I think this is still incorrect... Since it uses the latest SRI from the
stack it will completely ignore the already existing QueryLimits created at the
bottom of the stack, and create a new instance to track just this part of the
processing - whereas we want to track the resource consumption across the whole
end-to-end request processing, beginning with the first SRI (at the bottom of
the stack) allocated to this query.
##########
solr/core/src/java/org/apache/solr/search/QueryLimits.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search;
+
+import static org.apache.solr.search.SolrQueryTimeLimit.hasTimeLimit;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.lucene.index.QueryTimeout;
+import org.apache.solr.request.SolrQueryRequest;
+
+/**
+ * Represents the limitations on the query. These limits might be wall clock
time, cpu time, memory,
+ * or other resource limits. Exceeding any specified limit will cause {@link
#shouldExit()} to
+ * return true the next time it is checked (it may be checked in either Lucene
code or Solr code)
+ */
+public class QueryLimits implements QueryTimeout {
+ private final List<QueryTimeout> limits =
+ new ArrayList<>(3); // timeAllowed, cpu, and memory anticipated
+
+ public static QueryLimits NONE = new QueryLimits();
+
+ private QueryLimits() {}
+
+ /**
+ * Implementors of a Query Limit should add an if block here to activate it,
and typically this if
+ * statement will hinge on hasXXXLimit() static method attached to the
implementation class.
+ *
+ * @param req the current SolrQueryRequest.
+ */
+ public QueryLimits(SolrQueryRequest req) {
+ if (hasTimeLimit(req)) {
+ limits.add(new SolrQueryTimeLimit(req));
+ }
+ }
+
+ @Override
+ public boolean shouldExit() {
+ return limits.stream().anyMatch(QueryTimeout::shouldExit);
+ }
+
+ /**
+ * Method to diagnose limit exceeded. Note that while this should always
list the exceeded limit,
+ * it may also nominate additional limits that have been exceeded since the
actual check that
+ * cause the failure. This gap is intentional to avoid overly complicated
(and possibly expensive)
+ * tracking code that would have to run within the shouldExit method. This
method should only be
+ * used to report a failure since it incurs the cost of rechecking every
configured limit and does
+ * not short circuit.
+ *
+ * @return A string describing the state pass/fail state of each limit
specified for this request.
+ */
+ public String limitStatusMessage() {
+ StringBuilder sb = new StringBuilder();
+ boolean first = true;
+ for (QueryTimeout limit : limits) {
+ if (first) {
+ first = false;
+ sb.append("Query limits:");
+ }
+ sb.append("[");
+ sb.append(limit.getClass().getName());
Review Comment:
Maybe getShortName() ?
--
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]
