Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debug(ci): debug ci failure in metadata-io test #11519

Closed
wants to merge 20 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ private static void addFilterToQueryBuilder(
criterion.getValues())));
orQuery.should(andQuery);
}
if (!orQuery.should().isEmpty()) {
orQuery.minimumShouldMatch(1);
}
rootQuery.filter(orQuery);
}

Expand Down Expand Up @@ -177,21 +180,26 @@ private SearchResponse executeGroupByLineageSearchQuery(
// directions for lineage
// set up filters for each relationship type in the correct direction to limit buckets
BoolQueryBuilder sourceFilterQuery = QueryBuilders.boolQuery();
sourceFilterQuery.minimumShouldMatch(1);

validEdges.stream()
.filter(pair -> RelationshipDirection.OUTGOING.equals(pair.getValue().getDirection()))
.forEach(
pair ->
sourceFilterQuery.should(
getAggregationFilter(pair, RelationshipDirection.OUTGOING)));
if (!sourceFilterQuery.should().isEmpty()) {
sourceFilterQuery.minimumShouldMatch(1);
}

BoolQueryBuilder destFilterQuery = QueryBuilders.boolQuery();
destFilterQuery.minimumShouldMatch(1);
validEdges.stream()
.filter(pair -> RelationshipDirection.INCOMING.equals(pair.getValue().getDirection()))
.forEach(
pair ->
destFilterQuery.should(getAggregationFilter(pair, RelationshipDirection.INCOMING)));
if (!destFilterQuery.should().isEmpty()) {
destFilterQuery.minimumShouldMatch(1);
}

FilterAggregationBuilder sourceRelationshipTypeFilters =
AggregationBuilders.filter(FILTER_BY_SOURCE_RELATIONSHIP, sourceFilterQuery);
Expand Down Expand Up @@ -347,6 +355,9 @@ public static BoolQueryBuilder buildQuery(
relationshipType ->
relationshipQuery.should(
QueryBuilders.termQuery(RELATIONSHIP_TYPE, relationshipType)));
if (!relationshipQuery.should().isEmpty()) {
relationshipQuery.minimumShouldMatch(1);
}
finalQuery.filter(relationshipQuery);
}

Expand Down Expand Up @@ -697,6 +708,9 @@ public static QueryBuilder getLineageQuery(
urns, edgesPerEntityType.get(entityType), graphFilters));
}
});
if (!entityTypeQueries.should().isEmpty()) {
entityTypeQueries.minimumShouldMatch(1);
}

BoolQueryBuilder finalQuery = QueryBuilders.boolQuery();

Expand Down Expand Up @@ -741,6 +755,10 @@ static QueryBuilder getLineageQueryForEntityType(
query.should(getIncomingEdgeQuery(urns, incomingEdges, graphFilters));
}

if (!query.should().isEmpty()) {
query.minimumShouldMatch(1);
}

return query;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ public static QueryBuilder getUrnStatusQuery(
if (removed) {
finalQuery.filter(QueryBuilders.termQuery(statusField, removed.toString()));
} else {
finalQuery.minimumShouldMatch(1);
finalQuery.should(QueryBuilders.termQuery(statusField, removed.toString()));
finalQuery.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(statusField)));
}

if (!finalQuery.should().isEmpty()) {
finalQuery.minimumShouldMatch(1);
}

return finalQuery;
}

Expand Down Expand Up @@ -102,7 +105,7 @@ public static QueryBuilder getEdgeTimeFilterQuery(
* 2. The createdOn and updatedOn window does not exist on the edge at all (support legacy cases)
* 3. Special lineage case: The edge is marked as a "manual" edge, meaning that the time filters should NOT be applied.
*/
BoolQueryBuilder timeFilterQuery = QueryBuilders.boolQuery();
BoolQueryBuilder timeFilterQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);
timeFilterQuery.should(buildTimeWindowFilter(startTimeMillis, endTimeMillis));
timeFilterQuery.should(buildTimestampsMissingFilter());
timeFilterQuery.should(buildManualLineageFilter());
Expand Down Expand Up @@ -158,7 +161,7 @@ public static QueryBuilder getEdgeTimeFilterQuery(
*/
private static QueryBuilder buildTimeWindowFilter(
final long startTimeMillis, final long endTimeMillis) {
final BoolQueryBuilder timeWindowQuery = QueryBuilders.boolQuery();
final BoolQueryBuilder timeWindowQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

/*
* To perform comparison:
Expand Down Expand Up @@ -198,7 +201,7 @@ private static QueryBuilder buildTimestampsMissingFilter() {

private static QueryBuilder buildNotExistsFilter(String fieldName) {
// This filter returns 'true' if the field DOES NOT EXIST or it exists but is equal to 0.
final BoolQueryBuilder notExistsFilter = QueryBuilders.boolQuery();
final BoolQueryBuilder notExistsFilter = QueryBuilders.boolQuery().minimumShouldMatch(1);
notExistsFilter.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(fieldName)));
notExistsFilter.should(QueryBuilders.boolQuery().must(QueryBuilders.termQuery(fieldName, 0L)));
return notExistsFilter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.linkedin.metadata.search.utils.SearchUtils.applyDefaultSearchFlags;

import com.google.common.annotations.VisibleForTesting;
import com.linkedin.common.urn.Urn;
import com.linkedin.metadata.browse.BrowseResult;
import com.linkedin.metadata.browse.BrowseResultV2;
Expand Down Expand Up @@ -30,6 +31,7 @@
import java.util.Optional;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.opensearch.action.explain.ExplainResponse;
Expand All @@ -51,7 +53,7 @@ public class ElasticSearchService implements EntitySearchService, ElasticSearchI

private static final int MAX_RUN_IDS_INDEXED = 25; // Save the previous 25 run ids in the index.
private final EntityIndexBuilders indexBuilders;
private final ESSearchDAO esSearchDAO;
@VisibleForTesting @Getter private final ESSearchDAO esSearchDAO;
private final ESBrowseDAO esBrowseDAO;
private final ESWriteDAO esWriteDAO;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

import com.codahale.metrics.Timer;
import com.datahub.util.exception.ESQueryException;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.annotations.VisibleForTesting;
import com.linkedin.data.template.LongMap;
import com.linkedin.metadata.config.search.SearchConfiguration;
Expand Down Expand Up @@ -78,6 +80,24 @@ public class ESSearchDAO {
@Nonnull private final SearchConfiguration searchConfiguration;
@Nullable private final CustomSearchConfiguration customSearchConfiguration;
@Nonnull private final QueryFilterRewriteChain queryFilterRewriteChain;
private final boolean testLoggingEnabled;

public ESSearchDAO(
RestHighLevelClient client,
boolean pointInTimeCreationEnabled,
String elasticSearchImplementation,
@Nonnull SearchConfiguration searchConfiguration,
@Nullable CustomSearchConfiguration customSearchConfiguration,
@Nonnull QueryFilterRewriteChain queryFilterRewriteChain) {
this(
client,
pointInTimeCreationEnabled,
elasticSearchImplementation,
searchConfiguration,
customSearchConfiguration,
queryFilterRewriteChain,
false);
}

public long docCount(@Nonnull OperationContext opContext, @Nonnull String entityName) {
return docCount(opContext, entityName, null);
Expand Down Expand Up @@ -279,6 +299,11 @@ public SearchResult search(
searchRequest.indices(
entityNames.stream().map(indexConvention::getEntityIndexName).toArray(String[]::new));
searchRequestTimer.stop();

if (testLoggingEnabled) {
testLog(opContext.getObjectMapper(), searchRequest);
}

// Step 2: execute the query and extract results, validated against document model as well
return executeAndExtract(opContext, entitySpecs, searchRequest, transformedFilters, from, size);
}
Expand Down Expand Up @@ -478,6 +503,11 @@ public ScrollResult scroll(
}

scrollRequestTimer.stop();

if (testLoggingEnabled) {
testLog(opContext.getObjectMapper(), searchRequest);
}

return executeAndExtract(
opContext, entitySpecs, searchRequest, transformedFilters, keepAlive, size);
}
Expand Down Expand Up @@ -605,4 +635,21 @@ public ExplainResponse explain(
throw new IllegalStateException("Failed to explain query:", e);
}
}

private void testLog(ObjectMapper mapper, SearchRequest searchRequest) {
try {
log.warn("SearchRequest(custom): {}", mapper.writeValueAsString(customSearchConfiguration));
final String[] indices = searchRequest.indices();
log.warn(
String.format(
"SearchRequest(indices): %s",
mapper.writerWithDefaultPrettyPrinter().writeValueAsString(indices)));
log.warn(
String.format(
"SearchRequest(query): %s",
mapper.writeValueAsString(mapper.readTree(searchRequest.source().toString()))));
} catch (JsonProcessingException e) {
log.warn("Error writing test log");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,12 @@ private BoolQueryBuilder handleNestedFilters(
mustNotQueryBuilders.forEach(expandedQueryBuilder::mustNot);
expandedQueryBuilder.queryName(boolQueryBuilder.queryName());
expandedQueryBuilder.adjustPureNegative(boolQueryBuilder.adjustPureNegative());
expandedQueryBuilder.minimumShouldMatch(boolQueryBuilder.minimumShouldMatch());
expandedQueryBuilder.boost(boolQueryBuilder.boost());

if (!expandedQueryBuilder.should().isEmpty()) {
expandedQueryBuilder.minimumShouldMatch(1);
}

return expandedQueryBuilder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ public SearchRequest getSearchRequest(
QueryConfiguration customQueryConfig =
customizedQueryHandler.lookupQueryConfig(input).orElse(null);

BoolQueryBuilder baseQuery = QueryBuilders.boolQuery();
baseQuery.minimumShouldMatch(1);
BoolQueryBuilder baseQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

// Initial query with input filters
BoolQueryBuilder filterQuery =
Expand Down Expand Up @@ -176,12 +175,15 @@ public BoolQueryBuilder getQuery(
BoolQueryBuilder finalQuery =
Optional.ofNullable(customAutocompleteConfig)
.flatMap(cac -> CustomizedQueryHandler.boolQueryBuilder(objectMapper, cac, query))
.orElse(QueryBuilders.boolQuery())
.minimumShouldMatch(1);
.orElse(QueryBuilders.boolQuery());

getAutocompleteQuery(customAutocompleteConfig, autocompleteFields, query)
.ifPresent(finalQuery::should);

if (!finalQuery.should().isEmpty()) {
finalQuery.minimumShouldMatch(1);
}

return finalQuery;
}

Expand All @@ -200,8 +202,7 @@ private Optional<QueryBuilder> getAutocompleteQuery(

private static BoolQueryBuilder defaultQuery(
List<String> autocompleteFields, @Nonnull String query) {
BoolQueryBuilder finalQuery = QueryBuilders.boolQuery();
finalQuery.minimumShouldMatch(1);
BoolQueryBuilder finalQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

// Search for exact matches with higher boost and ngram matches
MultiMatchQueryBuilder autocompleteQueryBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ private QueryBuilder buildInternalQuery(
cqc ->
CustomizedQueryHandler.boolQueryBuilder(
opContext.getObjectMapper(), cqc, sanitizedQuery))
.orElse(QueryBuilders.boolQuery())
.minimumShouldMatch(1);
.orElse(QueryBuilders.boolQuery());

if (fulltext && !query.startsWith(STRUCTURED_QUERY_PREFIX)) {
getSimpleQuery(opContext.getEntityRegistry(), customQueryConfig, entitySpecs, sanitizedQuery)
Expand Down Expand Up @@ -135,6 +134,10 @@ private QueryBuilder buildInternalQuery(
}
}

if (!finalQuery.should().isEmpty()) {
finalQuery.minimumShouldMatch(1);
}

return finalQuery;
}

Expand Down Expand Up @@ -368,6 +371,10 @@ private Optional<QueryBuilder> getSimpleQuery(
simplePerField.should(simpleBuilder);
});

if (!simplePerField.should().isEmpty()) {
simplePerField.minimumShouldMatch(1);
}

result = Optional.of(simplePerField);
}

Expand Down Expand Up @@ -454,7 +461,9 @@ private Optional<QueryBuilder> getPrefixAndExactMatchQuery(
}
});

return finalQuery.should().size() > 0 ? Optional.of(finalQuery) : Optional.empty();
return finalQuery.should().size() > 0
? Optional.of(finalQuery.minimumShouldMatch(1))
: Optional.empty();
}

private Optional<QueryBuilder> getStructuredQuery(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.stream.Stream;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.collections.CollectionUtils;
import org.opensearch.action.search.SearchRequest;
Expand All @@ -80,7 +81,7 @@ public class SearchRequestHandler {
private static final Map<List<EntitySpec>, SearchRequestHandler> REQUEST_HANDLER_BY_ENTITY_NAME =
new ConcurrentHashMap<>();
private final List<EntitySpec> entitySpecs;
private final Set<String> defaultQueryFieldNames;
@Getter private final Set<String> defaultQueryFieldNames;
@Nonnull private final HighlightBuilder highlights;

private final SearchConfiguration configs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ public static BoolQueryBuilder buildFilterQuery(
searchableFieldTypes,
opContext,
queryFilterRewriteChain)));
// The default is not always 1 (ensure consistent default)
finalQueryBuilder.minimumShouldMatch(1);
} else if (filter.getCriteria() != null) {
// Otherwise, build boolean query from the deprecated "criteria" field.
log.warn("Received query Filter with a deprecated field 'criteria'. Use 'or' instead.");
Expand All @@ -187,7 +185,8 @@ public static BoolQueryBuilder buildFilterQuery(
}
});
finalQueryBuilder.should(andQueryBuilder);
// The default is not always 1 (ensure consistent default)
}
if (!finalQueryBuilder.should().isEmpty()) {
finalQueryBuilder.minimumShouldMatch(1);
}
return finalQueryBuilder;
Expand Down Expand Up @@ -533,7 +532,7 @@ private static QueryBuilder getQueryBuilderFromCriterionForFieldToExpand(
final Map<String, Set<SearchableAnnotation.FieldType>> searchableFieldTypes,
@Nonnull OperationContext opContext,
@Nonnull QueryFilterRewriteChain queryFilterRewriteChain) {
final BoolQueryBuilder orQueryBuilder = new BoolQueryBuilder();
final BoolQueryBuilder orQueryBuilder = new BoolQueryBuilder().minimumShouldMatch(1);
for (String field : fields) {
orQueryBuilder.should(
getQueryBuilderFromCriterionForSingleField(
Expand Down Expand Up @@ -619,7 +618,7 @@ private static QueryBuilder buildWildcardQueryWithMultipleValues(
@Nullable String queryName,
@Nonnull AspectRetriever aspectRetriever,
String wildcardPattern) {
BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

for (String value : criterion.getValues()) {
boolQuery.should(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import static com.linkedin.metadata.search.utils.QueryUtils.*;
import static org.mockito.Mockito.*;
import static org.testng.AssertJUnit.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;

import com.datahub.util.RecordUtils;
import com.google.common.collect.ImmutableList;
Expand Down
Loading
Loading