Skip to content

Commit 875486c

Browse files
gdatahubclaude
andauthored
fix(graphql): revert Container.entities count-only batching (#18640) (#19104)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c2a7421 commit 875486c

6 files changed

Lines changed: 11 additions & 1068 deletions

File tree

datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.linkedin.datahub.graphql.concurrency.GraphQLConcurrencyUtils;
2626
import com.linkedin.datahub.graphql.featureflags.FeatureFlags;
2727
import com.linkedin.datahub.graphql.generated.*;
28-
import com.linkedin.datahub.graphql.loaders.ContainerEntityCountsBatchLoader;
2928
import com.linkedin.datahub.graphql.loaders.DomainEntityCountsBatchLoader;
3029
import com.linkedin.datahub.graphql.plugins.SemanticSearchPlugin;
3130
import com.linkedin.datahub.graphql.resolvers.MeResolver;
@@ -961,9 +960,6 @@ public GraphQLEngine.Builder builder() {
961960
builder
962961
.addDataLoaders(loaderSuppliers(loadableTypes))
963962
.addDataLoader("Aspect", context -> createDataLoader(aspectType, context))
964-
.addDataLoader(
965-
ContainerEntityCountsBatchLoader.LOADER_NAME,
966-
context -> ContainerEntityCountsBatchLoader.create(entityClient, context))
967963
.addDataLoader(
968964
DomainEntityCountsBatchLoader.LOADER_NAME,
969965
context -> DomainEntityCountsBatchLoader.create(entityClient, context))

datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/loaders/ContainerEntityCountsBatchLoader.java

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

datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/container/ContainerEntitiesResolver.java

Lines changed: 7 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,11 @@
44
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;
55

66
import com.google.common.collect.ImmutableList;
7-
import com.google.common.collect.ImmutableSet;
87
import com.linkedin.datahub.graphql.QueryContext;
98
import com.linkedin.datahub.graphql.concurrency.GraphQLConcurrencyUtils;
109
import com.linkedin.datahub.graphql.generated.Container;
1110
import com.linkedin.datahub.graphql.generated.ContainerEntitiesInput;
12-
import com.linkedin.datahub.graphql.generated.FacetFilterInput;
1311
import com.linkedin.datahub.graphql.generated.SearchResults;
14-
import com.linkedin.datahub.graphql.loaders.ContainerEntityCountsBatchLoader;
1512
import com.linkedin.datahub.graphql.types.mappers.UrnSearchResultsMapper;
1613
import com.linkedin.entity.client.EntityClient;
1714
import com.linkedin.metadata.Constants;
@@ -23,21 +20,16 @@
2320
import com.linkedin.metadata.query.filter.Filter;
2421
import graphql.schema.DataFetcher;
2522
import graphql.schema.DataFetchingEnvironment;
26-
import graphql.schema.DataFetchingFieldSelectionSet;
27-
import graphql.schema.SelectedField;
2823
import java.util.Collections;
2924
import java.util.List;
30-
import java.util.Set;
3125
import java.util.concurrent.CompletableFuture;
32-
import javax.annotation.Nullable;
3326
import lombok.extern.slf4j.Slf4j;
34-
import org.dataloader.DataLoader;
3527

3628
/** Retrieves a list of historical executions for a particular source. */
3729
@Slf4j
3830
public class ContainerEntitiesResolver implements DataFetcher<CompletableFuture<SearchResults>> {
3931

40-
public static final List<String> CONTAINABLE_ENTITY_NAMES =
32+
static final List<String> CONTAINABLE_ENTITY_NAMES =
4133
ImmutableList.of(
4234
Constants.DATASET_ENTITY_NAME,
4335
Constants.CHART_ENTITY_NAME,
@@ -50,11 +42,6 @@ public class ContainerEntitiesResolver implements DataFetcher<CompletableFuture<
5042
private static final Integer DEFAULT_COUNT = 20;
5143
private static final ContainerEntitiesInput DEFAULT_ENTITIES_INPUT = new ContainerEntitiesInput();
5244

53-
// Selections answerable from an aggregation alone. Deliberately excludes start/count: those are
54-
// echoed from the request rather than reported by the search layer on this path, so we only take
55-
// it when the caller cannot observe the difference.
56-
private static final Set<String> COUNT_ONLY_FIELDS = ImmutableSet.of("total", "__typename");
57-
5845
static {
5946
DEFAULT_ENTITIES_INPUT.setQuery(DEFAULT_QUERY);
6047
DEFAULT_ENTITIES_INPUT.setStart(DEFAULT_START);
@@ -79,89 +66,16 @@ public CompletableFuture<SearchResults> get(final DataFetchingEnvironment enviro
7966
? bindArgument(environment.getArgument(INPUT_ARG_NAME), ContainerEntitiesInput.class)
8067
: DEFAULT_ENTITIES_INPUT;
8168

82-
final String query = input.getQuery() != null ? input.getQuery() : DEFAULT_QUERY;
83-
final int start = input.getStart() != null ? input.getStart() : DEFAULT_START;
84-
final int count = input.getCount() != null ? input.getCount() : DEFAULT_COUNT;
85-
86-
// Fast path: every UI call site selects only `total`, so the hits the search would return are
87-
// discarded. Serve those from a batched, request-scoped aggregation instead, so a page of N
88-
// containers costs a fixed number of searches rather than N searches plus N primary-store
89-
// existence checks. See ContainerEntityCountsBatchLoader.
90-
if (canServeFromCounts(environment, query, input.getFilters())) {
91-
return resolveCountFromLoader(environment, urn, start, count);
92-
}
93-
94-
return resolveDirect(context, urn, input, query, start, count);
95-
}
96-
97-
private static boolean canServeFromCounts(
98-
final DataFetchingEnvironment environment,
99-
final String query,
100-
@Nullable final List<FacetFilterInput> filters) {
101-
// The batched loader forces query "*" and applies no facet filters, so anything relying on a
102-
// real query or filters must take the direct path to preserve exact behavior. `count` is not
103-
// consulted: the gate below already establishes that no hits are read, and `total` is
104-
// independent of paging.
105-
if (!DEFAULT_QUERY.equals(query) || (filters != null && !filters.isEmpty())) {
106-
return false;
107-
}
108-
return isCountOnlySelection(environment);
109-
}
110-
111-
/**
112-
* True when the caller reads nothing but the total. Uses the flattened selection set so fragment
113-
* spreads are resolved, and treats an empty selection as ineligible rather than as trivially
114-
* count-only.
115-
*/
116-
private static boolean isCountOnlySelection(final DataFetchingEnvironment environment) {
117-
final DataFetchingFieldSelectionSet selectionSet = environment.getSelectionSet();
118-
if (selectionSet == null) {
119-
return false;
120-
}
121-
final List<SelectedField> fields = selectionSet.getImmediateFields();
122-
return !fields.isEmpty()
123-
&& fields.stream().allMatch(field -> COUNT_ONLY_FIELDS.contains(field.getName()));
124-
}
125-
126-
private CompletableFuture<SearchResults> resolveCountFromLoader(
127-
final DataFetchingEnvironment environment,
128-
final String containerUrn,
129-
final int start,
130-
final int count) {
131-
final DataLoader<String, Long> loader =
132-
environment.getDataLoader(ContainerEntityCountsBatchLoader.LOADER_NAME);
133-
return loader.load(containerUrn).thenApply(total -> toCountOnlyResults(total, start, count));
134-
}
135-
136-
private static SearchResults toCountOnlyResults(
137-
final Long total, final int start, final int count) {
138-
final SearchResults results = new SearchResults();
139-
results.setStart(start);
140-
results.setCount(count);
141-
results.setTotal(total != null ? total.intValue() : 0);
142-
results.setSearchResults(Collections.emptyList());
143-
return results;
144-
}
69+
final String query = input.getQuery() != null ? input.getQuery() : "*";
70+
final int start = input.getStart() != null ? input.getStart() : 0;
71+
final int count = input.getCount() != null ? input.getCount() : 20;
14572

146-
/** The original, unbatched behavior: one search per invocation. */
147-
private CompletableFuture<SearchResults> resolveDirect(
148-
final QueryContext context,
149-
final String urn,
150-
final ContainerEntitiesInput input,
151-
final String query,
152-
final int start,
153-
final int count) {
15473
return GraphQLConcurrencyUtils.supplyAsync(
15574
() -> {
15675
try {
15776

158-
final CriterionArray criteria = new CriterionArray();
15977
final Criterion filterCriterion =
16078
buildCriterion(CONTAINER_FIELD_NAME + ".keyword", Condition.EQUAL, urn);
161-
criteria.add(filterCriterion);
162-
if (input.getFilters() != null) {
163-
input.getFilters().forEach(filter -> criteria.add(criterionFromFilter(filter)));
164-
}
16579

16680
return UrnSearchResultsMapper.map(
16781
context,
@@ -172,7 +86,9 @@ private CompletableFuture<SearchResults> resolveDirect(
17286
new Filter()
17387
.setOr(
17488
new ConjunctiveCriterionArray(
175-
new ConjunctiveCriterion().setAnd(criteria))),
89+
new ConjunctiveCriterion()
90+
.setAnd(
91+
new CriterionArray(ImmutableList.of(filterCriterion))))),
17692
start,
17793
count,
17894
Collections.emptyList()));

0 commit comments

Comments
 (0)