Skip to content

Commit 6f56752

Browse files
authored
ProfileCollectorManager to support child profile collectors (elastic#97387)
`ProfileCollectorManager` is our collector manager used for profiling which supports parallel collection. We have integrated it in the DFS phase which has a very simple collection hierarcy: a top docs collector only. In the query phase, we may have a top-level collector that holds two sub-collectors where each one is profiled separately. This commit adds support for children profilers to `ProfileCollectorManager` so that we can use it in the query phase as well.
1 parent 5c42bb5 commit 6f56752

File tree

5 files changed

+260
-54
lines changed

5 files changed

+260
-54
lines changed

docs/changelog/97387.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 97387
2+
summary: '`ProfileCollectorManager` to support child profile collectors'
3+
area: Search
4+
type: enhancement
5+
issues: []

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,14 @@ public void writeTo(StreamOutput out) throws IOException {
6464
out.writeString(getName());
6565
out.writeString(getReason());
6666
out.writeLong(getTime());
67-
out.writeList(getCollectorResults());
67+
out.writeList(getChildrenResults());
6868
}
6969

70-
public List<CollectorResult> getCollectorResults() {
70+
/**
71+
* Exposes a list of children collector results. Same as {@link ProfilerCollectorResult#getProfiledChildren()} with each
72+
* item in the list being cast to a {@link CollectorResult}
73+
*/
74+
public List<CollectorResult> getChildrenResults() {
7175
return super.getProfiledChildren().stream().map(profilerCollectorResult -> (CollectorResult) profilerCollectorResult).toList();
7276
}
7377

@@ -80,12 +84,12 @@ public boolean equals(Object obj) {
8084
return getName().equals(other.getName())
8185
&& getReason().equals(other.getReason())
8286
&& getTime() == other.getTime()
83-
&& getCollectorResults().equals(other.getCollectorResults());
87+
&& getChildrenResults().equals(other.getChildrenResults());
8488
}
8589

8690
@Override
8791
public int hashCode() {
88-
return Objects.hash(getName(), getReason(), getTime(), getCollectorResults());
92+
return Objects.hash(getName(), getReason(), getTime(), getChildrenResults());
8993
}
9094

9195
@Override
@@ -105,7 +109,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
105109

106110
if (getProfiledChildren().isEmpty() == false) {
107111
builder = builder.startArray(CHILDREN.getPreferredName());
108-
for (CollectorResult child : getCollectorResults()) {
112+
for (CollectorResult child : getChildrenResults()) {
109113
builder = child.toXContent(builder, params);
110114
}
111115
builder = builder.endArray();

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

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,54 +8,122 @@
88

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

11+
import org.apache.lucene.sandbox.search.ProfilerCollector;
1112
import org.apache.lucene.search.Collector;
1213
import org.apache.lucene.search.CollectorManager;
1314

1415
import java.io.IOException;
16+
import java.util.ArrayList;
1517
import java.util.Collection;
16-
import java.util.Collections;
1718
import java.util.List;
18-
import java.util.stream.Collectors;
1919

2020
/**
2121
* A {@link CollectorManager} that takes another CollectorManager as input and wraps all Collectors generated by it
2222
* in an {@link InternalProfileCollector}. It delegates all the profiling to the generated collectors via {@link #getCollectorTree()}
23-
* and joins them up when its {@link #reduce} method is called. The profile result can
23+
* and joins the different collector trees together when its {@link #reduce} method is called.
24+
* Supports optionally providing sub-collector managers for top docs as well as aggs collection, so that each
25+
* {@link InternalProfileCollector} created is provided with the corresponding sub-collectors that are children of the top-level collector.
26+
* @param <T> the return type of the wrapped collector manager, which the reduce method returns.
2427
*/
2528
public final class ProfileCollectorManager<T> implements CollectorManager<InternalProfileCollector, T> {
2629

27-
private final CollectorManager<Collector, T> collectorManager;
30+
private final CollectorManager<? extends Collector, T> collectorManager;
2831
private final String reason;
32+
private final ProfileCollectorManager<?> topDocsSubCollectorManager;
33+
private final ProfileCollectorManager<?> aggsSubCollectorManager;
34+
// this is a bit of a hack: it allows us to retrieve the last collector that newCollector has returned for sub-collector managers,
35+
// so that we can provide them to InternalProfileCollector's constructor as children. This is fine as newCollector does not get called
36+
// concurrently, but rather in advance before parallelizing the collection
37+
private InternalProfileCollector profileCollector;
38+
2939
private CollectorResult collectorTree;
3040

31-
@SuppressWarnings("unchecked")
3241
public ProfileCollectorManager(CollectorManager<? extends Collector, T> collectorManager, String reason) {
33-
this.collectorManager = (CollectorManager<Collector, T>) collectorManager;
42+
this(collectorManager, reason, null, null);
43+
}
44+
45+
public ProfileCollectorManager(
46+
CollectorManager<? extends Collector, T> collectorManager,
47+
String reason,
48+
ProfileCollectorManager<?> topDocsSubCollectorManager,
49+
ProfileCollectorManager<?> aggsSubCollectorManager
50+
) {
51+
this.collectorManager = collectorManager;
3452
this.reason = reason;
53+
assert assertSubCollectorManagers() : "top docs manager is null while aggs manager isn't";
54+
this.topDocsSubCollectorManager = topDocsSubCollectorManager;
55+
this.aggsSubCollectorManager = aggsSubCollectorManager;
56+
}
57+
58+
private boolean assertSubCollectorManagers() {
59+
if (aggsSubCollectorManager != null) {
60+
return topDocsSubCollectorManager != null;
61+
}
62+
return true;
3563
}
3664

3765
@Override
3866
public InternalProfileCollector newCollector() throws IOException {
39-
return new InternalProfileCollector(collectorManager.newCollector(), reason);
67+
Collector collector = collectorManager.newCollector();
68+
if (aggsSubCollectorManager == null && topDocsSubCollectorManager == null) {
69+
profileCollector = new InternalProfileCollector(collector, reason);
70+
} else if (aggsSubCollectorManager == null) {
71+
assert topDocsSubCollectorManager.profileCollector != null;
72+
profileCollector = new InternalProfileCollector(collector, reason, topDocsSubCollectorManager.profileCollector);
73+
} else {
74+
assert topDocsSubCollectorManager.profileCollector != null && aggsSubCollectorManager.profileCollector != null;
75+
profileCollector = new InternalProfileCollector(
76+
collector,
77+
reason,
78+
topDocsSubCollectorManager.profileCollector,
79+
aggsSubCollectorManager.profileCollector
80+
);
81+
}
82+
return profileCollector;
4083
}
4184

85+
@Override
4286
public T reduce(Collection<InternalProfileCollector> profileCollectors) throws IOException {
4387
assert profileCollectors.size() > 0 : "at least one collector expected";
44-
List<Collector> unwrapped = profileCollectors.stream()
45-
.map(InternalProfileCollector::getWrappedCollector)
46-
.collect(Collectors.toList());
47-
T returnValue = collectorManager.reduce(unwrapped);
48-
49-
List<CollectorResult> resultsPerProfiler = profileCollectors.stream()
50-
.map(ipc -> ipc.getCollectorTree())
51-
.collect(Collectors.toList());
88+
List<Collector> unwrapped = profileCollectors.stream().map(InternalProfileCollector::getWrappedCollector).toList();
89+
@SuppressWarnings("unchecked")
90+
CollectorManager<Collector, T> cm = (CollectorManager<Collector, T>) collectorManager;
91+
T returnValue = cm.reduce(unwrapped);
5292

93+
List<CollectorResult> resultsPerProfiler = profileCollectors.stream().map(InternalProfileCollector::getCollectorTree).toList();
5394
long totalTime = resultsPerProfiler.stream().map(CollectorResult::getTime).reduce(0L, Long::sum);
5495
String collectorName = resultsPerProfiler.get(0).getName();
55-
this.collectorTree = new CollectorResult(collectorName, reason, totalTime, Collections.emptyList());
96+
assert profileCollectors.stream().map(ProfilerCollector::getReason).allMatch(reason::equals);
97+
assert profileCollectors.stream().map(ProfilerCollector::getName).allMatch(collectorName::equals);
98+
assert assertChildrenSize(resultsPerProfiler);
99+
100+
List<CollectorResult> childrenResults = new ArrayList<>();
101+
// for the children collector managers, we rely on the chain on reduce calls to make their collector results available
102+
if (topDocsSubCollectorManager != null) {
103+
childrenResults.add(topDocsSubCollectorManager.getCollectorTree());
104+
}
105+
if (aggsSubCollectorManager != null) {
106+
childrenResults.add(aggsSubCollectorManager.getCollectorTree());
107+
}
108+
this.collectorTree = new CollectorResult(collectorName, reason, totalTime, childrenResults);
109+
56110
return returnValue;
57111
}
58112

113+
private boolean assertChildrenSize(List<CollectorResult> resultsPerProfiler) {
114+
int expectedSize = 0;
115+
if (topDocsSubCollectorManager != null) {
116+
expectedSize++;
117+
}
118+
if (aggsSubCollectorManager != null) {
119+
expectedSize++;
120+
}
121+
final int expectedChildrenSize = expectedSize;
122+
return resultsPerProfiler.stream()
123+
.map(collectorResult -> collectorResult.getChildrenResults().size())
124+
.allMatch(integer -> integer == expectedChildrenSize);
125+
}
126+
59127
public CollectorResult getCollectorTree() {
60128
if (this.collectorTree == null) {
61129
throw new IllegalStateException("A collectorTree hasn't been set yet. Call reduce() before attempting to retrieve it");

server/src/test/java/org/elasticsearch/search/dfs/DfsPhaseTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ protected LeafSlice[] slices(List<LeafReaderContext> leaves) {
101101
assertEquals("SimpleTopScoreDocCollector", (collectorResult.getName()));
102102
assertEquals("search_top_hits", (collectorResult.getReason()));
103103
assertTrue(collectorResult.getTime() > 0);
104-
List<CollectorResult> children = collectorResult.getCollectorResults();
104+
List<CollectorResult> children = collectorResult.getChildrenResults();
105105
if (children.size() > 0) {
106106
long totalTime = 0L;
107107
for (CollectorResult child : children) {

0 commit comments

Comments
 (0)