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

Handle inherited variants with different dimensions #10376

Merged
merged 1 commit into from
Aug 23, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -425,7 +425,7 @@ private boolean hasClusterController(HostResource host) {
* physical node. It will not achieve maximum spread over all levels in a multilevel group hierarchy.
*/
// Note: This method cannot be changed to draw different nodes without ensuring that it will draw nodes
// which overlaps with previously drawn nodes as this will prevent rolling upgrade
// which overlaps with previously drawn nodes as that will prevent rolling upgrade
private List<HostResource> drawContentHostsRecursively(int count, StorageGroup group) {
Set<HostResource> hosts = new HashSet<>();
if (group.getNodes().isEmpty()) {
Expand Down
Expand Up @@ -128,21 +128,16 @@ private Builder(HttpRequest parent, com.yahoo.jdisc.http.HttpRequest jdiscReques
}

private void populateProperties() {
if (parent == null) {
return;
}
if (parent == null) return;

properties.putAll(parent.propertyMap());
}

/**
* Add a parameter to the request. Multi-value parameters are not
* supported.
* Add a parameter to the request. Multi-value parameters are not supported.
*
* @param key
* parameter name
* @param value
* parameter value
* @param key parameter name
* @param value parameter value
* @return this Builder instance
*/
public Builder put(String key, String value) {
Expand All @@ -162,8 +157,7 @@ public Builder removeProperty(String parameterName) {
/**
* Set the HTTP method for the new request.
*
* @param method
* the HTTP method to use for the new request
* @param method the HTTP method to use for the new request
* @return this Builder instance
*/
public Builder method(Method method) {
Expand All @@ -174,8 +168,7 @@ public Builder method(Method method) {
/**
* Define the JDisc parent request.
*
* @param request
* a valid JDisc request for the current container
* @param request a valid JDisc request for the current container
* @return this Builder instance
*/
public Builder jdiscRequest(com.yahoo.jdisc.http.HttpRequest request) {
Expand Down Expand Up @@ -214,8 +207,7 @@ public Builder uri(URI uri) {
* instance's JDisc request, but no properties will be propagated into
* the original JDisc request.
*
* @return a new HttpRequest instance reflecting the given request data
* and parameters
* @return a new HttpRequest instance reflecting the given request data and parameters
*/
public HttpRequest createDirectRequest() {
ensureRequestData();
Expand Down
4 changes: 3 additions & 1 deletion container-search/abi-spec.json
Expand Up @@ -4153,6 +4153,7 @@
],
"methods": [
"public void <init>(com.yahoo.statistics.Statistics, com.yahoo.jdisc.Metric, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.config.QueryProfilesConfig, com.yahoo.container.core.ContainerHttpConfig, com.yahoo.search.searchchain.ExecutionFactory)",
"public void <init>(com.yahoo.statistics.Statistics, com.yahoo.jdisc.Metric, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry, com.yahoo.search.searchchain.ExecutionFactory, boolean, java.util.Optional)",
"public void <init>(com.yahoo.container.core.ChainsConfig, com.yahoo.search.config.IndexInfoConfig, com.yahoo.container.QrSearchersConfig, com.yahoo.vespa.configdefinition.SpecialtokensConfig, com.yahoo.statistics.Statistics, com.yahoo.language.Linguistics, com.yahoo.jdisc.Metric, com.yahoo.component.provider.ComponentRegistry, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.config.QueryProfilesConfig, com.yahoo.component.provider.ComponentRegistry, com.yahoo.container.core.ContainerHttpConfig)",
"public final com.yahoo.container.jdisc.HttpResponse handle(com.yahoo.container.jdisc.HttpRequest)",
"public com.yahoo.search.Result searchAndFill(com.yahoo.search.Query, com.yahoo.component.chain.Chain)",
Expand Down Expand Up @@ -7621,7 +7622,8 @@
"public com.yahoo.search.searchchain.Execution newExecution(com.yahoo.component.chain.Chain)",
"public com.yahoo.search.searchchain.SearchChainRegistry searchChainRegistry()",
"public com.yahoo.search.rendering.RendererRegistry rendererRegistry()",
"public void deconstruct()"
"public void deconstruct()",
"public static com.yahoo.search.searchchain.ExecutionFactory empty()"
],
"fields": []
},
Expand Down
Expand Up @@ -98,6 +98,7 @@ public QuotingSearcher(ComponentId id, QrQuotetableConfig config) {
setQuoteTable(new QuoteTable(config));
}

@Override
public Result search(Query query, Execution execution) {
Result result = execution.search(query);
execution.fill(result);
Expand Down
Expand Up @@ -122,23 +122,40 @@ public SearchHandler(Statistics statistics,
QueryProfilesConfig queryProfileConfig,
ContainerHttpConfig containerHttpConfig,
ExecutionFactory executionFactory) {
this(statistics,
metric,
executor,
accessLog,
QueryProfileConfigurer.createFromConfig(queryProfileConfig).compile(),
executionFactory,
queryProfileConfig.enableGroupingSessionCache(),
containerHttpConfig.hostResponseHeaderKey().equals("") ?
Optional.empty() : Optional.of( containerHttpConfig.hostResponseHeaderKey()));
}

public SearchHandler(Statistics statistics,
Metric metric,
Executor executor,
AccessLog accessLog,
CompiledQueryProfileRegistry queryProfileRegistry,
ExecutionFactory executionFactory,
boolean enableGroupingSessionCache,
Optional<String> hostResponseHeaderKey) {
super(executor, accessLog, metric, true);
log.log(LogLevel.DEBUG, "SearchHandler.init " + System.identityHashCode(this));
this.enableGroupingSessionCache = queryProfileConfig.enableGroupingSessionCache();
QueryProfileRegistry queryProfileRegistry = QueryProfileConfigurer.createFromConfig(queryProfileConfig);
this.queryProfileRegistry = queryProfileRegistry.compile();
this.enableGroupingSessionCache = enableGroupingSessionCache;
this.queryProfileRegistry = queryProfileRegistry;
this.executionFactory = executionFactory;

this.maxThreads = examineExecutor(executor);

searchConnections = new Value(SEARCH_CONNECTIONS, statistics,
new Value.Parameters().setLogRaw(true).setLogMax(true)
.setLogMean(true).setLogMin(true)
.setNameExtension(true)
.setCallback(new MeanConnections()));

this.hostResponseHeaderKey = containerHttpConfig.hostResponseHeaderKey().equals("") ?
Optional.empty() : Optional.of( containerHttpConfig.hostResponseHeaderKey());
.setLogMean(true).setLogMin(true)
.setNameExtension(true)
.setCallback(new MeanConnections()));

this.hostResponseHeaderKey = hostResponseHeaderKey;
}

/** @deprecated use the other constructor */
Expand Down
Expand Up @@ -330,21 +330,21 @@ final Object get(CompoundName name, DimensionBinding binding, Properties substit
return node;
}

final Object get(CompoundName name,DimensionBinding dimensionBinding) {
return lookup(name,false,dimensionBinding);
final Object get(CompoundName name, DimensionBinding dimensionBinding) {
return lookup(name, false, dimensionBinding);
}

/**
* Returns the node at the position prescribed by the given name (without doing substitutions) -
* a primitive value, a substitutable string, a query profile, or null if not found.
*/
public final Object lookup(String name, Map<String,String> context) {
public final Object lookup(String name, Map<String, String> context) {
return lookup(new CompoundName(name),true,DimensionBinding.createFrom(getDimensions(),context));
}

/** Sets a value in this or any nested profile using null as context */
public final void set(String name, Object value, QueryProfileRegistry registry) {
set(name,value,(Map<String,String>)null, registry);
set(name, value, (Map<String, String>)null, registry);
}

/**
Expand All @@ -357,16 +357,16 @@ public final void set(String name, Object value, QueryProfileRegistry registry)
* @throws IllegalArgumentException if the given name is illegal given the types of this or any nested query profile
* @throws IllegalStateException if this query profile is frozen
*/
public final void set(CompoundName name,Object value,Map<String,String> context, QueryProfileRegistry registry) {
public final void set(CompoundName name, Object value, Map<String,String> context, QueryProfileRegistry registry) {
set(name, value, DimensionBinding.createFrom(getDimensions(), context), registry);
}

public final void set(String name,Object value,Map<String,String> context, QueryProfileRegistry registry) {
public final void set(String name, Object value, Map<String,String> context, QueryProfileRegistry registry) {
set(new CompoundName(name), value, DimensionBinding.createFrom(getDimensions(), context), registry);
}

public final void set(String name,Object value,String[] dimensionValues, QueryProfileRegistry registry) {
set(name,value,DimensionValues.createFrom(dimensionValues), registry);
public final void set(String name, Object value, String[] dimensionValues, QueryProfileRegistry registry) {
set(name, value, DimensionValues.createFrom(dimensionValues), registry);
}

/**
Expand All @@ -382,7 +382,7 @@ public final void set(String name,Object value,String[] dimensionValues, QueryPr
* @throws IllegalArgumentException if the given name is illegal given the types of this or any nested query profile
* @throws IllegalStateException if this query profile is frozen
*/
public final void set(String name,Object value,DimensionValues dimensionValues, QueryProfileRegistry registry) {
public final void set(String name,Object value, DimensionValues dimensionValues, QueryProfileRegistry registry) {
set(new CompoundName(name), value, DimensionBinding.createFrom(getDimensions(), dimensionValues), registry);
}

Expand Down
Expand Up @@ -38,8 +38,7 @@ public static CompiledQueryProfile compile(QueryProfile in, CompiledQueryProfile
DimensionalMap.Builder<CompoundName, Object> unoverridables = new DimensionalMap.Builder<>();

// Resolve values for each existing variant and combine into a single data structure
Set<DimensionBindingForPath> variants = new HashSet<>();
collectVariants(CompoundName.empty, in, DimensionBinding.nullBinding, variants);
Set<DimensionBindingForPath> variants = collectVariants(CompoundName.empty, in, DimensionBinding.nullBinding);
variants.add(new DimensionBindingForPath(DimensionBinding.nullBinding, CompoundName.empty)); // if this contains no variants
if (log.isLoggable(Level.FINE))
log.fine("Compiling " + in.toString() + " having " + variants.size() + " variants");
Expand Down Expand Up @@ -67,41 +66,68 @@ public static CompiledQueryProfile compile(QueryProfile in, CompiledQueryProfile
*
* @param profile the profile we are collecting the variants of
* @param currentVariant the variant we must have to arrive at this point in the query profile graph
* @param allVariants the set of all variants accumulated so far
*/
private static void collectVariants(CompoundName path, QueryProfile profile, DimensionBinding currentVariant, Set<DimensionBindingForPath> allVariants) {
private static Set<DimensionBindingForPath> collectVariants(CompoundName path, QueryProfile profile, DimensionBinding currentVariant) {
Set<DimensionBindingForPath> variants = new HashSet<>();
variants.addAll(collectVariantsFromValues(path, profile.getContent(), currentVariant));
variants.addAll(collectVariantsInThis(path, profile, currentVariant));
if (profile instanceof BackedOverridableQueryProfile)
variants.addAll(collectVariantsInThis(path, ((BackedOverridableQueryProfile) profile).getBacking(), currentVariant));

Set<DimensionBindingForPath> parentVariants = new HashSet<>();
for (QueryProfile inheritedProfile : profile.inherited())
collectVariants(path, inheritedProfile, currentVariant, allVariants);
parentVariants = collectVariants(path, inheritedProfile, currentVariant);
variants.addAll(parentVariants);
variants.addAll(combined(variants, parentVariants)); // parents and children may have different variant dimensions
return variants;
}

collectVariantsFromValues(path, profile.getContent(), currentVariant, allVariants);
/** Generates a set of all the (legal) combinations of the variants in the given sets */
private static Set<DimensionBindingForPath> combined(Set<DimensionBindingForPath> v1s,
Set<DimensionBindingForPath> v2s) {
Set<DimensionBindingForPath> combinedVariants = new HashSet<>();
for (DimensionBindingForPath v1 : v1s) {
for (DimensionBindingForPath v2 : v2s) {
if ( ! v1.path().equals(v2.path())) continue;

collectVariantsInThis(path, profile, currentVariant, allVariants);
if (profile instanceof BackedOverridableQueryProfile)
collectVariantsInThis(path, ((BackedOverridableQueryProfile) profile).getBacking(), currentVariant, allVariants);
DimensionBinding combined = v1.binding().combineWith(v2.binding);
if ( combined.isInvalid() ) continue;

combinedVariants.add(new DimensionBindingForPath(combined, v1.path()));
}
}
return combinedVariants;
}

private static void collectVariantsInThis(CompoundName path, QueryProfile profile, DimensionBinding currentVariant, Set<DimensionBindingForPath> allVariants) {
private static Set<DimensionBindingForPath> collectVariantsInThis(CompoundName path, QueryProfile profile, DimensionBinding currentVariant) {
QueryProfileVariants profileVariants = profile.getVariants();
Set<DimensionBindingForPath> variants = new HashSet<>();
if (profileVariants != null) {
for (QueryProfileVariant variant : profile.getVariants().getVariants()) {
DimensionBinding combinedVariant =
DimensionBinding.createFrom(profile.getDimensions(), variant.getDimensionValues()).combineWith(currentVariant);
if (combinedVariant.isInvalid()) continue; // values at this point in the graph are unreachable
collectVariantsFromValues(path, variant.values(), combinedVariant, allVariants);

variants.addAll(collectVariantsFromValues(path, variant.values(), combinedVariant));
for (QueryProfile variantInheritedProfile : variant.inherited())
collectVariants(path, variantInheritedProfile, combinedVariant, allVariants);
variants.addAll(collectVariants(path, variantInheritedProfile, combinedVariant));
}
}
return variants;
}

private static void collectVariantsFromValues(CompoundName path, Map<String, Object> values, DimensionBinding currentVariant, Set<DimensionBindingForPath> allVariants) {
private static Set<DimensionBindingForPath> collectVariantsFromValues(CompoundName path,
Map<String, Object> values,
DimensionBinding currentVariant) {
Set<DimensionBindingForPath> variants = new HashSet<>();
if ( ! values.isEmpty())
allVariants.add(new DimensionBindingForPath(currentVariant, path)); // there are actual values for this variant
variants.add(new DimensionBindingForPath(currentVariant, path)); // there are actual values for this variant

for (Map.Entry<String, Object> entry : values.entrySet()) {
if (entry.getValue() instanceof QueryProfile)
collectVariants(path.append(entry.getKey()), (QueryProfile)entry.getValue(), currentVariant, allVariants);
variants.addAll(collectVariants(path.append(entry.getKey()), (QueryProfile)entry.getValue(), currentVariant));
}
return variants;
}

private static class DimensionBindingForPath {
Expand Down
Expand Up @@ -10,6 +10,7 @@
import com.yahoo.container.QrSearchersConfig;
import com.yahoo.container.core.ChainsConfig;
import com.yahoo.language.Linguistics;
import com.yahoo.language.simple.SimpleLinguistics;
import com.yahoo.prelude.IndexFacts;
import com.yahoo.prelude.IndexModel;
import com.yahoo.prelude.query.parser.SpecialTokenRegistry;
Expand Down Expand Up @@ -76,4 +77,14 @@ public void deconstruct() {
rendererRegistry.deconstruct();
}

public static ExecutionFactory empty() {
return new ExecutionFactory(new ChainsConfig.Builder().build(),
new IndexInfoConfig.Builder().build(),
new QrSearchersConfig.Builder().build(),
new ComponentRegistry<>(),
new SpecialtokensConfig.Builder().build(),
new SimpleLinguistics(),
new ComponentRegistry<>());
}

}
Expand Up @@ -3,6 +3,7 @@

import com.yahoo.jdisc.http.HttpRequest.Method;
import com.yahoo.container.jdisc.HttpRequest;
import com.yahoo.processing.execution.Execution;
import com.yahoo.processing.request.CompoundName;
import com.yahoo.yolean.Exceptions;
import com.yahoo.search.Query;
Expand All @@ -18,7 +19,12 @@

import java.util.HashMap;
import java.util.Map;
import static org.junit.Assert.*;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

/**
* @author bratseth
Expand All @@ -31,7 +37,7 @@ public void testValid() {
new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/validxml");
CompiledQueryProfileRegistry cRegistry= registry.compile();

QueryProfileType rootType=registry.getType("rootType");
QueryProfileType rootType = registry.getType("rootType");
assertEquals(1,rootType.inherited().size());
assertEquals("native",rootType.inherited().get(0).getId().getName());
assertTrue(rootType.isStrict());
Expand Down