Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Introduce a new pull-based ingestion plugin for file-based indexing (for local testing) ([#18591](https://github.com/opensearch-project/OpenSearch/pull/18591))
- Add support for search pipeline in search and msearch template ([#18564](https://github.com/opensearch-project/OpenSearch/pull/18564))
- Add BooleanQuery rewrite moving constant-scoring must clauses to filter clauses ([#18510](https://github.com/opensearch-project/OpenSearch/issues/18510))
- Add support for non-timing info in profiler ([#18460](https://github.com/opensearch-project/OpenSearch/issues/18460))

### Changed
- Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570))
Expand All @@ -37,7 +38,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Cannot communicate with HTTP/2 when reactor-netty is enabled ([#18599](https://github.com/opensearch-project/OpenSearch/pull/18599))
- Fix the visit of sub queries for HasParentQuery and HasChildQuery ([#18621](https://github.com/opensearch-project/OpenSearch/pull/18621))


### Security

[Unreleased 3.x]: https://github.com/opensearch-project/OpenSearch/compare/3.1...main
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public Weight createWeight(Query query, ScoreMode scoreMode, float boost) throws
// createWeight() is called for each query in the tree, so we tell the queryProfiler
// each invocation so that it can build an internal representation of the query
// tree
ContextualProfileBreakdown<QueryTimingType> profile = profiler.getQueryBreakdown(query);
ContextualProfileBreakdown profile = profiler.getQueryBreakdown(query);
Timer timer = profile.getTimer(QueryTimingType.CREATE_WEIGHT);
timer.start();
final Weight weight;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
*
* @opensearch.internal
*/
public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown<?>, E> {
public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown, E> {

protected ArrayList<PB> breakdowns;
/** Maps the Query to it's list of children. This is basically the dependency tree */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@

package org.opensearch.search.profile;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static java.util.Collections.emptyMap;

Expand All @@ -45,58 +48,51 @@
*
* @opensearch.internal
*/
public abstract class AbstractProfileBreakdown<T extends Enum<T>> {
public abstract class AbstractProfileBreakdown {

/**
* The accumulated timings for this query node
*/
protected final Timer[] timings;
protected final T[] timingTypes;
public static final String TIMING_TYPE_COUNT_SUFFIX = "_count";
public static final String TIMING_TYPE_START_TIME_SUFFIX = "_start_time";
protected final Map<String, ProfileMetric> metrics;

/** Sole constructor. */
public AbstractProfileBreakdown(Class<T> clazz) {
this.timingTypes = clazz.getEnumConstants();
timings = new Timer[timingTypes.length];
for (int i = 0; i < timings.length; ++i) {
timings[i] = new Timer();
}
public AbstractProfileBreakdown(Collection<Supplier<ProfileMetric>> metricSuppliers) {
this.metrics = metricSuppliers.stream().map(Supplier::get).collect(Collectors.toMap(ProfileMetric::getName, metric -> metric));
}

public Timer getTimer(T timing) {
return timings[timing.ordinal()];
public Timer getTimer(Enum<?> type) {
ProfileMetric metric = metrics.get(type.toString());
assert metric instanceof Timer : "Metric " + type + " is not a timer";
return (Timer) metric;
}

public void setTimer(T timing, Timer timer) {
timings[timing.ordinal()] = timer;
public ProfileMetric getMetric(String name) {
return metrics.get(name);

Check warning on line 67 in server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java#L67

Added line #L67 was not covered by tests
}

/**
* Build a timing count breakdown for current instance
* Build a breakdown for current instance
*/
public Map<String, Long> toBreakdownMap() {
Map<String, Long> map = new HashMap<>(this.timings.length * 3);
for (T timingType : this.timingTypes) {
map.put(timingType.toString(), this.timings[timingType.ordinal()].getApproximateTiming());
map.put(timingType + TIMING_TYPE_COUNT_SUFFIX, this.timings[timingType.ordinal()].getCount());
map.put(timingType + TIMING_TYPE_START_TIME_SUFFIX, this.timings[timingType.ordinal()].getEarliestTimerStartTime());
Map<String, Long> map = new TreeMap<>();
for (Map.Entry<String, ProfileMetric> entry : metrics.entrySet()) {
map.putAll(entry.getValue().toBreakdownMap());
}
return Collections.unmodifiableMap(map);
}

public long toNodeTime() {
long total = 0;
for (Map.Entry<String, ProfileMetric> entry : metrics.entrySet()) {
if (entry.getValue() instanceof Timer t) {
total += t.getApproximateTiming();
}
}
return total;
}

/**
* Fetch extra debugging information.
*/
public Map<String, Object> toDebugMap() {
return emptyMap();
}

public long toNodeTime() {
long total = 0;
for (T timingType : timingTypes) {
total += timings[timingType.ordinal()].getApproximateTiming();
}
return total;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
*
* @opensearch.internal
*/
public class AbstractProfiler<PB extends AbstractProfileBreakdown<?>, E> {
public abstract class AbstractProfiler<PB extends AbstractProfileBreakdown, E> {

protected final AbstractInternalProfileTree<PB, E> profileTree;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,29 @@
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.Collector;

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

/**
* Provide contextual profile breakdowns which are associated with freestyle context. Used when concurrent
* search over segments is activated and each collector needs own non-shareable profile breakdown instance.
*
* @opensearch.internal
*/
public abstract class ContextualProfileBreakdown<T extends Enum<T>> extends AbstractProfileBreakdown<T> {
public ContextualProfileBreakdown(Class<T> clazz) {
super(clazz);
public abstract class ContextualProfileBreakdown extends AbstractProfileBreakdown {

public ContextualProfileBreakdown(Collection<Supplier<ProfileMetric>> metrics) {
super(metrics);
}

/**
* Return (or create) contextual profile breakdown instance
* @param context freestyle context
* @return contextual profile breakdown instance
*/
public abstract AbstractProfileBreakdown<T> context(Object context);
public abstract AbstractProfileBreakdown context(Object context);

public void associateCollectorToLeaves(Collector collector, LeafReaderContext leaf) {}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.search.profile;

import org.opensearch.common.annotation.ExperimentalApi;

import java.util.Map;

/**
* A metric for profiling.
*/
@ExperimentalApi
public abstract class ProfileMetric {

private final String name;

public ProfileMetric(String name) {
this.name = name;
}

/**
*
* @return name of the metric
*/
public String getName() {
return name;
}

/**
*
* @return map representation of breakdown
*/
abstract public Map<String, Long> toBreakdownMap();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.search.profile;

import org.opensearch.search.profile.aggregation.AggregationTimingType;
import org.opensearch.search.profile.query.QueryTimingType;

import java.util.ArrayList;
import java.util.Collection;
import java.util.function.Supplier;

/**
* Utility class to provide profile metrics to breakdowns.
*/
public class ProfileMetricUtil {

Check warning on line 21 in server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java#L21

Added line #L21 was not covered by tests

public static Collection<Supplier<ProfileMetric>> getDefaultQueryProfileMetrics() {
Collection<Supplier<ProfileMetric>> metrics = new ArrayList<>();
for (QueryTimingType type : QueryTimingType.values()) {
metrics.add(() -> new Timer(type.toString()));
}
return metrics;
}

public static Collection<Supplier<ProfileMetric>> getAggregationProfileMetrics() {
Collection<Supplier<ProfileMetric>> metrics = new ArrayList<>();

Check warning on line 32 in server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java#L32

Added line #L32 was not covered by tests
for (AggregationTimingType type : AggregationTimingType.values()) {
metrics.add(() -> new Timer(type.toString()));

Check warning on line 34 in server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java#L34

Added line #L34 was not covered by tests
}
return metrics;

Check warning on line 36 in server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java#L36

Added line #L36 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ static void removeStartTimeFields(Map<String, Long> modifiedBreakdown) {
Iterator<Map.Entry<String, Long>> iterator = modifiedBreakdown.entrySet().iterator();
while (iterator.hasNext()) {
Map.Entry<String, Long> entry = iterator.next();
if (entry.getKey().endsWith(AbstractProfileBreakdown.TIMING_TYPE_START_TIME_SUFFIX)) {
if (entry.getKey().endsWith(Timer.TIMING_TYPE_START_TIME_SUFFIX)) {
iterator.remove();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public QueryProfiler addQueryProfiler() {

/** Get the current profiler. */
public QueryProfiler getCurrentQueryProfiler() {
return queryProfilers.get(queryProfilers.size() - 1);
return queryProfilers.getLast();
}

/** Return the list of all created {@link QueryProfiler}s so far. */
Expand Down
23 changes: 19 additions & 4 deletions server/src/main/java/org/opensearch/search/profile/Timer.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@

package org.opensearch.search.profile;

import java.util.HashMap;
import java.util.Map;

/** Helps measure how much time is spent running some methods.
* The {@link #start()} and {@link #stop()} methods should typically be called
* in a try/finally clause with {@link #start()} being called right before the
Expand All @@ -48,16 +51,19 @@
*
* @opensearch.internal
*/
public class Timer {
public class Timer extends ProfileMetric {
public static final String TIMING_TYPE_COUNT_SUFFIX = "_count";
public static final String TIMING_TYPE_START_TIME_SUFFIX = "_start_time";

private boolean doTiming;
private long timing, count, lastCount, start, earliestTimerStartTime;

public Timer() {
this(0, 0, 0, 0, 0);
public Timer(String name) {
super(name);
}

public Timer(long timing, long count, long lastCount, long start, long earliestTimerStartTime) {
public Timer(long timing, long count, long lastCount, long start, long earliestTimerStartTime, String name) {
super(name);
this.timing = timing;
this.count = count;
this.lastCount = lastCount;
Expand Down Expand Up @@ -131,4 +137,13 @@ public final long getApproximateTiming() {
}
return timing;
}

@Override
public Map<String, Long> toBreakdownMap() {
Map<String, Long> map = new HashMap<>();
map.put(getName(), getApproximateTiming());
map.put(getName() + TIMING_TYPE_COUNT_SUFFIX, getCount());
map.put(getName() + TIMING_TYPE_START_TIME_SUFFIX, getEarliestTimerStartTime());
return map;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@

import org.opensearch.common.annotation.PublicApi;
import org.opensearch.search.profile.AbstractProfileBreakdown;
import org.opensearch.search.profile.ProfileMetric;
import org.opensearch.search.profile.ProfileMetricUtil;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Supplier;

import static java.util.Collections.unmodifiableMap;

Expand All @@ -46,11 +50,15 @@
* @opensearch.api
*/
@PublicApi(since = "1.0.0")
public class AggregationProfileBreakdown extends AbstractProfileBreakdown<AggregationTimingType> {
public class AggregationProfileBreakdown extends AbstractProfileBreakdown {
private final Map<String, Object> extra = new HashMap<>();

public AggregationProfileBreakdown() {
super(AggregationTimingType.class);
this(ProfileMetricUtil.getAggregationProfileMetrics());
}

Check warning on line 58 in server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java#L57-L58

Added lines #L57 - L58 were not covered by tests

public AggregationProfileBreakdown(Collection<Supplier<ProfileMetric>> timers) {
super(timers);

Check warning on line 61 in server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java#L61

Added line #L61 was not covered by tests
}

/**
Expand All @@ -64,4 +72,5 @@
public Map<String, Object> toDebugMap() {
return unmodifiableMap(extra);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

package org.opensearch.search.profile.aggregation;

import org.opensearch.search.profile.AbstractProfileBreakdown;
import org.opensearch.search.profile.ProfileResult;
import org.opensearch.search.profile.Timer;

import java.util.HashMap;
import java.util.LinkedList;
Expand All @@ -31,7 +31,7 @@
private static final String MAX_PREFIX = "max_";
private static final String MIN_PREFIX = "min_";
private static final String AVG_PREFIX = "avg_";
private static final String START_TIME_KEY = AggregationTimingType.INITIALIZE + AbstractProfileBreakdown.TIMING_TYPE_START_TIME_SUFFIX;
private static final String START_TIME_KEY = AggregationTimingType.INITIALIZE + Timer.TIMING_TYPE_START_TIME_SUFFIX;
private static final String[] breakdownCountStatsTypes = { "build_leaf_collector_count", "collect_count" };

@Override
Expand Down Expand Up @@ -82,8 +82,7 @@
// Profiled breakdown total time
for (AggregationTimingType timingType : AggregationTimingType.values()) {
String breakdownTimingType = timingType.toString();
Long startTime = profileResult.getTimeBreakdown()
.get(breakdownTimingType + AbstractProfileBreakdown.TIMING_TYPE_START_TIME_SUFFIX);
Long startTime = profileResult.getTimeBreakdown().get(breakdownTimingType + Timer.TIMING_TYPE_START_TIME_SUFFIX);

Check warning on line 85 in server/src/main/java/org/opensearch/search/profile/aggregation/ConcurrentAggregationProfiler.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/aggregation/ConcurrentAggregationProfiler.java#L85

Added line #L85 was not covered by tests
Long endTime = startTime + profileResult.getTimeBreakdown().get(breakdownTimingType);
minSliceStartTimeMap.put(
breakdownTimingType,
Expand All @@ -103,7 +102,7 @@
// Profiled breakdown count
for (AggregationTimingType timingType : AggregationTimingType.values()) {
String breakdownType = timingType.toString();
String breakdownTypeCount = breakdownType + AbstractProfileBreakdown.TIMING_TYPE_COUNT_SUFFIX;
String breakdownTypeCount = breakdownType + Timer.TIMING_TYPE_COUNT_SUFFIX;

Check warning on line 105 in server/src/main/java/org/opensearch/search/profile/aggregation/ConcurrentAggregationProfiler.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/profile/aggregation/ConcurrentAggregationProfiler.java#L105

Added line #L105 was not covered by tests
breakdown.put(
breakdownTypeCount,
breakdown.getOrDefault(breakdownTypeCount, 0L) + profileResult.getTimeBreakdown().get(breakdownTypeCount)
Expand Down
Loading
Loading