Avoid double decrement on current query counter
This commit fixes a double decrement bug on the current query counter. The double decrement arises in a situation when the fetch phase is inlined for a query that is only touching one shard. After the query phase succeeds we decrement the current query counter. If the fetch phase ultimately fails, an exception is thrown and we decrement the current query counter again in the catch block. We also add assertions that all current stats counters remain non-negative at all times. Relates #24922
This commit is contained in:
parent
5da8ce8318
commit
3448028af7
|
@ -80,8 +80,10 @@ public final class ShardSearchStats implements SearchOperationListener {
|
|||
computeStats(searchContext, statsHolder -> {
|
||||
if (searchContext.hasOnlySuggest()) {
|
||||
statsHolder.suggestCurrent.dec();
|
||||
assert statsHolder.suggestCurrent.count() >= 0;
|
||||
} else {
|
||||
statsHolder.queryCurrent.dec();
|
||||
assert statsHolder.queryCurrent.count() >= 0;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
@ -92,9 +94,11 @@ public final class ShardSearchStats implements SearchOperationListener {
|
|||
if (searchContext.hasOnlySuggest()) {
|
||||
statsHolder.suggestMetric.inc(tookInNanos);
|
||||
statsHolder.suggestCurrent.dec();
|
||||
assert statsHolder.suggestCurrent.count() >= 0;
|
||||
} else {
|
||||
statsHolder.queryMetric.inc(tookInNanos);
|
||||
statsHolder.queryCurrent.dec();
|
||||
assert statsHolder.queryCurrent.count() >= 0;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
@ -114,6 +118,7 @@ public final class ShardSearchStats implements SearchOperationListener {
|
|||
computeStats(searchContext, statsHolder -> {
|
||||
statsHolder.fetchMetric.inc(tookInNanos);
|
||||
statsHolder.fetchCurrent.dec();
|
||||
assert statsHolder.fetchCurrent.count() >= 0;
|
||||
});
|
||||
}
|
||||
|
||||
|
@ -174,6 +179,7 @@ public final class ShardSearchStats implements SearchOperationListener {
|
|||
@Override
|
||||
public void onFreeScrollContext(SearchContext context) {
|
||||
totalStats.scrollCurrent.dec();
|
||||
assert totalStats.scrollCurrent.count() >= 0;
|
||||
totalStats.scrollMetric.inc(System.nanoTime() - context.getOriginNanoTime());
|
||||
}
|
||||
|
||||
|
|
|
@ -251,6 +251,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
|
|||
final SearchContext context = createAndPutContext(request);
|
||||
final SearchOperationListener operationListener = context.indexShard().getSearchOperationListener();
|
||||
context.incRef();
|
||||
boolean queryPhaseSuccess = false;
|
||||
try {
|
||||
context.setTask(task);
|
||||
operationListener.onPreQueryPhase(context);
|
||||
|
@ -265,6 +266,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
|
|||
contextProcessedSuccessfully(context);
|
||||
}
|
||||
final long afterQueryTime = System.nanoTime();
|
||||
queryPhaseSuccess = true;
|
||||
operationListener.onQueryPhase(context, afterQueryTime - time);
|
||||
if (request.numberOfShards() == 1) {
|
||||
return executeFetchPhase(context, operationListener, afterQueryTime);
|
||||
|
@ -276,7 +278,9 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
|
|||
e = (e.getCause() == null || e.getCause() instanceof Exception) ?
|
||||
(Exception) e.getCause() : new ElasticsearchException(e.getCause());
|
||||
}
|
||||
operationListener.onFailedQueryPhase(context);
|
||||
if (!queryPhaseSuccess) {
|
||||
operationListener.onFailedQueryPhase(context);
|
||||
}
|
||||
logger.trace("Query phase failed", e);
|
||||
processFailure(context, e);
|
||||
throw ExceptionsHelper.convertToRuntime(e);
|
||||
|
|
Loading…
Reference in New Issue