diff options
37 files changed, 343 insertions, 508 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 2347faf233b..99c95670208 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -146,15 +146,16 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } public PrepareResult deploy(CompressedApplicationInputStream in, PrepareParams prepareParams) { - Tenant tenant = tenantRepository.getTenant(prepareParams.getApplicationId().tenant()); - return deploy(tenant, in, prepareParams, false, false, clock.instant()); + return deploy(in, prepareParams, false, false, clock.instant()); } - public PrepareResult deploy(Tenant tenant, CompressedApplicationInputStream in, PrepareParams prepareParams, + public PrepareResult deploy(CompressedApplicationInputStream in, PrepareParams prepareParams, boolean ignoreLockFailure, boolean ignoreSessionStaleFailure, Instant now) { File tempDir = Files.createTempDir(); - long sessionId = createSession(tenant, prepareParams.getTimeoutBudget(), decompressApplication(in, tempDir), prepareParams.getApplicationName()); + ApplicationId applicationId = prepareParams.getApplicationId(); + long sessionId = createSession(applicationId, prepareParams.getTimeoutBudget(), decompressApplication(in, tempDir)); cleanupApplicationDirectory(tempDir, logger); + Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); return prepareAndActivate(tenant, sessionId, prepareParams, ignoreLockFailure, ignoreSessionStaleFailure, now); } @@ -327,8 +328,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye throw new IllegalStateException("Session not prepared: " + sessionId); } - public long createSessionFromExisting(Tenant tenant, DeployLogger logger, - TimeoutBudget timeoutBudget, ApplicationId applicationId) { + public long createSessionFromExisting(ApplicationId applicationId, DeployLogger logger, TimeoutBudget timeoutBudget) { + Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); LocalSessionRepo localSessionRepo = tenant.getLocalSessionRepo(); SessionFactory sessionFactory = tenant.getSessionFactory(); LocalSession fromSession = getExistingSession(tenant, applicationId); @@ -337,17 +338,18 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye return session.getSessionId(); } - public long createSession(Tenant tenant, TimeoutBudget timeoutBudget, InputStream in, String contentType, String applicationName) { + public long createSession(ApplicationId applicationId, TimeoutBudget timeoutBudget, InputStream in, String contentType) { File tempDir = Files.createTempDir(); - long sessionId = createSession(tenant, timeoutBudget, decompressApplication(in, contentType, tempDir), applicationName); + long sessionId = createSession(applicationId, timeoutBudget, decompressApplication(in, contentType, tempDir)); cleanupApplicationDirectory(tempDir, logger); return sessionId; } - public long createSession(Tenant tenant, TimeoutBudget timeoutBudget, File applicationDirectory, String applicationName) { + public long createSession(ApplicationId applicationId, TimeoutBudget timeoutBudget, File applicationDirectory) { + Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); LocalSessionRepo localSessionRepo = tenant.getLocalSessionRepo(); SessionFactory sessionFactory = tenant.getSessionFactory(); - LocalSession session = sessionFactory.createSession(applicationDirectory, applicationName, timeoutBudget); + LocalSession session = sessionFactory.createSession(applicationDirectory, applicationId, timeoutBudget); localSessionRepo.addSession(session); return session.getSessionId(); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java index 9c19b4dce87..411e24cdbc6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java @@ -68,8 +68,7 @@ public class ApplicationApiHandler extends SessionHandler { SessionCreateHandler.validateDataAndHeader(request); PrepareResult result = - applicationRepository.deploy(tenant, - CompressedApplicationInputStream.createFromCompressedStream(request.getData(), request.getHeader(contentTypeHeader)), + applicationRepository.deploy(CompressedApplicationInputStream.createFromCompressedStream(request.getData(), request.getHeader(contentTypeHeader)), prepareParams, shouldIgnoreLockFailure(request), shouldIgnoreSessionStaleFailure(request), diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java index 07858d3864c..caf95f9f503 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java @@ -5,6 +5,8 @@ import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; @@ -13,7 +15,6 @@ import com.yahoo.log.LogLevel; import com.yahoo.slime.Slime; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.deploy.DeployHandlerLogger; -import com.yahoo.vespa.config.server.tenant.Tenant; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.server.TimeoutBudget; import com.yahoo.vespa.config.server.http.BadRequestException; @@ -52,17 +53,18 @@ public class SessionCreateHandler extends SessionHandler { Slime deployLog = createDeployLog(); final TenantName tenantName = Utils.getTenantNameFromSessionRequest(request); Utils.checkThatTenantExists(tenantRepository, tenantName); - Tenant tenant = tenantRepository.getTenant(tenantName); TimeoutBudget timeoutBudget = SessionHandler.getTimeoutBudget(request, zookeeperBarrierTimeout); DeployLogger logger = createLogger(request, deployLog, tenantName); long sessionId; if (request.hasProperty("from")) { ApplicationId applicationId = getFromApplicationId(request); - sessionId = applicationRepository.createSessionFromExisting(tenant, logger, timeoutBudget, applicationId); + sessionId = applicationRepository.createSessionFromExisting(applicationId, logger, timeoutBudget); } else { validateDataAndHeader(request); String name = getNameProperty(request, logger); - sessionId = applicationRepository.createSession(tenant, timeoutBudget, request.getData(), request.getHeader(ApplicationApiHandler.contentTypeHeader), name); + // TODO: we are always using instance name 'default' here, fix + ApplicationId applicationId = ApplicationId.from(tenantName, ApplicationName.from(name), InstanceName.defaultName()); + sessionId = applicationRepository.createSession(applicationId, timeoutBudget, request.getData(), request.getHeader(ApplicationApiHandler.contentTypeHeader)); } return createResponse(request, tenantName, deployLog, sessionId); } @@ -86,12 +88,12 @@ public class SessionCreateHandler extends SessionHandler { .instanceName(match.group(6)).build(); } - static DeployHandlerLogger createLogger(HttpRequest request, Slime deployLog, TenantName tenant) { + private static DeployHandlerLogger createLogger(HttpRequest request, Slime deployLog, TenantName tenant) { return SessionHandler.createLogger(deployLog, request, new ApplicationId.Builder().tenant(tenant).applicationName("-").build()); } - static String getNameProperty(HttpRequest request, DeployLogger logger) { + private static String getNameProperty(HttpRequest request, DeployLogger logger) { String name = request.getProperty("name"); // TODO: Do we need validation of this parameter? if (name == null) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java index 172620a643a..bd9da36a2ba 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.server.session; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.server.TimeoutBudget; import java.io.File; @@ -19,11 +20,11 @@ public interface SessionFactory { * * * @param applicationDirectory a File pointing to an application. - * @param applicationName name of the application for this new session. + * @param applicationId application id for this new session. * @param timeoutBudget Timeout for creating session and waiting for other servers. * @return a new session */ - LocalSession createSession(File applicationDirectory, String applicationName, TimeoutBudget timeoutBudget); + LocalSession createSession(File applicationDirectory, ApplicationId applicationId, TimeoutBudget timeoutBudget); /** * Creates a new deployment session from an already existing session. diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java index f260fade41c..7285ff905ff 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java @@ -1,7 +1,6 @@ // 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 com.yahoo.config.application.api.ApplicationMetaData; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.application.provider.*; @@ -64,7 +63,7 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { this.sessionPreparer = globalComponentRegistry.getSessionPreparer(); this.curator = globalComponentRegistry.getCurator(); this.configCurator = globalComponentRegistry.getConfigCurator(); - this.sessionCounter = new SessionCounter(globalComponentRegistry.getConfigCurator(), tenant);; + this.sessionCounter = new SessionCounter(globalComponentRegistry.getConfigCurator(), tenant); this.sessionsPath = TenantRepository.getSessionsPath(tenant); this.applicationRepo = applicationRepo; this.tenantFileSystemDirs = tenantFileSystemDirs; @@ -76,8 +75,8 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { } @Override - public LocalSession createSession(File applicationFile, String applicationName, TimeoutBudget timeoutBudget) { - return create(applicationFile, applicationName, nonExistingActiveSession, timeoutBudget); + public LocalSession createSession(File applicationFile, ApplicationId applicationId, TimeoutBudget timeoutBudget) { + return create(applicationFile, applicationId, nonExistingActiveSession, timeoutBudget); } private void ensureZKPathDoesNotExist(Path sessionPath) { @@ -122,18 +121,17 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { DeployLogger logger, TimeoutBudget timeoutBudget) { File existingApp = getSessionAppDir(existingSession.getSessionId()); - ApplicationMetaData metaData = FilesApplicationPackage.readMetaData(existingApp); ApplicationId existingApplicationId = existingSession.getApplicationId(); long liveApp = getLiveApp(existingApplicationId); logger.log(LogLevel.DEBUG, "Create from existing application id " + existingApplicationId + ", live app for it is " + liveApp); - LocalSession session = create(existingApp, metaData.getApplicationName(), liveApp, timeoutBudget); + LocalSession session = create(existingApp, existingApplicationId, liveApp, timeoutBudget); session.setApplicationId(existingApplicationId); session.setVespaVersion(existingSession.getVespaVersion()); return session; } - private LocalSession create(File applicationFile, String applicationName, long currentlyActiveSession, TimeoutBudget timeoutBudget) { + private LocalSession create(File applicationFile, ApplicationId applicationId, long currentlyActiveSession, TimeoutBudget timeoutBudget) { long sessionId = sessionCounter.nextSessionId(); Path sessionIdPath = sessionsPath.append(String.valueOf(sessionId)); log.log(LogLevel.DEBUG, TenantRepository.logPre(tenant) + "Next session id is " + sessionId + " , sessionIdPath=" + sessionIdPath.getAbsolute()); @@ -147,7 +145,8 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { nodeFlavors); File userApplicationDir = tenantFileSystemDirs.getUserApplicationDir(sessionId); IOUtils.copyDirectory(applicationFile, userApplicationDir); - ApplicationPackage applicationPackage = createApplication(applicationFile, userApplicationDir, applicationName, sessionId, currentlyActiveSession); + ApplicationPackage applicationPackage = createApplication(applicationFile, userApplicationDir, + applicationId.application().value(), sessionId, currentlyActiveSession); applicationPackage.writeMetaData(); return createSessionFromApplication(applicationPackage, sessionId, sessionZooKeeperClient, timeoutBudget, clock); } catch (Exception e) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index c489c51038f..90f0b5ee4e5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -2,6 +2,9 @@ package com.yahoo.vespa.config.server; import com.yahoo.config.model.application.provider.FilesApplicationPackage; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.config.server.http.CompressedApplicationInputStream; @@ -80,20 +83,23 @@ public class ApplicationRepositoryTest { private PrepareResult prepareAndActivateApp(File application) throws IOException { FilesApplicationPackage appDir = FilesApplicationPackage.fromFile(application); - long sessionId = applicationRepository.createSession(tenant, timeoutBudget, appDir.getAppDir(), "testapp"); + long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, appDir.getAppDir()); return applicationRepository.prepareAndActivate(tenant, sessionId, prepareParams(), false, false, Instant.now()); } private PrepareResult createAndPrepareAndActivateApp() throws IOException { File file = CompressedApplicationInputStreamTest.createTarFile(); - return applicationRepository.deploy(tenant, - CompressedApplicationInputStream.createFromCompressedStream( + return applicationRepository.deploy(CompressedApplicationInputStream.createFromCompressedStream( new FileInputStream(file), ApplicationApiHandler.APPLICATION_X_GZIP), prepareParams(), false, false, Instant.now()); } private PrepareParams prepareParams() { - return new PrepareParams.Builder().build(); + return new PrepareParams.Builder().applicationId(applicationId()).build(); + } + + private ApplicationId applicationId() { + return ApplicationId.from(tenantName, ApplicationName.from("testapp"), InstanceName.defaultName()); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 266833ac7f3..ce53451ae2e 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -150,7 +150,7 @@ public class DeployTester { if (vespaVersion != null) paramsBuilder.vespaVersion(vespaVersion); - long sessionId = applicationRepository.createSession(tenant, timeoutBudget, testApp, appName); + long sessionId = applicationRepository.createSession(id, timeoutBudget, testApp); applicationRepository.prepare(tenant, sessionId, paramsBuilder.build(), now); applicationRepository.activate(tenant, sessionId, timeoutBudget, false, false); this.id = id; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java index 0f20ca3163b..930ae98c9e9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java @@ -191,9 +191,9 @@ public class SessionHandlerTest { public String applicationName; @Override - public LocalSession createSession(File applicationDirectory, String applicationName, TimeoutBudget timeoutBudget) { + public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, TimeoutBudget timeoutBudget) { createCalled = true; - this.applicationName = applicationName; + this.applicationName = applicationId.application().value(); if (doThrow) { throw new RuntimeException("foo"); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java index 7434bf411dd..531a2e3745b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java @@ -4,6 +4,10 @@ package com.yahoo.vespa.config.server.session; import com.google.common.io.Files; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.model.application.provider.BaseDeployLogger; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.TenantName; import com.yahoo.io.IOUtils; import com.yahoo.path.Path; import com.yahoo.vespa.config.server.*; @@ -34,7 +38,7 @@ public class SessionFactoryTest extends TestWithTenant { private SessionFactory factory; @Before - public void setup_test() throws Exception { + public void setup_test() { factory = tenant.getSessionFactory(); } @@ -67,8 +71,8 @@ public class SessionFactoryTest extends TestWithTenant { } @Test(expected = RuntimeException.class) - public void require_that_invalid_app_dir_is_handled() throws IOException { - factory.createSession(new File("doesnotpointtoavaliddir"), "music", TimeoutBudgetTest.day()); + public void require_that_invalid_app_dir_is_handled() { + createSession(new File("doesnotpointtoavaliddir"), "music"); } private LocalSession getLocalSession() throws IOException { @@ -78,6 +82,11 @@ public class SessionFactoryTest extends TestWithTenant { private LocalSession getLocalSession(String appName) throws IOException { CompressedApplicationInputStream app = CompressedApplicationInputStream.createFromCompressedStream( new FileInputStream(CompressedApplicationInputStreamTest.createTarFile()), ApplicationApiHandler.APPLICATION_X_GZIP); - return factory.createSession(app.decompress(Files.createTempDir()), appName, TimeoutBudgetTest.day()); + return createSession(app.decompress(Files.createTempDir()), appName); + } + + private LocalSession createSession(File applicationDirectory, String appName) { + ApplicationId applicationId = ApplicationId.from(TenantName.defaultName(), ApplicationName.from(appName), InstanceName.defaultName()); + return factory.createSession(applicationDirectory, applicationId, TimeoutBudgetTest.day()); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java index 298bd839393..4310124037e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; +import java.util.function.BiConsumer; /** * A regular hit from a Vespa backend @@ -217,7 +218,13 @@ public class FastHit extends Hit { return getSummaryValue(name); } - /** Returns the fields of this as a read-only map. This is more costly than fieldIterator() */ + @Override + public void forEachField(BiConsumer<String, Object> consumer) { + super.forEachField(consumer); + for (SummaryData summaryData : summaries) + summaryData.forEachField(consumer); + } + @Override public Map<String, Object> fields() { Map<String, Object> fields = new HashMap<>(); @@ -228,14 +235,23 @@ public class FastHit extends Hit { return fields; } - /** Returns a modifiable iterator over the fields of this */ @Override public Iterator<Map.Entry<String, Object>> fieldIterator() { return new FieldIterator(this, super.fieldIterator()); } + /** + * Returns the keys of the fields of this hit as a modifiable view. + * This follows the rules of key sets returned from maps: Key removals are reflected + * in the map, add and addAll is not supported. + */ + @Override + public Set<String> fieldKeys() { + return new FieldSet(this); + } + /** Returns a modifiable iterator over the field names of this */ - Iterator<String> fieldNameIterator() { + private Iterator<String> fieldNameIterator() { return new FieldNameIterator(this, super.fieldKeys().iterator()); } @@ -268,16 +284,6 @@ public class FastHit extends Hit { return removedValue; } - /** - * Returns the keys of the fields of this hit as a modifiable view. - * This follows the rules of key sets returned from maps: Key removals are reflected - * in the map, add and addAll is not supported. - */ - @Override - public Set<String> fieldKeys() { - return new FieldSet(this); - } - private Set<String> mapFieldKeys() { return super.fieldKeys(); } @@ -520,25 +526,36 @@ public class FastHit extends Hit { return fieldType.convert(fieldValue); } - /** Decodes the given summary into the field map in the parent */ - private void decodeInto(FastHit hit) { - hit.reserve(type.getFieldCount()); - for (DocsumField field : type.getFields()) { - String fieldName = field.getName(); - Inspector f = data.field(fieldName); - if (field.getEmulConfig().forceFillEmptyFields() || f.valid()) { - if (hit.getField(fieldName) == null) - hit.setField(fieldName, field.convert(f)); - } - } + void forEachField(BiConsumer<String, Object> consumer) { + data.traverse((ObjectTraverser)(name, value) -> { + if ( ! shadowed(name) && ! removed(name)) + consumer.accept(name, value); + }); } - private Iterator<Map.Entry<String, Object>> fieldIterator() { - return new SummaryDataFieldIterator(hit, type, data.fields().iterator(), index); + Iterator<Map.Entry<String, Object>> fieldIterator() { + return new SummaryDataFieldIterator(this, type, data.fields().iterator()); + } + + Iterator<String> fieldNameIterator() { + return new SummaryDataFieldNameIterator(this, data.fields().iterator()); + } + + /** + * Returns whether this field is present in the map properties + * or an earlier (lower index) summary in this hit + */ + private boolean shadowed(String name) { + if (hit.hasField(name)) return true; + for (int i = 0; i < index; i++) { + if (hit.summaries.get(i).type.fieldNames().contains(name)) + return true; + } + return false; } - private Iterator<String> fieldNameIterator() { - return new SummaryDataFieldNameIterator(hit, data.fields().iterator(), index); + private boolean removed(String fieldName) { + return hit.removedFields != null && hit.removedFields.contains(fieldName); } /** @@ -547,19 +564,15 @@ public class FastHit extends Hit { */ private static abstract class SummaryDataIterator<VALUE> implements Iterator<VALUE> { - private final FastHit hit; + private final SummaryData summaryData; private final Iterator<Map.Entry<String, Inspector>> fieldIterator; - private final int index; /** The next value or null if none, eagerly read because we need to skip removed and overwritten values */ private VALUE next; - SummaryDataIterator(FastHit hit, - Iterator<Map.Entry<String, Inspector>> fieldIterator, - int index) { - this.hit = hit; + SummaryDataIterator(SummaryData summaryData, Iterator<Map.Entry<String, Inspector>> fieldIterator) { + this.summaryData = summaryData; this.fieldIterator = fieldIterator; - this.index = index; } @Override @@ -583,40 +596,23 @@ public class FastHit extends Hit { Map.Entry<String, Inspector> nextEntry = fieldIterator.next(); String fieldName = nextEntry.getKey(); next = toValue(nextEntry); - if ( next != null && - ! hit.hasField(fieldName) && - ! isRemoved(fieldName) && - ! isAlreadyReturned(fieldName)) + if ( next != null && ! summaryData.shadowed(fieldName) && ! summaryData.removed(fieldName)) return; } next = null; } - private boolean isRemoved(String fieldName) { - return hit.removedFields != null && hit.removedFields.contains(fieldName); - } - - private boolean isAlreadyReturned(String fieldName) { - for (int i = 0; i < index; i++) { - if (hit.summaries.get(i).type.fieldNames().contains(fieldName)) - return true; - } - return false; - } - } - /** Iterator over the fields in a SummaryData instance. Read only. */ private static class SummaryDataFieldIterator extends SummaryDataIterator<Map.Entry<String, Object>> { private final DocsumDefinition type; - SummaryDataFieldIterator(FastHit hit, + SummaryDataFieldIterator(SummaryData summaryData, DocsumDefinition type, - Iterator<Map.Entry<String, Inspector>> fieldIterator, - int index) { - super(hit, fieldIterator, index); + Iterator<Map.Entry<String, Inspector>> fieldIterator) { + super(summaryData, fieldIterator); this.type = type; advanceNext(); } @@ -654,10 +650,9 @@ public class FastHit extends Hit { /** Iterator over the field names in a SummaryData instance. Read only. */ private static class SummaryDataFieldNameIterator extends SummaryDataIterator<String> { - SummaryDataFieldNameIterator(FastHit hit, - Iterator<Map.Entry<String, Inspector>> fieldIterator, - int index) { - super(hit, fieldIterator, index); + SummaryDataFieldNameIterator(SummaryData summaryData, + Iterator<Map.Entry<String, Inspector>> fieldIterator) { + super(summaryData, fieldIterator); advanceNext(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java index 164ff9a993b..415ebd7871c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java @@ -74,7 +74,7 @@ public class DocumentSourceSearcher extends Searcher { removePropertiesNotStartingByA(attributeHit); attributeHit.setFillable(); attributeHit.setRelevance(fullHit.getRelevance()); - for (Object propertyKeyObject : (Set) fullHit.fields().keySet()) { + for (Object propertyKeyObject : fullHit.fields().keySet()) { String propertyKey=propertyKeyObject.toString(); if (propertyKey.startsWith("attribute")) attributeHit.setField(propertyKey, fullHit.getField(propertyKey)); diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java index 9e1808bb08e..ca87c0c1d46 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java @@ -107,6 +107,12 @@ public class JuniperSearcher extends Searcher { if (searchDefinitionField == null) continue; String searchDefinitionName = searchDefinitionField.toString(); + // TODO: Switch to iterate over indexes in the outer loop: + //for (Index index : indexFacts.getIndexes(searchDefinitionName())) { + // if (index.getDynamicSummary() || index.getHighlightSummary()) { + // insertTags(hit.buildHitField(index.getName(), true, true), bolding, index.getDynamicSummary()); + // } + //} for (String fieldName : hit.fields().keySet()) { Index index = indexFacts.getIndex(fieldName, searchDefinitionName); if (index.getDynamicSummary() || index.getHighlightSummary()) diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java index 7ff5b1b1a0e..d4cad7f1246 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java @@ -20,7 +20,7 @@ import com.yahoo.search.searchchain.Execution; * * May be extended to do quoting template sensitive. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class QuotingSearcher extends Searcher { diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java b/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java index 259b219b181..37906c8012f 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java @@ -13,7 +13,7 @@ import java.util.Map; /** * This is a helper class for rendering grouping results. * - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ public abstract class HitRenderer { @@ -50,9 +50,7 @@ public abstract class HitRenderer { if (hit instanceof RootGroup) { renderContinuation(Continuation.THIS_PAGE, ((RootGroup)hit).continuation(), writer); } - for (String label : hit.fieldKeys()) { - writer.openTag(TAG_OUTPUT).attribute(ATR_LABEL, label).content(hit.getField(label), false).closeTag(); - } + hit.forEachField((name, value) -> writer.openTag(TAG_OUTPUT).attribute(ATR_LABEL, name).content(value, false).closeTag()); } else if (hit instanceof HitList) { writer.openTag(TAG_HIT_LIST).attribute(ATR_LABEL, ((HitList)hit).getLabel()); renderContinuations(((HitList)hit).continuations(), writer); diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java index 97a5bdd72ca..3e6e82a5584 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java @@ -37,11 +37,7 @@ public class Organizer { sectionGroup.setQuery(result.hits().getQuery()); if (errors!=null && errors instanceof DefaultErrorHit) sectionGroup.add((DefaultErrorHit)errors); - for (Iterator<Map.Entry<String, Object>> it = result.hits().fieldIterator(); it.hasNext(); ) { - Map.Entry<String, Object> field = it.next(); - sectionGroup.setField(field.getKey(), field.getValue()); - } - + result.hits().forEachField((name, value) -> sectionGroup.setField(name, value)); result.setHits(sectionGroup); } diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java index c64927072e2..2768a546cd0 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java @@ -160,7 +160,7 @@ public class NGramSearcher extends Searcher { if (hit.isMeta()) continue; Object sddocname = hit.getField(Hit.SDDOCNAME_FIELD); if (sddocname == null) return; - for (String fieldName : hit.fieldKeys()) { + for (String fieldName : hit.fieldKeys()) { // TODO: Iterate over indexes instead Index index = session.getIndex(fieldName, sddocname.toString()); if (index.isNGram() && (index.getHighlightSummary() || index.getDynamicSummary())) { hit.setField(fieldName, recombineNGramsField(hit.getField(fieldName), index.getGramSize())); diff --git a/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java index bae1185d6a9..2d69a262f15 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java @@ -185,28 +185,19 @@ public final class DefaultRenderer extends AsynchronousSectionedRenderer<Result> private void renderHitFields(XMLWriter writer, Hit hit) { renderSyntheticRelevanceField(writer, hit); - for (Iterator<Map.Entry<String, Object>> it = hit.fieldIterator(); it.hasNext(); ) { - renderField(writer, hit, it); - } - } - - private void renderField(XMLWriter writer, Hit hit, Iterator<Map.Entry<String, Object>> it) { - renderGenericField(writer, hit, it.next()); + hit.forEachField((name, value) -> renderField(writer, name, value)); } - private void renderGenericField(XMLWriter writer, Hit hit, Map.Entry<String, Object> entry) { - String fieldName = entry.getKey(); - - // skip depending on hit type - if (fieldName.startsWith("$")) return; // Don't render fields that start with $ // TODO: Move to should render + private void renderField(XMLWriter writer, String name, Object value) { + if (name.startsWith("$")) return; - writeOpenFieldElement(writer, fieldName); - renderFieldContent(writer, hit, fieldName); + writeOpenFieldElement(writer, name); + renderFieldContent(writer, value); writeCloseFieldElement(writer); } - private void renderFieldContent(XMLWriter writer, Hit hit, String fieldName) { - writer.escapedContent(asXML(hit.getField(fieldName)), false); + private void renderFieldContent(XMLWriter writer, Object value) { + writer.escapedContent(asXML(value), false); } private String asXML(Object value) { diff --git a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java index 34b02b1bee8..6c7018317c3 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java @@ -14,7 +14,7 @@ import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.document.json.JsonWriter; -import com.yahoo.prelude.fastsearch.FastHit; +import com.yahoo.lang.MutableBoolean; import com.yahoo.processing.Response; import com.yahoo.processing.execution.Execution.Trace; import com.yahoo.processing.rendering.AsynchronousSectionedRenderer; @@ -49,6 +49,7 @@ import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; import java.io.StringWriter; +import java.io.UncheckedIOException; import java.math.BigDecimal; import java.math.BigInteger; import java.nio.charset.StandardCharsets; @@ -472,14 +473,14 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { return ! (hit instanceof DefaultErrorHit); } - private boolean fieldsStart(boolean hasFieldsField) throws IOException { - if (hasFieldsField) return true; + private void fieldsStart(MutableBoolean hasFieldsField) throws IOException { + if (hasFieldsField.get()) return; generator.writeObjectFieldStart(FIELDS); - return true; + hasFieldsField.set(true); } - private void fieldsEnd(boolean hasFieldsField) throws IOException { - if (!hasFieldsField) return; + private void fieldsEnd(MutableBoolean hasFieldsField) throws IOException { + if ( ! hasFieldsField.get()) return; generator.writeEndObject(); } @@ -508,39 +509,37 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { } private void renderAllFields(Hit hit) throws IOException { - boolean hasFieldsField = false; - - hasFieldsField |= renderTotalHitCount(hit, hasFieldsField); - hasFieldsField |= renderStandardFields(hit, hasFieldsField); + MutableBoolean hasFieldsField = new MutableBoolean(false); + renderTotalHitCount(hit, hasFieldsField); + renderStandardFields(hit, hasFieldsField); fieldsEnd(hasFieldsField); } - private boolean renderStandardFields(Hit hit, boolean initialHasFieldsField) throws IOException { - boolean hasFieldsField = initialHasFieldsField; - for (String fieldName : hit.fieldKeys()) { - if (!shouldRender(fieldName, hit)) continue; - - // We can't look at the size of fieldKeys() and know whether we need - // the fields object, as all fields may be hidden. - hasFieldsField |= fieldsStart(hasFieldsField); - renderField(fieldName, hit); - } - return hasFieldsField; + private void renderStandardFields(Hit hit, MutableBoolean hasFieldsField) { + hit.forEachField((name, value) -> { + try { + if (shouldRender(name, value)) { + fieldsStart(hasFieldsField); + renderField(name, value); + } + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + }); } - private boolean shouldRender(String fieldName, Hit hit) { + private boolean shouldRender(String name, Object value) { if (debugRendering) return true; - if (fieldName.startsWith(VESPA_HIDDEN_FIELD_PREFIX)) return false; + if (name.startsWith(VESPA_HIDDEN_FIELD_PREFIX)) return false; - Object field = hit.getField(fieldName); - - if (field instanceof CharSequence && ((CharSequence) field).length() == 0) return false; + if (value instanceof CharSequence && ((CharSequence) value).length() == 0) return false; // StringFieldValue cannot hold a null, so checking length directly is OK: - if (field instanceof StringFieldValue && ((StringFieldValue) field).getString().isEmpty()) return false; + if (value instanceof StringFieldValue && ((StringFieldValue) value).getString().isEmpty()) return false; - if (field instanceof NanNumber) return false; + if (value instanceof NanNumber) return false; return true; } @@ -607,17 +606,16 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { return (id instanceof RawBucketId ? Arrays.toString(((RawBucketId) id).getTo()) : id.getTo()).toString(); } - private boolean renderTotalHitCount(Hit hit, boolean hasFieldsField) throws IOException { - if ( ! (getRecursionLevel() == 1 && hit instanceof HitGroup)) return false; + private void renderTotalHitCount(Hit hit, MutableBoolean hasFieldsField) throws IOException { + if ( ! (getRecursionLevel() == 1 && hit instanceof HitGroup)) return; fieldsStart(hasFieldsField); generator.writeNumberField(TOTAL_COUNT, getResult().getTotalHitCount()); - return true; } - private void renderField(String fieldName, Hit hit) throws IOException { - generator.writeFieldName(fieldName); - renderFieldContents(hit.getField(fieldName)); + private void renderField(String name, Object value) throws IOException { + generator.writeFieldName(name); + renderFieldContents(value); } private void renderFieldContents(Object field) throws IOException { diff --git a/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java index dc72061e224..08599540cb1 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java @@ -271,25 +271,19 @@ public final class SyncDefaultRenderer extends Renderer { private void renderHitFields(XMLWriter writer, Hit hit) { renderSyntheticRelevanceField(writer, hit); - for (Iterator<Map.Entry<String, Object>> it = hit.fieldIterator(); it.hasNext(); ) { - renderField(writer, hit, it); - } + hit.forEachField((name, value) -> renderField(writer, name, value)); } - private void renderField(XMLWriter writer, Hit hit, Iterator<Map.Entry<String, Object>> it) { - Map.Entry<String, Object> entry = it.next(); - String fieldName = entry.getKey(); - - if ( ! shouldRenderField(hit, fieldName)) return; - if (fieldName.startsWith("$")) return; // Don't render fields that start with $ // TODO: Move to should render + private void renderField(XMLWriter writer, String name, Object value) { + if (name.startsWith("$")) return; - writeOpenFieldElement(writer, fieldName); - renderFieldContent(writer, hit, fieldName); + writeOpenFieldElement(writer, name); + renderFieldContent(writer, value); writeCloseFieldElement(writer); } - private void renderFieldContent(XMLWriter writer, Hit hit, String fieldName) { - writer.escapedContent(asXML(hit.getField(fieldName)), false); + private void renderFieldContent(XMLWriter writer, Object value) { + writer.escapedContent(asXML(value), false); } private String asXML(Object value) { @@ -304,10 +298,10 @@ public final class SyncDefaultRenderer extends Renderer { } private void renderSyntheticRelevanceField(XMLWriter writer, Hit hit) { - final String relevancyFieldName = "relevancy"; - final Relevance relevance = hit.getRelevance(); + String relevancyFieldName = "relevancy"; + Relevance relevance = hit.getRelevance(); - if (shouldRenderField(hit, relevancyFieldName) && relevance != null) { + if (relevance != null) { renderSimpleField(writer, relevancyFieldName, relevance); } } @@ -332,11 +326,6 @@ public final class SyncDefaultRenderer extends Renderer { writer.closeStartTag(); } - private boolean shouldRenderField(Hit hit, String relevancyFieldName) { - // skip depending on hit type - return true; - } - private void renderHitAttributes(XMLWriter writer, Hit hit) { writer.attribute(TYPE, hit.types().stream().collect(Collectors.joining(" "))); if (hit.getRelevance() != null) { diff --git a/container-search/src/main/java/com/yahoo/search/result/Hit.java b/container-search/src/main/java/com/yahoo/search/result/Hit.java index 15c148b7db7..f68916c8a68 100644 --- a/container-search/src/main/java/com/yahoo/search/result/Hit.java +++ b/container-search/src/main/java/com/yahoo/search/result/Hit.java @@ -21,6 +21,7 @@ import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.function.BiConsumer; import java.util.stream.Collectors; /** @@ -401,10 +402,30 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi /** Returns the name of the source creating this hit */ public String getSource() { return source; } + /** + * Receive a callback on the given object for each field in this hit. + * This is the most resource efficient way of traversing all the fields of a hit. + */ + public void forEachField(BiConsumer<String, Object> consumer) { + if (fields == null) return; + fields.forEach(consumer); + } + /** Returns the fields of this as a read-only map. This is more costly than fieldIterator() */ - // TODO Should it be deprecated ? public Map<String, Object> fields() { return getUnmodifiableFieldMap(); } + /** Returns a modifiable iterator over the fields of this */ + public Iterator<Map.Entry<String, Object>> fieldIterator() { return getFieldMap().entrySet().iterator(); } + + /** + * Returns the keys of the fields of this hit as a modifiable view. + * This follows the rules of key sets returned from maps: Key removals are reflected + * in the map, add and addAll is not supported. + */ + public Set<String> fieldKeys() { + return getFieldMap().keySet(); + } + /** Allocate room for the given number of fields to avoid resizing. */ public void reserve(int minSize) { getFieldMap(minSize); @@ -419,9 +440,6 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi return getFieldMap().put(key, value); } - /** Returns a modifiable iterator over the fields of this */ - public Iterator<Map.Entry<String, Object>> fieldIterator() { return getFieldMap().entrySet().iterator(); } - /** Returns a field value or null if not present */ public Object getField(String value) { return fields != null ? fields.get(value) : null; } @@ -439,15 +457,6 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi return getFieldMap().remove(field); } - /** - * Returns the keys of the fields of this hit as a modifiable view. - * This follows the rules of key sets returned from maps: Key removals are reflected - * in the map, add and addAll is not supported. - */ - public Set<String> fieldKeys() { - return getFieldMap().keySet(); - } - protected boolean hasField(String name) { return fields != null && fields.containsKey(name); } diff --git a/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java b/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java index 75c2865d0a5..14dce2f6342 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java +++ b/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java @@ -52,12 +52,7 @@ public class FieldFilter extends Searcher { for (Iterator<Hit> i = result.hits().unorderedDeepIterator(); i.hasNext();) { Hit h = i.next(); if (h.isMeta()) continue; - for (Iterator<Entry<String, Object>> fields = h.fieldIterator(); fields.hasNext();) { - Entry<String, Object> field = fields.next(); - if ( ! requestedFields.contains(field.getKey())) - fields.remove(); - } - + h.fieldKeys().retainAll(requestedFields); } } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java index 83871b559b4..1b74a6c5b29 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java @@ -31,6 +31,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class SlimeSummaryTestCase { @@ -286,6 +287,14 @@ public class SlimeSummaryTestCase { assertEquals(field.getValue(), expected.get(field.getKey())); } assertEquals(expected.size(), fieldIteratorFieldCount); + // field traverser + Map<String, Object> traversed = new HashMap<>(); + hit.forEachField((name, value) -> { + if (traversed.containsKey(name)) + fail("Multiple callbacks for " + name); + traversed.put(name, value); + }); + assertEquals(expected, traversed); } private byte[] emptySummary() { diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index defabc7463b..4a0075c0cf1 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -1069,7 +1069,6 @@ public class JsonRendererTestCase { r.getElapsedTime().add(t); renderer.setTimeSource(() -> 8L); String summary = render(r); - System.out.println(summary); assertEqualJson(expected, summary); } diff --git a/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java index f5097aec6e1..50313b630ea 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java @@ -20,9 +20,10 @@ import com.yahoo.search.searchchain.testutil.DocumentSourceSearcher; /** * Smoketest that we remove fields in a sane manner. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class FieldFilterTestCase { + private static final String FIELD_C = "c"; private static final String FIELD_B = "b"; private static final String FIELD_A = "a"; @@ -41,8 +42,7 @@ public class FieldFilterTestCase { DocumentSourceSearcher mockBackend = new DocumentSourceSearcher(); mockBackend.addResult(query, result); - searchChain = new Chain<Searcher>(new FieldFilter(), - mockBackend); + searchChain = new Chain<Searcher>(new FieldFilter(), mockBackend); context = Execution.Context.createContextStub(null); execution = new Execution(searchChain, context); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java index 9f6684ba84a..9b496526804 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java @@ -1,8 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; - import java.util.Optional; /** @@ -22,9 +20,6 @@ public interface ConfigServerApi extends AutoCloseable { <T> T delete(String path, Class<T> wantedReturnType); - /** Set or update the socket factory */ - void setSSLConnectionSocketFactory(SSLConnectionSocketFactory sslSocketFactory); - /** Close the underlying HTTP client and any threads this class might have started. */ @Override void close(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java index c29028c493e..25ec4fbd1dd 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java @@ -4,6 +4,10 @@ package com.yahoo.vespa.hosted.node.admin.configserver; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.config.provision.HostName; +import com.yahoo.vespa.athenz.api.AthenzService; +import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; +import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; +import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import org.apache.http.HttpHeaders; @@ -18,6 +22,8 @@ import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.entity.StringEntity; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -27,6 +33,8 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import static java.util.Collections.singleton; + /** * Retries request on config server a few times before giving up. Assumes that all requests should be sent with * content-type application/json @@ -38,83 +46,61 @@ public class ConfigServerApiImpl implements ConfigServerApi { private final ObjectMapper mapper = new ObjectMapper(); - private final List<URI> configServerHosts; + private final List<URI> configServers; - // If this instance is associated with asynchronous updating, this field is set - // to unregister from the updater at close() - private Optional<SslConnectionSocketFactoryUpdater> socketFactoryUpdater = Optional.empty(); + private Runnable runOnClose = () -> {}; /** - * The 'client' will be periodically re-created by clientRefresherScheduler if we provide keyStoreOptions - * or trustStoreOptions. This is needed because the key/trust stores are updated outside of node-admin, - * but we want to use the most recent store. + * The 'client' may be periodically re-created through calls to setSSLConnectionSocketFactory. * * The 'client' reference must be volatile because it is set and read in different threads, and visibility * of changes is only guaranteed for volatile variables. */ private volatile SelfCloseableHttpClient client; - /** - * Creates an api for talking to the config servers. - * - * <p>Registers with a SslConnectionSocketFactoryUpdater to keep the socket factory - * up to date as e.g. any client certificate expires (and unregisters at {@link #close()}) - */ - public static ConfigServerApiImpl create(ConfigServerInfo configServerInfo, - SslConnectionSocketFactoryUpdater updater) { - return createFor(updater, configServerInfo.getConfigServerUris()); + public static ConfigServerApiImpl create(ConfigServerInfo info, SiaIdentityProvider provider) { + return new ConfigServerApiImpl( + info.getConfigServerUris(), + new AthenzIdentityVerifier(singleton(info.getAthenzIdentity().get())), + provider); } - /** - * Creates an api for talking to a single config server. - * - * <p>Registers with a SslConnectionSocketFactoryUpdater to keep the socket factory - * up to date as e.g. any client certificate expires (and unregisters at {@link #close()}) - */ - public static ConfigServerApiImpl createFor(ConfigServerInfo configServerInfo, - SslConnectionSocketFactoryUpdater updater, - HostName configServer) { - URI uri = configServerInfo.getConfigServerUri(configServer.value()); - return createFor(updater, Collections.singletonList(uri)); + public static ConfigServerApiImpl createFor(ConfigServerInfo info, + SiaIdentityProvider provider, + HostName configServerHostname) { + return new ConfigServerApiImpl( + Collections.singleton(info.getConfigServerUri(configServerHostname.value())), + new AthenzIdentityVerifier(singleton(info.getAthenzIdentity().get())), + provider); } - /** - * Creates an api for talking to the config servers with a fixed socket factory. - * - * <p>This may be used to avoid requiring background certificate signing requests (CSR) - * against the config server when client validation is enabled in the config server. - */ - public static ConfigServerApiImpl createWithSocketFactory( - List<URI> configServerHosts, - SSLConnectionSocketFactory socketFactory) { - return new ConfigServerApiImpl(configServerHosts, socketFactory); - } + private ConfigServerApiImpl(Collection<URI> configServers, + HostnameVerifier verifier, + SiaIdentityProvider identityProvider) { + this(configServers, createClient(identityProvider.getIdentitySslContext(), verifier)); - static ConfigServerApiImpl createForTestingWithClient(List<URI> configServerHosts, - SelfCloseableHttpClient client) { - return new ConfigServerApiImpl(configServerHosts, client); + // Register callback for updates to the SSLContext + ServiceIdentityProvider.Listener listener = (SSLContext sslContext, AthenzService identity) -> { + this.client = createClient(sslContext, verifier); + }; + identityProvider.addIdentityListener(listener); + this.runOnClose = () -> identityProvider.removeIdentityListener(listener); } - private static ConfigServerApiImpl createFor(SslConnectionSocketFactoryUpdater updater, - List<URI> configServers) { - ConfigServerApiImpl api = new ConfigServerApiImpl( - configServers, - // Passing null here (only) works because startSocketFactoryUpdating will - // set the client. This avoids an unnecessary allocation of a client. - (SelfCloseableHttpClient) null); - api.startSocketFactoryUpdating(updater); - assert api.client != null; - return api; + private ConfigServerApiImpl(Collection<URI> configServers, SelfCloseableHttpClient client) { + this.configServers = randomizeConfigServerUris(configServers); + this.client = client; } - private ConfigServerApiImpl(Collection<URI> configServerUris, - SSLConnectionSocketFactory sslConnectionSocketFactory) { - this(configServerUris, new SelfCloseableHttpClient(sslConnectionSocketFactory)); + public static ConfigServerApiImpl createForTestingWithSocketFactory( + List<URI> configServerHosts, + SSLConnectionSocketFactory socketFactory) { + return new ConfigServerApiImpl(configServerHosts, new SelfCloseableHttpClient(socketFactory)); } - private ConfigServerApiImpl(Collection<URI> configServerHosts, SelfCloseableHttpClient client) { - this.configServerHosts = randomizeConfigServerUris(configServerHosts); - this.client = client; + static ConfigServerApiImpl createForTestingWithClient(List<URI> configServerHosts, + SelfCloseableHttpClient client) { + return new ConfigServerApiImpl(configServerHosts, client); } interface CreateRequest { @@ -123,7 +109,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { private <T> T tryAllConfigServers(CreateRequest requestFactory, Class<T> wantedReturnType) { Exception lastException = null; - for (URI configServer : configServerHosts) { + for (URI configServer : configServers) { final CloseableHttpResponse response; try { response = client.execute(requestFactory.createRequest(configServer)); @@ -163,7 +149,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { } throw new RuntimeException("All requests against the config servers (" - + configServerHosts + ") failed, last as follows:", lastException); + + configServers + ") failed, last as follows:", lastException); } @Override @@ -210,13 +196,20 @@ public class ConfigServerApiImpl implements ConfigServerApi { }, wantedReturnType); } + @Override + public void close() { + runOnClose.run(); + client.close(); + } + private void setContentTypeToApplicationJson(HttpRequestBase request) { request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json"); } - @Override - public void setSSLConnectionSocketFactory(SSLConnectionSocketFactory sslSocketFactory) { - this.client = new SelfCloseableHttpClient(sslSocketFactory); + private static SelfCloseableHttpClient createClient( + SSLContext sslContext, HostnameVerifier configServerVerifier) { + return new SelfCloseableHttpClient( + new SSLConnectionSocketFactory(sslContext, configServerVerifier)); } // Shuffle config server URIs to balance load @@ -225,20 +218,4 @@ public class ConfigServerApiImpl implements ConfigServerApi { Collections.shuffle(shuffledConfigServerHosts); return shuffledConfigServerHosts; } - - private void startSocketFactoryUpdating(SslConnectionSocketFactoryUpdater updater) { - updater.registerConfigServerApi(this); - this.socketFactoryUpdater = Optional.of(updater); - } - - private void stopSocketFactoryUpdating() { - this.socketFactoryUpdater.ifPresent(updater -> updater.unregisterConfigServerApi(this)); - this.socketFactoryUpdater = Optional.empty(); - } - - @Override - public void close() { - stopSocketFactoryUpdating(); - client.close(); - } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java index 7c15f94852b..4a2496f4d3e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver; -import com.yahoo.config.provision.HostName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.configserver.state.State; @@ -18,8 +17,8 @@ public interface ConfigServerClients { /** Get handle to /orchestrator/v1/ REST API */ Orchestrator orchestrator(); - /** Get handle to the /state/v1 REST API of the specified config server */ - default State state(HostName hostname) { throw new java.lang.UnsupportedOperationException(); } + /** Get handle to the /state/v1 REST API */ + default State state() { throw new UnsupportedOperationException(); } void stop(); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java index 07dbc8aef9c..6c982bfa71c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java @@ -1,9 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver; -import com.yahoo.config.provision.HostName; -import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; -import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.RealNodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; @@ -11,38 +8,26 @@ import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorI import com.yahoo.vespa.hosted.node.admin.configserver.state.State; import com.yahoo.vespa.hosted.node.admin.configserver.state.StateImpl; -import java.util.concurrent.ConcurrentHashMap; - /** + * {@link ConfigServerClients} using the default implementation for the various clients, + * and backed by a {@link ConfigServerApi}. + * * @author freva */ public class RealConfigServerClients implements ConfigServerClients { - - private final SslConnectionSocketFactoryUpdater updater; - - // ConfigServerApi that talks to all config servers private final ConfigServerApi configServerApi; - private final NodeRepository nodeRepository; private final Orchestrator orchestrator; - private final ConcurrentHashMap<HostName, State> states = new ConcurrentHashMap<>(); - private final ConfigServerInfo configServerInfo; + private final State state; /** - * Create config server clients against a real (remote) config server. - * - * If a client certificate is required, one will be requested from the config server - * and kept up to date. On failure, this constructor will throw an exception and - * the caller may retry later. + * @param configServerApi the backend API to use - will be closed at {@link #stop()}. */ - public RealConfigServerClients(SiaIdentityProvider identityProvider, ConfigServerInfo info) { - this.configServerInfo = info; - updater = SslConnectionSocketFactoryUpdater.createAndRefreshKeyStoreIfNeeded(identityProvider, info.getAthenzIdentity().get()); - - configServerApi = ConfigServerApiImpl.create(info, updater); - + public RealConfigServerClients(ConfigServerApi configServerApi) { + this.configServerApi = configServerApi; nodeRepository = new RealNodeRepository(configServerApi); orchestrator = new OrchestratorImpl(configServerApi); + state = new StateImpl(configServerApi); } @Override @@ -56,21 +41,12 @@ public class RealConfigServerClients implements ConfigServerClients { } @Override - public State state(HostName hostname) { - return states.computeIfAbsent(hostname, this::createState); + public State state() { + return state; } @Override public void stop() { - updater.unregisterConfigServerApi(configServerApi); configServerApi.close(); - updater.close(); - } - - private State createState(HostName hostname) { - ConfigServerApi configServerApi = ConfigServerApiImpl.createFor( - configServerInfo, updater, hostname); - - return new StateImpl(configServerApi); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdater.java deleted file mode 100644 index 1888749b998..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdater.java +++ /dev/null @@ -1,104 +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.hosted.node.admin.configserver; - -import com.yahoo.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.api.AthenzService; -import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; -import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; -import com.yahoo.vespa.athenz.tls.SslContextBuilder; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import java.util.HashSet; -import java.util.Set; - -import static java.util.Collections.singleton; - -/** - * Responsible for updating {@link SSLConnectionSocketFactory} on {@link ConfigServerApiImpl} asynchronously - * using SIA based certificates (through {@link SiaIdentityProvider}). - * - * @author bjorncs - * @author hakon - */ -public class SslConnectionSocketFactoryUpdater implements AutoCloseable { - - private final Object monitor = new Object(); - private final HostnameVerifier configServerHostnameVerifier; - private final SiaIdentityProvider sia; - - private final Set<ConfigServerApi> configServerApis = new HashSet<>(); - private SSLConnectionSocketFactory socketFactory; - - /** - * Creates an updater with valid initial {@link SSLConnectionSocketFactory} - * - * @throws RuntimeException if e.g. key store options have been specified, but was unable - * create a create a key store with a valid certificate - */ - public static SslConnectionSocketFactoryUpdater createAndRefreshKeyStoreIfNeeded(SiaIdentityProvider identityProvider, - AthenzIdentity configserverIdentity) { - return new SslConnectionSocketFactoryUpdater(identityProvider, createHostnameVerifier(configserverIdentity)); - } - - SslConnectionSocketFactoryUpdater(SiaIdentityProvider siaIdentityProvider, - HostnameVerifier configServerHostnameVerifier) { - this.configServerHostnameVerifier = configServerHostnameVerifier; - this.sia = siaIdentityProvider; - if (siaIdentityProvider != null) { - siaIdentityProvider.addIdentityListener(this::updateSocketFactory); - socketFactory = createSocketFactory(siaIdentityProvider.getIdentitySslContext()); - } else { - socketFactory = createDefaultSslConnectionSocketFactory(); - } - } - - private void updateSocketFactory(SSLContext sslContext, AthenzService identity) { - synchronized (monitor) { - socketFactory = createSocketFactory(sslContext); - configServerApis.forEach(api -> api.setSSLConnectionSocketFactory(socketFactory)); - } - } - - public SSLConnectionSocketFactory getCurrentSocketFactory() { - synchronized (monitor) { - return socketFactory; - } - } - - /** Register a {@link ConfigServerApi} whose {@link SSLConnectionSocketFactory} will be kept up to date */ - public void registerConfigServerApi(ConfigServerApi configServerApi) { - synchronized (monitor) { - configServerApi.setSSLConnectionSocketFactory(socketFactory); - configServerApis.add(configServerApi); - } - } - - public void unregisterConfigServerApi(ConfigServerApi configServerApi) { - synchronized (monitor) { - configServerApis.remove(configServerApi); - } - } - - @Override - public void close() { - if (sia != null) { - sia.deconstruct(); - } - } - - private SSLConnectionSocketFactory createSocketFactory(SSLContext sslContext) { - return new SSLConnectionSocketFactory(sslContext, configServerHostnameVerifier); - } - - private SSLConnectionSocketFactory createDefaultSslConnectionSocketFactory() { - SSLContext sslContext = new SslContextBuilder().build(); - return createSocketFactory(sslContext); - } - - private static HostnameVerifier createHostnameVerifier(AthenzIdentity identity) { - return new AthenzIdentityVerifier(singleton(identity)); - } - -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java index ac1b1cbb600..4bac2cdfbf2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java @@ -10,6 +10,8 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.component.DockerAdminComponent; import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApiImpl; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerClients; import com.yahoo.vespa.hosted.node.admin.configserver.RealConfigServerClients; @@ -22,8 +24,9 @@ public class NodeAdminProvider implements Provider<NodeAdminStateUpdater> { Docker docker, MetricReceiverWrapper metricReceiver, ClassLocking classLocking) { - ConfigServerClients clients = - new RealConfigServerClients(identityProvider, new ConfigServerInfo(configServerConfig)); + ConfigServerInfo configServerInfo = new ConfigServerInfo(configServerConfig); + ConfigServerApi configServerApi = ConfigServerApiImpl.create(configServerInfo, identityProvider); + ConfigServerClients clients = new RealConfigServerClients(configServerApi); dockerAdmin = new DockerAdminComponent(configServerConfig, identityProvider, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdaterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdaterTest.java deleted file mode 100644 index 6c6f8cf40fe..00000000000 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdaterTest.java +++ /dev/null @@ -1,31 +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.hosted.node.admin.configserver; - -import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; -import com.yahoo.vespa.athenz.tls.SslContextBuilder; -import org.junit.Test; - -import static org.junit.Assert.assertNotNull; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -/** - * @author bjorncs - */ -public class SslConnectionSocketFactoryUpdaterTest { - - @Test - public void creates_default_ssl_connection_factory_when_no_sia_provided() { - SslConnectionSocketFactoryUpdater updater = - new SslConnectionSocketFactoryUpdater(null, (hostname, session) -> true); - assertNotNull(updater.getCurrentSocketFactory()); - } - - @Test - public void creates_ssl_connection_factory_when_sia_provided() { - SiaIdentityProvider sia = mock(SiaIdentityProvider.class); - when(sia.getIdentitySslContext()).thenReturn(new SslContextBuilder().build()); - SslConnectionSocketFactoryUpdater updater = new SslConnectionSocketFactoryUpdater(sia, (hostname, session) -> true); - assertNotNull(updater.getCurrentSocketFactory()); - } -}
\ No newline at end of file diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java index ae5ba9681e6..045b1116740 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java @@ -67,7 +67,7 @@ public class RealNodeRepositoryTest { try { final int port = findRandomOpenPort(); container = JDisc.fromServicesXml(ContainerConfig.servicesXmlV2(port), Networking.enable); - ConfigServerApi configServerApi = ConfigServerApiImpl.createWithSocketFactory( + ConfigServerApi configServerApi = ConfigServerApiImpl.createForTestingWithSocketFactory( Collections.singletonList(URI.create("http://127.0.0.1:" + port)), SSLConnectionSocketFactory.getSocketFactory()); waitForJdiscContainerToServe(configServerApi); diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 2cb23965450..8de03411ea3 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -52,8 +52,9 @@ struct StupidMetaStore : search::IDocumentMetaStore { void foreach(const search::IGidToLidMapperVisitor &) const override { } }; -FeatureSet::SP findFeatureSet(const DocsumRequest &req, - MatchToolsFactory &mtf, bool summaryFeatures) { +FeatureSet::SP +findFeatureSet(const DocsumRequest &req, MatchToolsFactory &mtf, bool summaryFeatures) +{ std::vector<uint32_t> docs; for (size_t i = 0; i < req.hits.size(); ++i) { if (req.hits[i].docid != search::endDocId) { @@ -300,8 +301,9 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl coverage.degradeTimeout(); LOG(debug, "soft doomed, degraded from timeout covered = %lu", coverage.getCovered()); } - LOG(debug, "numThreadsPerSearch = %zu. Configured = %d, estimated hits=%d, totalHits=%ld", - numThreadsPerSearch, _rankSetup->getNumThreadsPerSearch(), estHits, reply->totalHitCount); + LOG(debug, "numThreadsPerSearch = %zu. Configured = %d, estimated hits=%d, totalHits=%ld , rankprofile=%s", + numThreadsPerSearch, _rankSetup->getNumThreadsPerSearch(), estHits, reply->totalHitCount, + request.ranking.c_str()); } total_matching_time.stop(); my_stats.queryCollateralTime(total_matching_time.elapsed().sec() - my_stats.queryLatencyAvg()); @@ -311,8 +313,11 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl std::lock_guard<std::mutex> guard(_statsLock); _stats.add(my_stats); if (my_stats.softDoomed()) { - LOG(info, "Triggered softtimeout limit=%1.3f and duration=%1.3f", softLimit.sec(), duration.sec()); + double old = _stats.softDoomFactor(); _stats.updatesoftDoomFactor(request.getTimeout(), softLimit, duration); + LOG(info, "Triggered softtimeout factor adjustment. limit=%1.3f and duration=%1.3f, rankprofile=%s" + ", factor adjusted from %1.3f to %1.3f", + softLimit.sec(), duration.sec(), request.ranking.c_str(), old, _stats.softDoomFactor()); } } return reply; diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.h b/searchcore/src/vespa/searchcore/proton/matching/matcher.h index 8415f659473..4786e9f7f7c 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.h +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.h @@ -18,23 +18,19 @@ #include <vespa/vespalib/util/thread_bundle.h> #include <mutex> -namespace search { -namespace grouping { +namespace search::grouping { class GroupingContext; class GroupingSession; -} // namespace search::grouping; -namespace index { class Schema; } -namespace attribute { class IAttributeContext; } -namespace engine { +} +namespace search::index { class Schema; } +namespace search::attribute { class IAttributeContext; } +namespace search::engine { class Request; class SearchRequest; class DocsumRequest; class SearchReply; } - -class IDocumentMetaStore; - -} // namespace search +namespace search { class IDocumentMetaStore; } namespace proton::matching { @@ -59,16 +55,13 @@ private: uint32_t _distributionKey; search::FeatureSet::SP - getFeatureSet(const search::engine::DocsumRequest & req, - ISearchContext & searchCtx, + getFeatureSet(const search::engine::DocsumRequest & req, ISearchContext & searchCtx, search::attribute::IAttributeContext & attrCtx, - SessionManager &sessionMgr, - bool summaryFeatures); + SessionManager &sessionMgr, bool summaryFeatures); std::unique_ptr<search::engine::SearchReply> - handleGroupingSession( - SessionManager &sessionMgr, - search::grouping::GroupingContext & groupingContext, - std::unique_ptr<search::grouping::GroupingSession> gs); + handleGroupingSession(SessionManager &sessionMgr, + search::grouping::GroupingContext & groupingContext, + std::unique_ptr<search::grouping::GroupingSession> gs); size_t computeNumThreadsPerSearch(search::queryeval::Blueprint::HitEstimate hits, const search::fef::Properties & rankProperties) const; @@ -91,12 +84,9 @@ public: * @param props ranking configuration * @param clock used for timeout handling **/ - Matcher(const search::index::Schema &schema, - const search::fef::Properties &props, - const vespalib::Clock &clock, - QueryLimiter &queryLimiter, - const IConstantValueRepo &constantValueRepo, - uint32_t distributionKey); + Matcher(const search::index::Schema &schema, const search::fef::Properties &props, + const vespalib::Clock &clock, QueryLimiter &queryLimiter, + const IConstantValueRepo &constantValueRepo, uint32_t distributionKey); const search::fef::IIndexEnvironment &get_index_env() const { return _indexEnv; } @@ -112,8 +102,7 @@ public: * function is exposed for testing purposes. **/ std::unique_ptr<MatchToolsFactory> - create_match_tools_factory(const search::engine::Request &request, - ISearchContext &searchContext, + create_match_tools_factory(const search::engine::Request &request, ISearchContext &searchContext, search::attribute::IAttributeContext &attrContext, const search::IDocumentMetaStore &metaStore, const search::fef::Properties &feature_overrides) const; @@ -130,12 +119,9 @@ public: * @param metaStore the document meta store used to map from lid to gid **/ std::unique_ptr<search::engine::SearchReply> - match(const search::engine::SearchRequest &request, - vespalib::ThreadBundle &threadBundle, - ISearchContext &searchContext, - search::attribute::IAttributeContext &attrContext, - SessionManager &sessionManager, - const search::IDocumentMetaStore &metaStore, + match(const search::engine::SearchRequest &request, vespalib::ThreadBundle &threadBundle, + ISearchContext &searchContext, search::attribute::IAttributeContext &attrContext, + SessionManager &sessionManager, const search::IDocumentMetaStore &metaStore, SearchSession::OwnershipBundle &&owned_objects); /** @@ -149,10 +135,8 @@ public: * @return calculated summary features. **/ search::FeatureSet::SP - getSummaryFeatures(const search::engine::DocsumRequest & req, - ISearchContext & searchCtx, - search::attribute::IAttributeContext & attrCtx, - SessionManager &sessionManager); + getSummaryFeatures(const search::engine::DocsumRequest & req, ISearchContext & searchCtx, + search::attribute::IAttributeContext & attrCtx, SessionManager &sessionManager); /** * Perform matching for the documents in the given docsum request @@ -165,10 +149,8 @@ public: * @return calculated rank features. **/ search::FeatureSet::SP - getRankFeatures(const search::engine::DocsumRequest & req, - ISearchContext & searchCtx, - search::attribute::IAttributeContext & attrCtx, - SessionManager &sessionManager); + getRankFeatures(const search::engine::DocsumRequest & req, ISearchContext & searchCtx, + search::attribute::IAttributeContext & attrCtx, SessionManager &sessionManager); /** * @return true if this rankprofile has summary-features enabled diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java b/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java new file mode 100644 index 00000000000..b009ffdd2b6 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java @@ -0,0 +1,24 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.lang; + +/** + * A mutable boolean + * + * @author bratseth + */ +public class MutableBoolean { + + private boolean value; + + public MutableBoolean(boolean value) { + this.value = value; + } + + public boolean get() { return value; } + + public void set(boolean value) { this.value = value; } + + @Override + public String toString() { return Boolean.toString(value); } + +} diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java b/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java index 34e928db674..a988a3f6fa2 100644 --- a/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java +++ b/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java @@ -30,4 +30,7 @@ public class MutableInteger { return value; } + @Override + public String toString() { return Integer.toString(value); } + } diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java b/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java index e0e4a0828a9..36d3a8a6343 100644 --- a/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java +++ b/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java @@ -30,4 +30,7 @@ public class MutableLong { return value; } + @Override + public String toString() { return Long.toString(value); } + } |