Skip to content

Commit 7fb3442

Browse files
authored
Switch to Lucene ProfilerCollector (elastic#95526)
The profile functionality relies on two custom collector implementations that wrap a given collector to monitor its execution time and expose the profile results tree. The functionality has been contributed to Lucene with apache/lucene#144 hence we can start migrating to relying on the functionalities that Lucene offers for it. We keep our own collector which extends ProfilerCollector from Lucene and exposes profile results that extends ProfilerCollectorResult which can be serialized over the wire as well as to xcontent.
1 parent 545a51d commit 7fb3442

File tree

5 files changed

+36
-254
lines changed

5 files changed

+36
-254
lines changed

server/src/main/java/org/elasticsearch/search/profile/query/CollectorResult.java

Lines changed: 17 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.elasticsearch.search.profile.query;
1010

11+
import org.apache.lucene.sandbox.search.ProfilerCollectorResult;
1112
import org.elasticsearch.common.Strings;
1213
import org.elasticsearch.common.io.stream.StreamInput;
1314
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -32,7 +33,7 @@
3233
* Collectors used in the search. Children CollectorResult's may be
3334
* embedded inside of a parent CollectorResult
3435
*/
35-
public class CollectorResult implements ToXContentObject, Writeable {
36+
public class CollectorResult extends ProfilerCollectorResult implements ToXContentObject, Writeable {
3637

3738
public static final String REASON_SEARCH_COUNT = "search_count";
3839
public static final String REASON_SEARCH_TOP_HITS = "search_top_hits";
@@ -49,85 +50,27 @@ public class CollectorResult implements ToXContentObject, Writeable {
4950
private static final ParseField TIME_NANOS = new ParseField("time_in_nanos");
5051
private static final ParseField CHILDREN = new ParseField("children");
5152

52-
/**
53-
* A more friendly representation of the Collector's class name
54-
*/
55-
private final String collectorName;
56-
57-
/**
58-
* A "hint" to help provide some context about this Collector
59-
*/
60-
private final String reason;
61-
62-
/**
63-
* The total elapsed time for this Collector
64-
*/
65-
private final long time;
66-
67-
/**
68-
* A list of children collectors "embedded" inside this collector
69-
*/
70-
private final List<CollectorResult> children;
71-
7253
public CollectorResult(String collectorName, String reason, long time, List<CollectorResult> children) {
73-
this.collectorName = collectorName;
74-
this.reason = reason;
75-
this.time = time;
76-
this.children = children;
54+
super(collectorName, reason, time, new ArrayList<>(children));
7755
}
7856

7957
/**
8058
* Read from a stream.
8159
*/
8260
public CollectorResult(StreamInput in) throws IOException {
83-
this.collectorName = in.readString();
84-
this.reason = in.readString();
85-
this.time = in.readLong();
86-
int size = in.readVInt();
87-
this.children = new ArrayList<>(size);
88-
for (int i = 0; i < size; i++) {
89-
CollectorResult child = new CollectorResult(in);
90-
this.children.add(child);
91-
}
61+
super(in.readString(), in.readString(), in.readLong(), in.readList(CollectorResult::new));
9262
}
9363

9464
@Override
9565
public void writeTo(StreamOutput out) throws IOException {
96-
out.writeString(collectorName);
97-
out.writeString(reason);
98-
out.writeLong(time);
99-
out.writeVInt(children.size());
100-
for (CollectorResult child : children) {
101-
child.writeTo(out);
102-
}
66+
out.writeString(getName());
67+
out.writeString(getReason());
68+
out.writeLong(getTime());
69+
out.writeList(getCollectorResults());
10370
}
10471

105-
/**
106-
* @return the profiled time for this collector (inclusive of children)
107-
*/
108-
public long getTime() {
109-
return this.time;
110-
}
111-
112-
/**
113-
* @return a human readable "hint" about what this collector was used for
114-
*/
115-
public String getReason() {
116-
return this.reason;
117-
}
118-
119-
/**
120-
* @return the lucene class name of the collector
121-
*/
122-
public String getName() {
123-
return this.collectorName;
124-
}
125-
126-
/**
127-
* @return a list of children collectors
128-
*/
129-
public List<CollectorResult> getProfiledChildren() {
130-
return children;
72+
public List<CollectorResult> getCollectorResults() {
73+
return super.getProfiledChildren().stream().map(profilerCollectorResult -> (CollectorResult) profilerCollectorResult).toList();
13174
}
13275

13376
@Override
@@ -136,15 +79,15 @@ public boolean equals(Object obj) {
13679
return false;
13780
}
13881
CollectorResult other = (CollectorResult) obj;
139-
return collectorName.equals(other.collectorName)
140-
&& reason.equals(other.reason)
141-
&& time == other.time
142-
&& children.equals(other.children);
82+
return getName().equals(other.getName())
83+
&& getReason().equals(other.getReason())
84+
&& getTime() == other.getTime()
85+
&& getCollectorResults().equals(other.getCollectorResults());
14386
}
14487

14588
@Override
14689
public int hashCode() {
147-
return Objects.hash(collectorName, reason, time, children);
90+
return Objects.hash(getName(), getReason(), getTime(), getCollectorResults());
14891
}
14992

15093
@Override
@@ -162,9 +105,9 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
162105
}
163106
builder.field(TIME_NANOS.getPreferredName(), getTime());
164107

165-
if (children.isEmpty() == false) {
108+
if (getProfiledChildren().isEmpty() == false) {
166109
builder = builder.startArray(CHILDREN.getPreferredName());
167-
for (CollectorResult child : children) {
110+
for (CollectorResult child : getCollectorResults()) {
168111
builder = child.toXContent(builder, params);
169112
}
170113
builder = builder.endArray();

server/src/main/java/org/elasticsearch/search/profile/query/InternalProfileCollector.java

Lines changed: 14 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,12 @@
88

99
package org.elasticsearch.search.profile.query;
1010

11-
import org.apache.lucene.index.LeafReaderContext;
11+
import org.apache.lucene.sandbox.search.ProfilerCollector;
1212
import org.apache.lucene.search.Collector;
13-
import org.apache.lucene.search.LeafCollector;
14-
import org.apache.lucene.search.ScoreMode;
1513

16-
import java.io.IOException;
1714
import java.util.ArrayList;
15+
import java.util.Arrays;
1816
import java.util.List;
19-
import java.util.Objects;
2017

2118
/**
2219
* This class wraps a Lucene Collector and times the execution of:
@@ -27,56 +24,13 @@
2724
*
2825
* InternalProfiler facilitates the linking of the Collector graph
2926
*/
30-
public class InternalProfileCollector implements Collector {
27+
public class InternalProfileCollector extends ProfilerCollector {
3128

32-
/**
33-
* A more friendly representation of the Collector's class name
34-
*/
35-
private final String collectorName;
36-
37-
/**
38-
* A "hint" to help provide some context about this Collector
39-
*/
40-
private final String reason;
41-
42-
/** The wrapped collector */
43-
private final ProfileCollector collector;
44-
45-
/**
46-
* An array of "embedded" children collectors
47-
*/
4829
private final InternalProfileCollector[] children;
4930

50-
public InternalProfileCollector(Collector collector, String reason, Collector... children) {
51-
this.collector = new ProfileCollector(collector);
52-
this.reason = reason;
53-
this.collectorName = deriveCollectorName(collector);
54-
Objects.requireNonNull(children, "children collectors cannot be null");
55-
this.children = new InternalProfileCollector[children.length];
56-
for (int i = 0; i < children.length; i++) {
57-
this.children[i] = (InternalProfileCollector) Objects.requireNonNull(children[i], "child collector cannot be null");
58-
}
59-
}
60-
61-
/**
62-
* @return the profiled time for this collector (inclusive of children)
63-
*/
64-
public long getTime() {
65-
return collector.getTime();
66-
}
67-
68-
/**
69-
* @return a human readable "hint" about what this collector was used for
70-
*/
71-
public String getReason() {
72-
return this.reason;
73-
}
74-
75-
/**
76-
* @return the lucene class name of the collector
77-
*/
78-
public String getName() {
79-
return this.collectorName;
31+
public InternalProfileCollector(Collector collector, String reason, InternalProfileCollector... children) {
32+
super(collector, reason, Arrays.asList(children));
33+
this.children = children;
8034
}
8135

8236
/**
@@ -88,7 +42,8 @@ public String getName() {
8842
* @param c The Collector to derive a name from
8943
* @return A (hopefully) prettier name
9044
*/
91-
private String deriveCollectorName(Collector c) {
45+
@Override
46+
protected String deriveCollectorName(Collector c) {
9247
String s = c.getClass().getSimpleName();
9348

9449
// MutiCollector which wraps multiple BucketCollectors is generated
@@ -99,32 +54,18 @@ private String deriveCollectorName(Collector c) {
9954
}
10055

10156
// Aggregation collector toString()'s include the user-defined agg name
102-
if (reason.equals(CollectorResult.REASON_AGGREGATION) || reason.equals(CollectorResult.REASON_AGGREGATION_GLOBAL)) {
103-
s += ": [" + c.toString() + "]";
57+
if (getReason().equals(CollectorResult.REASON_AGGREGATION) || getReason().equals(CollectorResult.REASON_AGGREGATION_GLOBAL)) {
58+
s += ": [" + c + "]";
10459
}
10560
return s;
10661
}
10762

108-
@Override
109-
public LeafCollector getLeafCollector(LeafReaderContext context) throws IOException {
110-
return collector.getLeafCollector(context);
111-
}
112-
113-
@Override
114-
public ScoreMode scoreMode() {
115-
return collector.scoreMode();
116-
}
117-
11863
public CollectorResult getCollectorTree() {
119-
return InternalProfileCollector.doGetCollectorTree(this);
120-
}
121-
122-
private static CollectorResult doGetCollectorTree(InternalProfileCollector collector) {
123-
List<CollectorResult> childResults = new ArrayList<>(collector.children.length);
124-
for (InternalProfileCollector child : collector.children) {
125-
CollectorResult result = doGetCollectorTree(child);
64+
List<CollectorResult> childResults = new ArrayList<>(children.length);
65+
for (InternalProfileCollector child : children) {
66+
CollectorResult result = child.getCollectorTree();
12667
childResults.add(result);
12768
}
128-
return new CollectorResult(collector.getName(), collector.getReason(), collector.getTime(), childResults);
69+
return new CollectorResult(getName(), getReason(), getTime(), childResults);
12970
}
13071
}

server/src/main/java/org/elasticsearch/search/profile/query/ProfileCollector.java

Lines changed: 0 additions & 90 deletions
This file was deleted.

server/src/main/java/org/elasticsearch/search/query/QueryPhase.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252

5353
import java.io.IOException;
5454
import java.util.ArrayList;
55+
import java.util.Arrays;
5556
import java.util.List;
5657
import java.util.concurrent.ExecutorService;
5758

@@ -309,7 +310,10 @@ private static CollectorManager<Collector, Void> wrapWithProfilerCollectorManage
309310
if (profilers == null) {
310311
return new SingleThreadCollectorManager(collector);
311312
}
312-
return new InternalProfileCollectorManager(new InternalProfileCollector(collector, profilerName, children));
313+
InternalProfileCollector[] childProfileCollectors = Arrays.stream(children)
314+
.map(c -> (InternalProfileCollector) c)
315+
.toArray(InternalProfileCollector[]::new);
316+
return new InternalProfileCollectorManager(new InternalProfileCollector(collector, profilerName, childProfileCollectors));
313317
}
314318

315319
private static void searchWithCollectorManager(

0 commit comments

Comments
 (0)