diff options
36 files changed, 383 insertions, 336 deletions
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java index 35db7b5fef3..8920acd431b 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java @@ -16,56 +16,34 @@ import java.util.jar.Manifest; import java.util.stream.Collectors; /** - * "Public package" in this context means a package that is either exported or declared as "Global-Package" - * in a bundle's manifest. + * Static utilities for analyzing jar files. * * @author Tony Vaagenes * @author ollivir + * @author gjoranv */ public class AnalyzeBundle { - public static class PublicPackages { - public final List<Export> exports; - public final List<String> globals; - PublicPackages(List<Export> exports, List<String> globals) { - this.exports = exports; - this.globals = globals; - } - - public Set<String> exportedPackageNames() { - return exports.stream() - .map(Export::getPackageNames) - .flatMap(Collection::stream) - .collect(Collectors.toSet()); - } - - } - - public static PublicPackages publicPackagesAggregated(Collection<File> jarFiles) { + public static List<Export> exportedPackagesAggregated(Collection<File> jarFiles) { List<Export> exports = new ArrayList<>(); - List<String> globals = new ArrayList<>(); for (File jarFile : jarFiles) { - PublicPackages pp = publicPackages(jarFile); - exports.addAll(pp.exports); - globals.addAll(pp.globals); - - // TODO: remove and clean up everything related to global packages. - if (! pp.globals.isEmpty()) throw new RuntimeException("Found global packages in bundle " + jarFile.getAbsolutePath()); + var exported = exportedPackages(jarFile); + exports.addAll(exported); } - return new PublicPackages(exports, globals); + return exports; } - static PublicPackages publicPackages(File jarFile) { + static List<Export> exportedPackages(File jarFile) { try { Optional<Manifest> jarManifest = JarFiles.getManifest(jarFile); if (jarManifest.isPresent()) { Manifest manifest = jarManifest.get(); if (isOsgiManifest(manifest)) { - return new PublicPackages(parseExports(manifest), parseGlobals(manifest)); + return parseExports(manifest); } } - return new PublicPackages(Collections.emptyList(), Collections.emptyList()); + return Collections.emptyList(); } catch (Exception e) { throw new RuntimeException(String.format("Invalid manifest in bundle '%s'", jarFile.getPath()), e); } @@ -75,24 +53,10 @@ public class AnalyzeBundle { return JarFiles.getManifest(jarFile).flatMap(AnalyzeBundle::getBundleSymbolicName); } - private static List<Export> parseExportsFromAttribute(Manifest manifest, String attributeName) { - return getMainAttributeValue(manifest, attributeName).map(ExportPackageParser::parseExports).orElseGet(() -> new ArrayList<>()); - } - private static List<Export> parseExports(Manifest jarManifest) { - return parseExportsFromAttribute(jarManifest, "Export-Package"); - } - - private static List<String> parseGlobals(Manifest manifest) { - List<Export> globals = parseExportsFromAttribute(manifest, "Global-Package"); - - for (Export export : globals) { - if (export.getParameters().isEmpty() == false) { - throw new RuntimeException("Parameters not valid for Global-Package."); - } - } - - return globals.stream().flatMap(g -> g.getPackageNames().stream()).collect(Collectors.toList()); + return getMainAttributeValue(jarManifest, "Export-Package") + .map(ExportPackageParser::parseExports) + .orElseGet(ArrayList::new); } private static Optional<String> getMainAttributeValue(Manifest manifest, String attributeName) { diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java index 73fdcf1c471..41420b38360 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java @@ -2,7 +2,6 @@ package com.yahoo.container.plugin.mojo; import com.google.common.collect.Sets; -import com.yahoo.container.plugin.bundle.AnalyzeBundle; import com.yahoo.container.plugin.classanalysis.Analyze; import com.yahoo.container.plugin.classanalysis.ClassFileMetaData; import com.yahoo.container.plugin.classanalysis.ExportPackageAnnotation; @@ -27,7 +26,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Enumeration; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -39,7 +37,7 @@ import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.container.plugin.bundle.AnalyzeBundle.publicPackagesAggregated; +import static com.yahoo.container.plugin.bundle.AnalyzeBundle.exportedPackagesAggregated; import static com.yahoo.container.plugin.osgi.ExportPackages.exportsByPackageName; import static com.yahoo.container.plugin.osgi.ImportPackages.calculateImports; import static com.yahoo.container.plugin.util.Files.allDescendantFiles; @@ -98,12 +96,11 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { Artifacts.ArtifactSet artifactSet = Artifacts.getArtifacts(project); warnOnUnsupportedArtifacts(artifactSet.getNonJarArtifacts()); - // Packages from Export-Package and Global-Package headers in provided scoped jars - AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars = publicPackagesAggregated( + List<Export> exportedPackagesFromProvidedJars = exportedPackagesAggregated( artifactSet.getJarArtifactsProvided().stream().map(Artifact::getFile).collect(Collectors.toList())); // Packages from Export-Package headers in provided scoped jars - Set<String> exportedPackagesFromProvidedDeps = publicPackagesFromProvidedJars.exportedPackageNames(); + Set<String> exportedPackagesFromProvidedDeps = ExportPackages.packageNames(exportedPackagesFromProvidedJars); // Packaged defined in this project's code PackageTally projectPackages = getProjectClassesTally(); @@ -114,8 +111,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { // The union of packages in the project and compile scoped jars PackageTally includedPackages = projectPackages.combine(compileJarsPackages); - warnIfPackagesDefinedOverlapsGlobalPackages(includedPackages.definedPackages(), publicPackagesFromProvidedJars.globals); - logDebugPackageSets(publicPackagesFromProvidedJars, includedPackages); + logDebugPackageSets(exportedPackagesFromProvidedJars, includedPackages); if (hasJdiscCoreProvided(artifactSet.getJarArtifactsProvided())) { // jdisc_core being provided guarantees that log output does not contain its exported packages @@ -129,7 +125,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { Map<String, Import> calculatedImports = calculateImports(includedPackages.referencedPackages(), includedPackages.definedPackages(), - exportsByPackageName(publicPackagesFromProvidedJars.exports)); + exportsByPackageName(exportedPackagesFromProvidedJars)); Map<String, Optional<String>> manualImports = emptyToNone(importPackage).map(GenerateOsgiManifestMojo::getManualImports) .orElseGet(HashMap::new); @@ -144,11 +140,11 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { } } - private void logDebugPackageSets(AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars, PackageTally includedPackages) { + private void logDebugPackageSets(List<Export> exportedPackagesFromProvidedJars, PackageTally includedPackages) { if (getLog().isDebugEnabled()) { getLog().debug("Referenced packages = " + includedPackages.referencedPackages()); getLog().debug("Defined packages = " + includedPackages.definedPackages()); - getLog().debug("Exported packages of dependencies = " + publicPackagesFromProvidedJars.exports.stream() + getLog().debug("Exported packages of dependencies = " + exportedPackagesFromProvidedJars.stream() .map(e -> "(" + e.getPackageNames().toString() + ", " + e.version().orElse("")).collect(Collectors.joining(", "))); } } @@ -201,15 +197,6 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { } } - private static void warnIfPackagesDefinedOverlapsGlobalPackages(Set<String> internalPackages, List<String> globalPackages) - throws MojoExecutionException { - Set<String> overlap = Sets.intersection(internalPackages, new HashSet<>(globalPackages)); - if (! overlap.isEmpty()) { - throw new MojoExecutionException( - "The following packages are both global and included in the bundle:\n " + String.join("\n ", overlap)); - } - } - private Collection<String> osgiExportPackages(Map<String, ExportPackageAnnotation> exportedPackages) { return exportedPackages.entrySet().stream().map(entry -> entry.getKey() + ";version=" + entry.getValue().osgiVersion()) .collect(Collectors.toList()); diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java index 253e0727050..fd2d098d74a 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java @@ -6,6 +6,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; /** * @author Tony Vaagenes @@ -37,6 +39,7 @@ public class ExportPackages { public List<Parameter> getParameters() { return parameters; } + } public static class Parameter { @@ -57,6 +60,13 @@ public class ExportPackages { } } + public static Set<String> packageNames(Collection<Export> exports) { + return exports.stream() + .map(Export::getPackageNames) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + } + public static Map<String, Export> exportsByPackageName(Collection<Export> exports) { Map<String, Export> ret = new HashMap<>(); for (Export export : exports) { diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java index a09604b842b..133cca164f8 100644 --- a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java +++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java @@ -1,8 +1,8 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.plugin.bundle; -import com.yahoo.container.plugin.bundle.AnalyzeBundle.PublicPackages; import com.yahoo.container.plugin.osgi.ExportPackages; +import com.yahoo.container.plugin.osgi.ExportPackages.Export; import org.hamcrest.Description; import org.hamcrest.TypeSafeMatcher; import org.junit.Rule; @@ -14,7 +14,6 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import static com.yahoo.container.plugin.classanalysis.TestUtilities.throwableMessage; import static org.hamcrest.CoreMatchers.containsString; @@ -29,19 +28,16 @@ import static org.junit.Assert.assertThat; * @author ollivir */ public class AnalyzeBundleTest { - private final List<ExportPackages.Export> exports; - private final Set<String> exportedPackageNames; - private final Map<String, ExportPackages.Export> exportsByPackageName; + private final List<Export> exports; + private final Map<String, Export> exportsByPackageName; File jarDir = new File("src/test/resources/jar"); public AnalyzeBundleTest() { File notOsgi = new File(jarDir, "notAOsgiBundle.jar"); File simple = new File(jarDir, "simple1.jar"); - PublicPackages pp = AnalyzeBundle.publicPackagesAggregated(Arrays.asList(notOsgi, simple)); - this.exports = pp.exports; - this.exportedPackageNames = pp.exportedPackageNames(); - this.exportsByPackageName = ExportPackages.exportsByPackageName(exports); + exports = AnalyzeBundle.exportedPackagesAggregated(Arrays.asList(notOsgi, simple)); + exportsByPackageName = ExportPackages.exportsByPackageName(this.exports); } private File jarFile(String name) { @@ -61,7 +57,7 @@ public class AnalyzeBundleTest { @Test public void exported_class_names_can_be_retrieved() { - assertThat(exportedPackageNames, is(new HashSet<>(exports.get(0).getPackageNames()))); + assertThat(ExportPackages.packageNames(exports), is(new HashSet<>(exports.get(0).getPackageNames()))); } @Rule @@ -75,7 +71,7 @@ public class AnalyzeBundleTest { exception.expectMessage(matchesPattern("Invalid manifest in bundle '.*errorExport.jar'")); exception.expectCause(throwableMessage(startsWith("Failed parsing Export-Package"))); - AnalyzeBundle.publicPackages(jarFile("errorExport.jar")); + AnalyzeBundle.exportedPackages(jarFile("errorExport.jar")); } private TypeSafeMatcher<String> matchesPattern(String pattern) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java index f8b48da291d..e618326eff5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java @@ -553,9 +553,10 @@ public class ConvertedModel { if (node instanceof ReferenceNode) { ReferenceNode referenceNode = (ReferenceNode)node; if (referenceNode.getOutput() == null) { // function references cannot specify outputs - names.add(referenceNode.getName()); - if (model.functions().containsKey(referenceNode.getName())) { - addFunctionNamesIn(RankingExpression.from(model.functions().get(referenceNode.getName())).getRoot(), names, model); + if (names.add(referenceNode.getName())) { + if (model.functions().containsKey(referenceNode.getName())) { + addFunctionNamesIn(RankingExpression.from(model.functions().get(referenceNode.getName())).getRoot(), names, model); + } } } } diff --git a/configd/src/apps/sentinel/service.cpp b/configd/src/apps/sentinel/service.cpp index 9f4196a2457..a502ce50f71 100644 --- a/configd/src/apps/sentinel/service.cpp +++ b/configd/src/apps/sentinel/service.cpp @@ -73,7 +73,6 @@ Service::reconfigure(const SentinelConfig::Service& config) _config = new SentinelConfig::Service(config); if ((_state == READY) || (_state == FINISHED) || (_state == RESTARTING)) { - resetRestartPenalty(); if (_isAutomatic) { LOG(debug, "%s: Restarting due to new config", name().c_str()); start(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionStateWatcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionStateWatcher.java deleted file mode 100644 index acbb1dc81ce..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionStateWatcher.java +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.session; - -import java.util.logging.Level; -import com.yahoo.text.Utf8; -import com.yahoo.vespa.curator.Curator; -import org.apache.curator.framework.recipes.cache.ChildData; - -import java.util.concurrent.Executor; -import java.util.logging.Logger; - -/** - * Watches one particular local session (/config/v2/tenants/<tenantName>/sessions/<n>/sessionState in ZooKeeper) - * to pick up when an application is deleted (the delete might be done on any config server in the cluster) - * - * @author Harald Musum - */ -public class LocalSessionStateWatcher { - - private static final Logger log = Logger.getLogger(LocalSessionStateWatcher.class.getName()); - - private final Curator.FileCache fileCache; - private final LocalSession session; - private final SessionRepository sessionRepository; - private final Executor zkWatcherExecutor; - - LocalSessionStateWatcher(Curator.FileCache fileCache, LocalSession session, - SessionRepository sessionRepository, Executor zkWatcherExecutor) { - this.fileCache = fileCache; - this.session = session; - this.sessionRepository = sessionRepository; - this.zkWatcherExecutor = zkWatcherExecutor; - this.fileCache.start(); - this.fileCache.addListener(this::nodeChanged); - } - - // Will delete session if it exists in local session repo - private void sessionChanged(Session.Status status) { - long sessionId = session.getSessionId(); - log.log(status == Session.Status.DELETE ? Level.INFO : Level.FINE, - session.logPre() + "Session change: Local session " + sessionId + " changed status to " + status); - - if (status.equals(Session.Status.DELETE) && sessionRepository.getLocalSession(sessionId) != null) { - log.log(Level.FINE, session.logPre() + "Deleting session " + sessionId); - sessionRepository.deleteLocalSession(session); - } - } - - public long getSessionId() { - return session.getSessionId(); - } - - public void close() { - try { - fileCache.close(); - } catch (Exception e) { - log.log(Level.WARNING, "Exception when closing watcher", e); - } - } - - public void nodeChanged() { - zkWatcherExecutor.execute(() -> { - try { - ChildData node = fileCache.getCurrentData(); - if (node != null) { - sessionChanged(Session.Status.parse(Utf8.toString(node.getData()))); - } - } catch (Exception e) { - log.log(Level.WARNING, session.logPre() + "Error handling session changed for session " + getSessionId(), e); - } - }); - } - -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionStateWatcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionStateWatcher.java deleted file mode 100644 index 68345d3862d..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionStateWatcher.java +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.session; - -import java.util.logging.Level; -import com.yahoo.text.Utf8; -import com.yahoo.vespa.config.server.ReloadHandler; -import com.yahoo.vespa.config.server.monitoring.MetricUpdater; -import com.yahoo.vespa.curator.Curator; -import org.apache.curator.framework.recipes.cache.ChildData; - -import java.util.concurrent.Executor; -import java.util.logging.Logger; - -/** - * Watches one particular session (/config/v2/tenants/<tenantName>/sessions/<n>/sessionState in ZooKeeper) - * The session must be in the session repo. - * - * @author Vegard Havdal - */ -public class RemoteSessionStateWatcher { - - private static final Logger log = Logger.getLogger(RemoteSessionStateWatcher.class.getName()); - - private final Curator.FileCache fileCache; - private final ReloadHandler reloadHandler; - private final RemoteSession session; - private final MetricUpdater metrics; - private final Executor zkWatcherExecutor; - - RemoteSessionStateWatcher(Curator.FileCache fileCache, - ReloadHandler reloadHandler, - RemoteSession session, - MetricUpdater metrics, - Executor zkWatcherExecutor) { - this.fileCache = fileCache; - this.reloadHandler = reloadHandler; - this.session = session; - this.metrics = metrics; - this.fileCache.start(); - this.fileCache.addListener(this::nodeChanged); - this.zkWatcherExecutor = zkWatcherExecutor; - } - - private void sessionChanged(Session.Status status) { - // valid for NEW -> PREPARE transitions, not ACTIVATE -> PREPARE. - if (status.equals(Session.Status.PREPARE)) { - log.log(Level.FINE, session.logPre() + "Loading prepared session: " + session.getSessionId()); - session.loadPrepared(); - } else if (status.equals(Session.Status.ACTIVATE)) { - session.makeActive(reloadHandler); - } else if (status.equals(Session.Status.DEACTIVATE)) { - session.deactivate(); - } else if (status.equals(Session.Status.DELETE)) { - session.deactivate(); - } - } - - public long getSessionId() { - return session.getSessionId(); - } - - public void close() { - try { - fileCache.close(); - } catch (Exception e) { - log.log(Level.WARNING, "Exception when closing watcher", e); - } - } - - private void nodeChanged() { - zkWatcherExecutor.execute(() -> { - Session.Status currentStatus = session.getStatus(); - Session.Status newStatus = Session.Status.NONE; - try { - ChildData node = fileCache.getCurrentData(); - if (node != null) { - newStatus = Session.Status.parse(Utf8.toString(node.getData())); - log.log(Level.FINE, session.logPre() + "Session change: Remote session " + session.getSessionId() + - " changed status from " + currentStatus.name() + " to " + newStatus.name()); - sessionChanged(newStatus); - } - } catch (Exception e) { - log.log(Level.WARNING, session.logPre() + "Error handling session change from " + currentStatus.name() + - " to " + newStatus.name() + " for session " + getSessionId(), e); - metrics.incSessionChangeErrors(); - } - }); - } - -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index f05463b8e3b..2110e6476b6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -72,8 +72,7 @@ public class SessionRepository { private final SessionCache<LocalSession> localSessionCache = new SessionCache<>(); private final SessionCache<RemoteSession> remoteSessionCache = new SessionCache<>(); - private final Map<Long, LocalSessionStateWatcher> localSessionStateWatchers = new HashMap<>(); - private final Map<Long, RemoteSessionStateWatcher> remoteSessionStateWatchers = new HashMap<>(); + private final Map<Long, SessionStateWatcher> sessionStateWatchers = new HashMap<>(); private final Duration sessionLifetime; private final Clock clock; private final Curator curator; @@ -122,7 +121,8 @@ public class SessionRepository { localSessionCache.addSession(session); long sessionId = session.getSessionId(); Curator.FileCache fileCache = curator.createFileCache(getSessionStatePath(sessionId).getAbsolute(), false); - localSessionStateWatchers.put(sessionId, new LocalSessionStateWatcher(fileCache, session, this, zkWatcherExecutor)); + RemoteSession remoteSession = new RemoteSession(tenantName, sessionId, componentRegistry, createSessionZooKeeperClient(sessionId)); + addWatcher(sessionId, fileCache, remoteSession, Optional.of(session)); } public LocalSession getLocalSession(long sessionId) { @@ -207,7 +207,7 @@ public class SessionRepository { public void deleteLocalSession(LocalSession session) { long sessionId = session.getSessionId(); log.log(Level.FINE, "Deleting local session " + sessionId); - LocalSessionStateWatcher watcher = localSessionStateWatchers.remove(sessionId); + SessionStateWatcher watcher = sessionStateWatchers.remove(sessionId); if (watcher != null) watcher.close(); localSessionCache.removeSession(sessionId); NestedTransaction transaction = new NestedTransaction(); @@ -316,21 +316,22 @@ public class SessionRepository { * @param sessionId session id for the new session */ public void sessionAdded(long sessionId) { - log.log(Level.FINE, () -> "Adding session to SessionRepository: " + sessionId); - RemoteSession session = createRemoteSession(sessionId); + log.log(Level.FINE, () -> "Adding remote session to SessionRepository: " + sessionId); + RemoteSession remoteSession = createRemoteSession(sessionId); Curator.FileCache fileCache = curator.createFileCache(getSessionStatePath(sessionId).getAbsolute(), false); fileCache.addListener(this::nodeChanged); - loadSessionIfActive(session); - addRemoteSession(session); - remoteSessionStateWatchers.put(sessionId, new RemoteSessionStateWatcher(fileCache, reloadHandler, session, metrics, zkWatcherExecutor)); + loadSessionIfActive(remoteSession); + addRemoteSession(remoteSession); + Optional<LocalSession> localSession = Optional.empty(); if (distributeApplicationPackage.value()) { - Optional<LocalSession> localSession = createLocalSessionUsingDistributedApplicationPackage(sessionId); + localSession = createLocalSessionUsingDistributedApplicationPackage(sessionId); localSession.ifPresent(this::addSession); } + addWatcher(sessionId, fileCache, remoteSession, localSession); } private void sessionRemoved(long sessionId) { - RemoteSessionStateWatcher watcher = remoteSessionStateWatchers.remove(sessionId); + SessionStateWatcher watcher = sessionStateWatchers.remove(sessionId); if (watcher != null) watcher.close(); remoteSessionCache.removeSession(sessionId); metrics.incRemovedSessions(); @@ -595,6 +596,16 @@ public class SessionRepository { return new TenantFileSystemDirs(componentRegistry.getConfigServerDB(), tenantName).getUserApplicationDir(sessionId); } + private void addWatcher(long sessionId, Curator.FileCache fileCache, RemoteSession remoteSession, Optional<LocalSession> localSession) { + // Remote session will always be present in an existing state watcher, but local session might not + if (sessionStateWatchers.containsKey(sessionId)) { + localSession.ifPresent(session -> sessionStateWatchers.get(sessionId).addLocalSession(session)); + } else { + sessionStateWatchers.put(sessionId, new SessionStateWatcher(fileCache, reloadHandler, remoteSession, + localSession, metrics, zkWatcherExecutor, this)); + } + } + @Override public String toString() { return getLocalSessions().toString(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java new file mode 100644 index 00000000000..f182a28ab70 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java @@ -0,0 +1,107 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.session; + +import java.util.Optional; +import java.util.logging.Level; +import com.yahoo.text.Utf8; +import com.yahoo.vespa.config.server.ReloadHandler; +import com.yahoo.vespa.config.server.monitoring.MetricUpdater; +import com.yahoo.vespa.curator.Curator; +import org.apache.curator.framework.recipes.cache.ChildData; + +import java.util.concurrent.Executor; +import java.util.logging.Logger; + +/** + * Watches one particular session (/config/v2/tenants/<tenantName>/sessions/<n>/sessionState in ZooKeeper) + * The session must be in the session repo. + * + * @author Vegard Havdal + */ +public class SessionStateWatcher { + + private static final Logger log = Logger.getLogger(SessionStateWatcher.class.getName()); + + private final Curator.FileCache fileCache; + private final ReloadHandler reloadHandler; + private final RemoteSession remoteSession; + private final MetricUpdater metrics; + private final Executor zkWatcherExecutor; + private final SessionRepository sessionRepository; + private Optional<LocalSession> localSession; + + SessionStateWatcher(Curator.FileCache fileCache, + ReloadHandler reloadHandler, + RemoteSession remoteSession, + Optional<LocalSession> localSession, + MetricUpdater metrics, + Executor zkWatcherExecutor, + SessionRepository sessionRepository) { + this.fileCache = fileCache; + this.reloadHandler = reloadHandler; + this.remoteSession = remoteSession; + this.localSession = localSession; + this.metrics = metrics; + this.fileCache.start(); + this.fileCache.addListener(this::nodeChanged); + this.zkWatcherExecutor = zkWatcherExecutor; + this.sessionRepository = sessionRepository; + } + + private void sessionStatusChanged(Session.Status newStatus) { + long sessionId = remoteSession.getSessionId(); + log.log(Level.FINE, remoteSession.logPre() + "Session change: Session " + remoteSession.getSessionId() + " changed status to " + newStatus); + + if (newStatus.equals(Session.Status.PREPARE)) { + log.log(Level.FINE, remoteSession.logPre() + "Loading prepared session: " + sessionId); + remoteSession.loadPrepared(); + } else if (newStatus.equals(Session.Status.ACTIVATE)) { + remoteSession.makeActive(reloadHandler); + } else if (newStatus.equals(Session.Status.DEACTIVATE)) { + remoteSession.deactivate(); + } else if (newStatus.equals(Session.Status.DELETE)) { + remoteSession.deactivate(); + localSession.ifPresent(session -> { + log.log(Level.FINE, session.logPre() + "Deleting session " + sessionId); + sessionRepository.deleteLocalSession(session); + }); + } + } + + public long getSessionId() { + return remoteSession.getSessionId(); + } + + public void close() { + try { + fileCache.close(); + } catch (Exception e) { + log.log(Level.WARNING, "Exception when closing watcher", e); + } + } + + private void nodeChanged() { + zkWatcherExecutor.execute(() -> { + Session.Status currentStatus = remoteSession.getStatus(); + Session.Status newStatus = Session.Status.NONE; + try { + ChildData node = fileCache.getCurrentData(); + if (node != null) { + newStatus = Session.Status.parse(Utf8.toString(node.getData())); + log.log(Level.FINE, remoteSession.logPre() + "Session change: Remote session " + remoteSession.getSessionId() + + " changed status from " + currentStatus.name() + " to " + newStatus.name()); + sessionStatusChanged(newStatus); + } + } catch (Exception e) { + log.log(Level.WARNING, remoteSession.logPre() + "Error handling session change from " + currentStatus.name() + + " to " + newStatus.name() + " for session " + getSessionId(), e); + metrics.incSessionChangeErrors(); + } + }); + } + + void addLocalSession(LocalSession session) { + localSession = Optional.of(session); + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SilentDeployLogger.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SilentDeployLogger.java index 92ab6b3fbf5..9e2e5ddf698 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SilentDeployLogger.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SilentDeployLogger.java @@ -7,8 +7,9 @@ import java.util.logging.Logger; import com.yahoo.config.application.api.DeployLogger; /** - * The purpose of this is to mute the log messages from model and application building in {@link RemoteSession} that is triggered by {@link RemoteSessionStateWatcher}, since those messages already - * have been emitted by the prepare handler, for the same prepare operation. + * The purpose of this is to mute the log messages from model and application building in {@link RemoteSession} that + * is triggered by {@link SessionStateWatcher}, since those messages already have been emitted by the prepare + * handler, for the same prepare operation. * * @author vegardh * diff --git a/searchcore/src/apps/proton/proton.cpp b/searchcore/src/apps/proton/proton.cpp index 8f2be3257fc..8709b0d01b1 100644 --- a/searchcore/src/apps/proton/proton.cpp +++ b/searchcore/src/apps/proton/proton.cpp @@ -124,11 +124,9 @@ public: int64_t getGeneration() const override; }; -ProtonServiceLayerProcess::ProtonServiceLayerProcess(const config::ConfigUri & - configUri, +ProtonServiceLayerProcess::ProtonServiceLayerProcess(const config::ConfigUri & configUri, proton::Proton & proton, - PersistenceProvider * - downPersistence) + PersistenceProvider * downPersistence) : ServiceLayerProcess(configUri), _proton(proton), _metricManager(nullptr), diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp index 721ee9978b0..118cad4d8ef 100644 --- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp @@ -79,7 +79,7 @@ App::verify(const search::index::Schema &schema, const search::fef::Properties &props, const IConstantValueRepo &repo) { - proton::matching::IndexEnvironment indexEnv(schema, props, repo); + proton::matching::IndexEnvironment indexEnv(0, schema, props, repo); search::fef::BlueprintFactory factory; search::features::setup_search_features(factory); search::fef::test::setup_fef_test_plugin(factory); diff --git a/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp b/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp index 8c651c72df0..932ab6f4d14 100644 --- a/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp +++ b/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp @@ -42,7 +42,7 @@ struct Fixture { Fixture(Schema::UP schema_) : repo(), schema(std::move(schema_)), - env(*schema, Properties(), repo) + env(7, *schema, Properties(), repo) { } const FieldInfo *assertField(size_t idx, @@ -84,6 +84,11 @@ TEST_F("require that document meta store is always extracted in index environmen TEST_DO(f.assertHiddenAttributeField(0, "[documentmetastore]", DataType::RAW, CollectionType::SINGLE)); } +TEST_F("require that distribution key is visible in index environment", Fixture(buildEmptySchema())) +{ + ASSERT_EQUAL(7u, f.env.getDistributionKey()); +} + TEST_F("require that imported attribute fields are extracted in index environment", Fixture(buildSchema())) { ASSERT_EQUAL(3u, f.env.getNumFields()); diff --git a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp index b86750f15ee..d6d185ccab4 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp @@ -61,7 +61,8 @@ IndexEnvironment::insertField(const search::fef::FieldInfo &field) _fields.push_back(field); } -IndexEnvironment::IndexEnvironment(const search::index::Schema &schema, +IndexEnvironment::IndexEnvironment(uint32_t distributionKey, + const search::index::Schema &schema, const search::fef::Properties &props, const IConstantValueRepo &constantValueRepo) : _tableManager(), @@ -69,7 +70,8 @@ IndexEnvironment::IndexEnvironment(const search::index::Schema &schema, _fieldNames(), _fields(), _motivation(UNKNOWN), - _constantValueRepo(constantValueRepo) + _constantValueRepo(constantValueRepo), + _distributionKey(distributionKey) { _tableManager.addFactory(std::make_shared<search::fef::FunctionTableFactory>(256)); extractFields(schema); diff --git a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h index 5696fbf2379..7da45909577 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h +++ b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h @@ -25,6 +25,8 @@ private: std::vector<search::fef::FieldInfo> _fields; mutable FeatureMotivation _motivation; const IConstantValueRepo &_constantValueRepo; + uint32_t _distributionKey; + /** * Extract field information from the given schema and populate @@ -38,11 +40,13 @@ public: * Sets up this index environment based on the given schema and * properties. * + * @param distributionKey the distribution key for this node. * @param schema the index schema * @param props config * @param constantValueRepo repo used to access constant values for ranking **/ - IndexEnvironment(const search::index::Schema &schema, + IndexEnvironment(uint32_t distributionKey, + const search::index::Schema &schema, const search::fef::Properties &props, const IConstantValueRepo &constantValueRepo); @@ -55,6 +59,7 @@ public: void hintFeatureMotivation(FeatureMotivation motivation) const override; void hintFieldAccess(uint32_t fieldId) const override; void hintAttributeAccess(const string &name) const override; + uint32_t getDistributionKey() const override { return _distributionKey; } vespalib::eval::ConstantValue::UP getConstantValue(const vespalib::string &name) const override { return _constantValueRepo.getConstant(name); diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 93b4f63060f..735070002eb 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -100,7 +100,7 @@ handleGroupingSession(SessionManager &sessionMgr, GroupingContext & groupingCont Matcher::Matcher(const search::index::Schema &schema, const Properties &props, const vespalib::Clock &clock, QueryLimiter &queryLimiter, const IConstantValueRepo &constantValueRepo, uint32_t distributionKey) - : _indexEnv(schema, props, constantValueRepo), + : _indexEnv(distributionKey, schema, props, constantValueRepo), _blueprintFactory(), _rankSetup(), _viewResolver(ViewResolver::createFromSchema(schema)), diff --git a/searchlib/src/tests/features/prod_features.cpp b/searchlib/src/tests/features/prod_features.cpp index 3f71c9f85ea..c50c7a12698 100644 --- a/searchlib/src/tests/features/prod_features.cpp +++ b/searchlib/src/tests/features/prod_features.cpp @@ -34,6 +34,7 @@ #include <vespa/searchlib/features/setup.h> #include <vespa/searchlib/features/termfeature.h> #include <vespa/searchlib/features/utils.h> +#include <vespa/searchlib/features/uniquefeature.h> #include <vespa/searchlib/features/weighted_set_parser.hpp> #include <vespa/searchlib/fef/featurenamebuilder.h> #include <vespa/searchlib/fef/indexproperties.h> @@ -112,6 +113,7 @@ Test::Main() TEST_DO(testTerm()); TEST_FLUSH(); TEST_DO(testTermDistance()); TEST_FLUSH(); TEST_DO(testUtils()); TEST_FLUSH(); + TEST_DO(testUnique()); TEST_FLUSH(); TEST_DONE(); return 0; @@ -1564,6 +1566,24 @@ Test::testMatchCount() } void +Test::testUnique() +{ + { + UniqueBlueprint bp; + EXPECT_TRUE(assertCreateInstance(bp, "unique")); + FtFeatureTest ft(_factory, ""); + StringList params, in, out; + FT_SETUP_OK(bp, ft.getIndexEnv(), params, in, out.add("out")); + FT_DUMP_EMPTY(_factory, "unique"); + } + FtFeatureTest ft(_factory, "unique"); + ASSERT_TRUE(ft.setup()); + EXPECT_TRUE(ft.execute(0x10003,0, 1)); + EXPECT_TRUE(ft.execute(0x70003,0, 7)); + +} + +void Test::testMatches() { { // Test blueprint. diff --git a/searchlib/src/tests/features/prod_features.h b/searchlib/src/tests/features/prod_features.h index 6d30dd3fcd8..7f444e8ff60 100644 --- a/searchlib/src/tests/features/prod_features.h +++ b/searchlib/src/tests/features/prod_features.h @@ -39,6 +39,7 @@ public: void testRankingExpression(); void testTerm(); void testTermDistance(); + void testUnique(); static void testUtils(); static void setupForDotProductTest(FtFeatureTest & ft); diff --git a/searchlib/src/vespa/searchlib/features/CMakeLists.txt b/searchlib/src/vespa/searchlib/features/CMakeLists.txt index 727ace182eb..a3ce67c4bf6 100644 --- a/searchlib/src/vespa/searchlib/features/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/features/CMakeLists.txt @@ -63,6 +63,7 @@ vespa_add_library(searchlib_features OBJECT termfeature.cpp terminfofeature.cpp text_similarity_feature.cpp + uniquefeature.cpp utils.cpp valuefeature.cpp weighted_set_parser.cpp diff --git a/searchlib/src/vespa/searchlib/features/matchcountfeature.cpp b/searchlib/src/vespa/searchlib/features/matchcountfeature.cpp index f19e32793a1..5a14e65bd28 100644 --- a/searchlib/src/vespa/searchlib/features/matchcountfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/matchcountfeature.cpp @@ -9,6 +9,27 @@ using namespace search::fef; namespace search::features { +namespace { + +/** +* Implements the executor for the matchCount feature for index and +* attribute fields. +*/ +class MatchCountExecutor : public fef::FeatureExecutor { +private: + std::vector<fef::TermFieldHandle> _handles; + const fef::MatchData *_md; + + void handle_bind_match_data(const fef::MatchData &md) override { + _md = &md; + } + +public: + MatchCountExecutor(uint32_t fieldId, const fef::IQueryEnvironment &env); + + void execute(uint32_t docId) override; +}; + MatchCountExecutor::MatchCountExecutor(uint32_t fieldId, const IQueryEnvironment &env) : FeatureExecutor(), _handles(), @@ -23,8 +44,7 @@ MatchCountExecutor::MatchCountExecutor(uint32_t fieldId, const IQueryEnvironment } void -MatchCountExecutor::execute(uint32_t docId) -{ +MatchCountExecutor::execute(uint32_t docId) { size_t output = 0; for (uint32_t i = 0; i < _handles.size(); ++i) { const TermFieldMatchData *tfmd = _md->resolveTermField(_handles[i]); @@ -35,10 +55,6 @@ MatchCountExecutor::execute(uint32_t docId) outputs().set_number(0, static_cast<feature_t>(output)); } -void -MatchCountExecutor::handle_bind_match_data(const MatchData &md) -{ - _md = &md; } MatchCountBlueprint::MatchCountBlueprint() : diff --git a/searchlib/src/vespa/searchlib/features/matchcountfeature.h b/searchlib/src/vespa/searchlib/features/matchcountfeature.h index a81f4f084b8..88c577f8b24 100644 --- a/searchlib/src/vespa/searchlib/features/matchcountfeature.h +++ b/searchlib/src/vespa/searchlib/features/matchcountfeature.h @@ -7,23 +7,6 @@ namespace search::features { /** - * Implements the executor for the matchCount feature for index and - * attribute fields. - */ -class MatchCountExecutor : public fef::FeatureExecutor -{ -private: - std::vector<fef::TermFieldHandle> _handles; - const fef::MatchData *_md; - - void handle_bind_match_data(const fef::MatchData &md) override; - -public: - MatchCountExecutor(uint32_t fieldId, const fef::IQueryEnvironment &env); - void execute(uint32_t docId) override; -}; - -/** * Implements the blueprint for the matchCount executor. * * matchCount(name) diff --git a/searchlib/src/vespa/searchlib/features/matchesfeature.cpp b/searchlib/src/vespa/searchlib/features/matchesfeature.cpp index 8d72978adf7..6fbb2f5d5f2 100644 --- a/searchlib/src/vespa/searchlib/features/matchesfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/matchesfeature.cpp @@ -11,6 +11,27 @@ using namespace search::fef; namespace search::features { +namespace { + +/** + * Implements the executor for the matches feature for index and + * attribute fields. + */ +class MatchesExecutor : public fef::FeatureExecutor { +private: + std::vector<fef::TermFieldHandle> _handles; + const fef::MatchData *_md; + + void handle_bind_match_data(const fef::MatchData &md) override; + +public: + MatchesExecutor(uint32_t fieldId, + const fef::IQueryEnvironment &env, + uint32_t begin, uint32_t end); + + void execute(uint32_t docId) override; +}; + MatchesExecutor::MatchesExecutor(uint32_t fieldId, const search::fef::IQueryEnvironment &env, uint32_t begin, uint32_t end) @@ -27,8 +48,7 @@ MatchesExecutor::MatchesExecutor(uint32_t fieldId, } void -MatchesExecutor::execute(uint32_t docId) -{ +MatchesExecutor::execute(uint32_t docId) { size_t output = 0; for (uint32_t i = 0; i < _handles.size(); ++i) { const TermFieldMatchData *tfmd = _md->resolveTermField(_handles[i]); @@ -41,11 +61,12 @@ MatchesExecutor::execute(uint32_t docId) } void -MatchesExecutor::handle_bind_match_data(const MatchData &md) -{ +MatchesExecutor::handle_bind_match_data(const MatchData &md) { _md = &md; } +} + MatchesBlueprint::MatchesBlueprint() : Blueprint("matches"), _field(nullptr), diff --git a/searchlib/src/vespa/searchlib/features/matchesfeature.h b/searchlib/src/vespa/searchlib/features/matchesfeature.h index 53c6151a713..671d1fe31c0 100644 --- a/searchlib/src/vespa/searchlib/features/matchesfeature.h +++ b/searchlib/src/vespa/searchlib/features/matchesfeature.h @@ -7,25 +7,6 @@ namespace search::features { /** - * Implements the executor for the matches feature for index and - * attribute fields. - */ -class MatchesExecutor : public fef::FeatureExecutor -{ -private: - std::vector<fef::TermFieldHandle> _handles; - const fef::MatchData *_md; - - void handle_bind_match_data(const fef::MatchData &md) override; - -public: - MatchesExecutor(uint32_t fieldId, - const fef::IQueryEnvironment &env, - uint32_t begin, uint32_t end); - void execute(uint32_t docId) override; -}; - -/** * Implements the blueprint for the matches executor. * * matches(name) diff --git a/searchlib/src/vespa/searchlib/features/setup.cpp b/searchlib/src/vespa/searchlib/features/setup.cpp index bf8b59f100c..ea6ec842a00 100644 --- a/searchlib/src/vespa/searchlib/features/setup.cpp +++ b/searchlib/src/vespa/searchlib/features/setup.cpp @@ -53,6 +53,7 @@ #include "termfeature.h" #include "terminfofeature.h" #include "text_similarity_feature.h" +#include "uniquefeature.h" #include "valuefeature.h" #include "max_reduce_prod_join_replacer.h" @@ -121,6 +122,8 @@ void setup_search_features(fef::IBlueprintRegistry & registry) registry.addPrototype(std::make_shared<TermEditDistanceBlueprint>()); registry.addPrototype(std::make_shared<TermFieldMdBlueprint>()); registry.addPrototype(std::make_shared<ConstantBlueprint>()); + registry.addPrototype(std::make_shared<UniqueBlueprint>()); + // Ranking Expression auto replacers = std::make_unique<ListExpressionReplacer>(); diff --git a/searchlib/src/vespa/searchlib/features/uniquefeature.cpp b/searchlib/src/vespa/searchlib/features/uniquefeature.cpp new file mode 100644 index 00000000000..73ac4a1178e --- /dev/null +++ b/searchlib/src/vespa/searchlib/features/uniquefeature.cpp @@ -0,0 +1,64 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "uniquefeature.h" +#include <vespa/vespalib/util/stash.h> + +using namespace search::fef; + +namespace search::features { + +namespace { + +/** + * Implements the executor for combining lid and distribution key to form a globally unique value. + */ +class UniqueLidAndDistributionKeyExecutor : public fef::FeatureExecutor { +private: + uint32_t _distributionKey; + +public: + UniqueLidAndDistributionKeyExecutor(uint32_t distributionKey) + : _distributionKey(distributionKey) + { + assert( _distributionKey < 0x10000); + } + + void execute(uint32_t docId) override { + outputs().set_number(0, (uint64_t(docId) << 16u) | _distributionKey); + } +}; + +} + +UniqueBlueprint::UniqueBlueprint() : + Blueprint("unique"), + _distributionKey(0) +{ +} + +void +UniqueBlueprint::visitDumpFeatures(const IIndexEnvironment &, IDumpFeatureVisitor &) const +{ +} + +bool +UniqueBlueprint::setup(const IIndexEnvironment & env, const ParameterList & ) +{ + _distributionKey = env.getDistributionKey(); + describeOutput("out", "Returns (lid << 16) | distributionKey"); + return true; +} + +Blueprint::UP +UniqueBlueprint::createInstance() const +{ + return std::make_unique<UniqueBlueprint>(); +} + +FeatureExecutor & +UniqueBlueprint::createExecutor(const IQueryEnvironment &, vespalib::Stash &stash) const +{ + return stash.create<UniqueLidAndDistributionKeyExecutor>(_distributionKey); +} + +} diff --git a/searchlib/src/vespa/searchlib/features/uniquefeature.h b/searchlib/src/vespa/searchlib/features/uniquefeature.h new file mode 100644 index 00000000000..f21a427762a --- /dev/null +++ b/searchlib/src/vespa/searchlib/features/uniquefeature.h @@ -0,0 +1,32 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/searchlib/fef/blueprint.h> + +namespace search::features { + +/** + * Implements the blueprint for the unique rank feature. + * + * This will compute a globally unique id based on lid and distribution key. + * Cheap way to get deterministic ordering + * It will change if documents change lid. + */ + +class UniqueBlueprint : public fef::Blueprint +{ +private: + uint32_t _distributionKey; +public: + UniqueBlueprint(); + void visitDumpFeatures(const fef::IIndexEnvironment & env, fef::IDumpFeatureVisitor & visitor) const override; + fef::Blueprint::UP createInstance() const override; + fef::ParameterDescriptions getDescriptions() const override { + return fef::ParameterDescriptions(); + } + bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override; + fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override; +}; + +} diff --git a/searchlib/src/vespa/searchlib/fef/iindexenvironment.h b/searchlib/src/vespa/searchlib/fef/iindexenvironment.h index 7cf3f4e140c..bdeead3e852 100644 --- a/searchlib/src/vespa/searchlib/fef/iindexenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/iindexenvironment.h @@ -120,6 +120,8 @@ public: */ virtual std::unique_ptr<vespalib::eval::ConstantValue> getConstantValue(const vespalib::string &name) const = 0; + virtual uint32_t getDistributionKey() const = 0; + /** * Virtual destructor to allow safe subclassing. **/ diff --git a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h index 09c64fcac7a..d84cebc7f52 100644 --- a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h @@ -60,6 +60,7 @@ public: void hintFeatureMotivation(FeatureMotivation) const override {} void hintFieldAccess(uint32_t) const override {} void hintAttributeAccess(const string &) const override {} + uint32_t getDistributionKey() const override { return 3; } /** Returns a reference to the properties map of this. */ Properties &getProperties() { return _properties; } @@ -76,7 +77,7 @@ public: /** Returns a reference to the table manager of this. */ TableManager &getTableManager() { return _tableMan; } - virtual vespalib::eval::ConstantValue::UP getConstantValue(const vespalib::string &name) const override; + vespalib::eval::ConstantValue::UP getConstantValue(const vespalib::string &name) const override; void addConstantValue(const vespalib::string &name, vespalib::eval::ValueType type, diff --git a/searchlib/src/vespa/searchlib/fef/test/rankresult.cpp b/searchlib/src/vespa/searchlib/fef/test/rankresult.cpp index abaaa1d100e..15d4ce317cc 100644 --- a/searchlib/src/vespa/searchlib/fef/test/rankresult.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/rankresult.cpp @@ -6,9 +6,7 @@ #include <vespa/log/log.h> LOG_SETUP(".fef.rankresult"); -namespace search { -namespace fef { -namespace test { +namespace search::fef::test { RankResult::RankResult() : _rankScores(), @@ -107,6 +105,4 @@ std::ostream & operator<<(std::ostream & os, const RankResult & rhs) { return os << "]"; } -} // namespace test -} // namespace fef -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/fef/test/rankresult.h b/searchlib/src/vespa/searchlib/fef/test/rankresult.h index 46ae3673e06..8c1efdbb36c 100644 --- a/searchlib/src/vespa/searchlib/fef/test/rankresult.h +++ b/searchlib/src/vespa/searchlib/fef/test/rankresult.h @@ -2,13 +2,11 @@ #pragma once #include <vespa/searchlib/common/feature.h> -#include <map> #include <vespa/vespalib/stllike/string.h> #include <vector> +#include <map> -namespace search { -namespace fef { -namespace test { +namespace search::fef::test { class RankResult { public: @@ -107,7 +105,4 @@ private: double _epsilon; }; -} // namespace test -} // namespace fef -} // namespace search - +} diff --git a/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp b/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp index af9ba53ef70..bde93b9e4fb 100644 --- a/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp +++ b/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp @@ -14,7 +14,7 @@ ServiceLayerProcess::ServiceLayerProcess(const config::ConfigUri & configUri) { } -ServiceLayerProcess::~ServiceLayerProcess() {} +ServiceLayerProcess::~ServiceLayerProcess() = default; void ServiceLayerProcess::shutdown() @@ -26,7 +26,7 @@ ServiceLayerProcess::shutdown() void ServiceLayerProcess::createNode() { - _externalVisitors["searchvisitor"].reset(new streaming::SearchVisitorFactory(_configUri)); + _externalVisitors["searchvisitor"] = std::make_shared<streaming::SearchVisitorFactory>(_configUri); setupProvider(); _node = std::make_unique<ServiceLayerNode>(_configUri, _context, *this, getProvider(), _externalVisitors); _node->init(); diff --git a/streamingvisitors/src/vespa/searchvisitor/indexenvironment.h b/streamingvisitors/src/vespa/searchvisitor/indexenvironment.h index 5d684c4fea4..ac6836b08c5 100644 --- a/streamingvisitors/src/vespa/searchvisitor/indexenvironment.h +++ b/streamingvisitors/src/vespa/searchvisitor/indexenvironment.h @@ -81,6 +81,9 @@ public: const std::set<vespalib::string> & getHintedDumpAttributes() const { return _dumpAttributes; } + //TODO Wire in proper distribution key + uint32_t getDistributionKey() const override { return 0; } + }; } // namespace streaming diff --git a/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp b/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp index 8dbbbba98cf..2c5f5eeb1b7 100644 --- a/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp @@ -176,7 +176,8 @@ RankManager::Snapshot::setup(const RankManager & rm, const RankProfilesConfig & return setup(rm); } -void RankManager::notify(const vsm::VSMConfigSnapshot & snap) +void +RankManager::notify(const vsm::VSMConfigSnapshot & snap) { configureRankProfiles(*snap.getConfig<RankProfilesConfig>()); } @@ -187,7 +188,7 @@ RankManager::configureRankProfiles(const RankProfilesConfig & cfg) { LOG(debug, "configureRankProfiles(): Size of cfg rankprofiles: %zd", cfg.rankprofile.size()); - std::unique_ptr<Snapshot> snapshot(new Snapshot()); + auto snapshot = std::make_unique<Snapshot>(); if (snapshot->setup(*this, cfg)) { _snapshot.set(snapshot.release()); _snapshot.latch(); // switch to the new config object diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index cc48ad09aa6..76ef0f23dd2 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -357,7 +357,7 @@ SearchVisitorFactory::SearchVisitorFactory(const config::ConfigUri & configUri) VisitorEnvironment::UP SearchVisitorFactory::makeVisitorEnvironment(StorageComponent&) { - return VisitorEnvironment::UP(new SearchEnvironment(_configUri)); + return std::make_unique<SearchEnvironment>(_configUri); } storage::Visitor* diff --git a/tenant-base/pom.xml b/tenant-base/pom.xml index 767119d2a02..490dcf6371b 100644 --- a/tenant-base/pom.xml +++ b/tenant-base/pom.xml @@ -117,6 +117,11 @@ </exclusion> </exclusions> </dependency> + <dependency> + <groupId>org.junit.jupiter</groupId> + <artifactId>junit-jupiter-engine</artifactId> + <scope>test</scope> + </dependency> </dependencies> <profiles> |