diff options
| author | Jon Bratseth <bratseth@gmail.com> | 2022-08-17 09:57:44 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-08-17 09:57:44 +0200 |
| commit | 224b13638e794b01c741d5f345b0adfd65f7b66a (patch) | |
| tree | c4ab5bb92f4ab01f0b6c634c35deedd80568c7fe | |
| parent | 02d0725e489540eca682c453dbf254fc63992c6a (diff) | |
| parent | 5f57dc00592552a29cdab9c535ff05b03db08a5b (diff) | |
Merge branch 'master' into bratseth/defaultIndex-in-state
72 files changed, 601 insertions, 799 deletions
diff --git a/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp b/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp index 4af30a5d9d3..f9a4d5b43cc 100644 --- a/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp +++ b/ann_benchmark/src/vespa/ann_benchmark/vespa_ann_benchmark.cpp @@ -10,7 +10,7 @@ #include <vespa/searchcommon/attribute/config.h> #include <vespa/eval/eval/value.h> #include <vespa/vespalib/test/insertion_operators.h> -#include <ostream> +#include <iostream> #include <sstream> #include <limits> diff --git a/application/pom.xml b/application/pom.xml index 34bf1b56c0d..e2176d4deac 100644 --- a/application/pom.xml +++ b/application/pom.xml @@ -86,6 +86,10 @@ <artifactId>org.apache.felix.framework</artifactId> </dependency> <dependency> + <groupId>org.apache.felix</groupId> + <artifactId>org.apache.felix.log</artifactId> + </dependency> + <dependency> <groupId>org.apache.opennlp</groupId> <artifactId>opennlp-tools</artifactId> </dependency> diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 1a83afd1be5..a30cbb86299 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -189,6 +189,7 @@ <include>org.apache.commons:commons-compress:1.21:jar:test</include> <include>org.apache.commons:commons-math3:3.6.1:jar:test</include> <include>org.apache.felix:org.apache.felix.framework:[${felix.version}]:jar:test</include> + <include>org.apache.felix:org.apache.felix.log:1.0.1:jar:test</include> <include>org.apache.httpcomponents.client5:httpclient5:${httpclient5.version}:jar:test</include> <include>org.apache.httpcomponents.core5:httpcore5:${httpclient5.version}:jar:test</include> <include>org.apache.httpcomponents.core5:httpcore5-h2:${httpclient5.version}:jar:test</include> @@ -227,6 +228,8 @@ <include>org.junit.vintage:junit-vintage-engine:[${junit5.version}]:jar:test</include> <include>org.lz4:lz4-java:[${org.lz4.version}]:jar:test</include> <include>org.opentest4j:opentest4j:1.2.0:jar:test</include> + <include>org.osgi:org.osgi.compendium:4.1.0:jar:test</include> + <include>org.osgi:org.osgi.core:4.1.0:jar:test</include> <include>xerces:xercesImpl:2.12.1:jar:test</include> </allowed> </enforceDependencies> diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index 4097810b633..29887666a1b 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -22,7 +22,6 @@ import com.yahoo.vespa.clustercontroller.core.status.ClusterStateRequestHandler; import com.yahoo.vespa.clustercontroller.core.status.LegacyIndexPageRequestHandler; import com.yahoo.vespa.clustercontroller.core.status.LegacyNodePageRequestHandler; import com.yahoo.vespa.clustercontroller.core.status.NodeHealthRequestHandler; -import com.yahoo.vespa.clustercontroller.core.status.RunDataExtractor; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageResponse; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageServer; import com.yahoo.vespa.clustercontroller.core.status.statuspage.StatusPageServerInterface; @@ -101,13 +100,6 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta // deriving is done. private Set<String> configuredBucketSpaces = Collections.emptySet(); - private final RunDataExtractor dataExtractor = new RunDataExtractor() { - @Override - public FleetControllerOptions getOptions() { return options; } - @Override - public ContentCluster getCluster() { return cluster; } - }; - public FleetController(FleetControllerContext context, Timer timer, EventLog eventLog, @@ -155,8 +147,7 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta new ClusterStateRequestHandler(stateVersionTracker)); this.statusRequestRouter.addHandler( "^/$", - new LegacyIndexPageRequestHandler(timer, cluster, masterElectionHandler, stateVersionTracker, eventLog, - timer.getCurrentTimeInMillis(), dataExtractor)); + new LegacyIndexPageRequestHandler(timer, cluster, masterElectionHandler, stateVersionTracker, eventLog, options)); propagateOptions(); } @@ -1153,8 +1144,6 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta @Override public FleetController getFleetController() { return FleetController.this; } @Override - public SlobrokListener getNodeAddedOrRemovedListener() { return FleetController.this; } - @Override public NodeListener getNodeStateUpdateListener() { return FleetController.this; } }; @@ -1240,4 +1229,5 @@ public class FleetController implements NodeListener, SlobrokListener, SystemSta public EventLog getEventLog() { return eventLog; } + } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java index 01b8ed48c80..408c10e81b1 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/DatabaseHandler.java @@ -6,14 +6,12 @@ import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.ClusterStateBundle; import com.yahoo.vespa.clustercontroller.core.ContentCluster; -import com.yahoo.vespa.clustercontroller.core.FleetControllerContext; import com.yahoo.vespa.clustercontroller.core.FleetController; +import com.yahoo.vespa.clustercontroller.core.FleetControllerContext; import com.yahoo.vespa.clustercontroller.core.NodeInfo; import com.yahoo.vespa.clustercontroller.core.Timer; -import com.yahoo.vespa.clustercontroller.core.listeners.SlobrokListener; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; import org.apache.zookeeper.KeeperException; - import java.io.PrintWriter; import java.io.StringWriter; import java.util.Map; @@ -32,7 +30,6 @@ public class DatabaseHandler { public interface DatabaseContext { ContentCluster getCluster(); FleetController getFleetController(); - SlobrokListener getNodeAddedOrRemovedListener(); NodeListener getNodeStateUpdateListener(); } @@ -179,7 +176,7 @@ public class DatabaseHandler { private boolean usingZooKeeper() { return (zooKeeperAddress != null); } - private void connect(ContentCluster cluster, long currentTime) { + private void connect(long currentTime) { try { lastZooKeeperConnectionAttempt = currentTime; synchronized (databaseMonitor) { @@ -251,7 +248,7 @@ public class DatabaseHandler { return false; // Not time to attempt connection yet. } didWork = true; - connect(databaseContext.getCluster(), currentTime); + connect(currentTime); } try { synchronized (databaseMonitor) { @@ -344,7 +341,7 @@ public class DatabaseHandler { return didWork; } - public void setMasterVote(DatabaseContext databaseContext, int wantedMasterCandidate) throws InterruptedException { + public void setMasterVote(DatabaseContext databaseContext, int wantedMasterCandidate) { fleetControllerContext.log(logger, Level.FINE, () -> "Checking if master vote has been updated and need to be stored."); // Schedule a write if one of the following is true: // - There is already a pending vote to be written, that may have been written already without our knowledge diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java index 96dc114c734..4f20b3d0cdc 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/status/LegacyIndexPageRequestHandler.java @@ -36,22 +36,21 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa private final StateVersionTracker stateVersionTracker; private final EventLog eventLog; private final long startedTime; - private final RunDataExtractor data; + private final FleetControllerOptions options; public LegacyIndexPageRequestHandler(Timer timer, ContentCluster cluster, MasterElectionHandler masterElectionHandler, StateVersionTracker stateVersionTracker, EventLog eventLog, - long startedTime, - RunDataExtractor data) { + FleetControllerOptions options) { this.timer = timer; this.cluster = cluster; this.masterElectionHandler = masterElectionHandler; this.stateVersionTracker = stateVersionTracker; this.eventLog = eventLog; - this.startedTime = startedTime; - this.data = data; + this.startedTime = timer.getCurrentTimeInMillis(); + this.options = options; } @Override @@ -63,7 +62,7 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa response.setContentType("text/html"); StringBuilder content = new StringBuilder(); content.append("<!-- Answer to request " + request + " -->\n"); - response.writeHtmlHeader(content, cluster.getName() + " Cluster Controller " + data.getOptions().fleetControllerIndex + " Status Page"); + response.writeHtmlHeader(content, cluster.getName() + " Cluster Controller " + options.fleetControllerIndex + " Status Page"); content.append("<p><font size=\"-1\">") .append(" [ <a href=\"#config\">Current config</a>") .append(" | <a href=\"#clusterstates\">Cluster states</a>") @@ -72,19 +71,19 @@ public class LegacyIndexPageRequestHandler implements StatusPageServer.RequestHa content.append("<table><tr><td>UTC time when creating this page:</td><td align=\"right\">").append(RealTimer.printDateNoMilliSeconds(currentTime, tz)).append("</td></tr>"); //content.append("<tr><td>Fleetcontroller version:</td><td align=\"right\">" + Vtag.V_TAG_PKG + "</td></tr/>"); content.append("<tr><td>Cluster controller uptime:</td><td align=\"right\">" + RealTimer.printDuration(currentTime - startedTime) + "</td></tr></table>"); - if (masterElectionHandler.isAmongNthFirst(data.getOptions().stateGatherCount)) { + if (masterElectionHandler.isAmongNthFirst(options.stateGatherCount)) { // Table overview of all the nodes - writeHtmlState(cluster, content, timer, stateVersionTracker, data.getOptions(), eventLog); + writeHtmlState(cluster, content, timer, stateVersionTracker, options, eventLog); // Current cluster state and cluster state history writeHtmlState(stateVersionTracker, content); } else { // Overview of current config - data.getOptions().writeHtmlState(content); + options.writeHtmlState(content); } // State of master election - masterElectionHandler.writeHtmlState(content, data.getOptions().stateGatherCount); + masterElectionHandler.writeHtmlState(content, options.stateGatherCount); // Overview of current config - data.getOptions().writeHtmlState(content); + options.writeHtmlState(content); // Event log eventLog.writeHtmlState(content, null); response.writeHtmlFooter(content, ""); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java index 4d8c7799c58..c07a29b3c1e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/DatabaseHandlerTest.java @@ -9,14 +9,12 @@ import com.yahoo.vespa.clustercontroller.core.database.Database; import com.yahoo.vespa.clustercontroller.core.database.DatabaseFactory; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; -import com.yahoo.vespa.clustercontroller.core.listeners.SlobrokListener; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.MockitoAnnotations; - import java.util.Map; import java.util.TreeMap; @@ -71,11 +69,6 @@ public class DatabaseHandlerTest { } @Override - public SlobrokListener getNodeAddedOrRemovedListener() { - return null; - } - - @Override public NodeListener getNodeStateUpdateListener() { return null; } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java index 371a57e08c0..c39a5c52836 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java @@ -7,7 +7,6 @@ import com.yahoo.jrt.StringValue; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; -import com.yahoo.jrt.slobrok.api.BackOffPolicy; import com.yahoo.jrt.slobrok.server.Slobrok; import com.yahoo.log.LogSetup; import com.yahoo.vdslib.distribution.ConfiguredNode; @@ -42,7 +41,6 @@ import java.util.TreeMap; import java.util.TreeSet; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -83,14 +81,6 @@ public abstract class FleetControllerTest implements Waiter { LogSetup.initVespaLogging("fleetcontroller"); } - static class BackOff implements BackOffPolicy { - private int counter = 0; - public void reset() { counter = 0; } - public double get() { ++counter; return 0.01; } - public boolean shouldWarn(double v) { return ((counter % 1000) == 10); } - public boolean shouldInform(double v) { return false; } - } - public static class CleanupZookeeperLogsOnSuccess implements TestWatcher { public CleanupZookeeperLogsOnSuccess() {} @@ -334,116 +324,6 @@ public abstract class FleetControllerTest implements Waiter { fleetController.waitForCompleteCycle(timeout); } - private static class ExpectLine { - Pattern regex; - int matchedCount = 0; - int minCount = 1; - int maxCount = 1; - boolean repeatable() { return (maxCount == 0 || maxCount > matchedCount); } - boolean optional() { return (matchedCount >= minCount); } - - boolean matches(String event) { - if (event == null) return false; - boolean m = regex.matcher(event).matches(); - if (m) ++matchedCount; - return m; - } - - ExpectLine(String pattern) { - if (pattern.charAt(0) == '?') { - pattern = pattern.substring(1); - minCount = 0; - } else if (pattern.charAt(0) == '*') { - pattern = pattern.substring(1); - minCount = 0; - maxCount = 0; - } else if (pattern.charAt(0) == '+') { - pattern = pattern.substring(1); - maxCount = 0; - } - regex = Pattern.compile(pattern); - } - - public String toString() { - return "{"+minCount+","+maxCount+"}("+matchedCount+") " + regex; - } - } - - /** - * Verifies that node event list is equal to some expected value. - * The format of the expected values is as follows: - * <ul> - * <li>Each line in the exp string specifies a pattern to match one or more events. - * <li>A line starting with ? * or + means that the line can match 0 or 1, 0 or more or 1 or more respectively. - * <li>The rest of the line is a regular expression. - * </ul> - */ - protected void verifyNodeEvents(Node n, String exp) { - List<NodeEvent> events = fleetController.getNodeEvents(n); - String[] expectLines = exp.split("\n"); - List<ExpectLine> expected = new ArrayList<>(); - for (String line : expectLines) { - expected.add(new ExpectLine(line)); - } - - boolean mismatch = false; - StringBuilder errors = new StringBuilder(); - - int gotno = 0; - int expno = 0; - - while (gotno < events.size() || expno < expected.size()) { - String eventLine = null; - if (gotno < events.size()) { - NodeEvent e = events.get(gotno); - eventLine = e.toString(); - } - - ExpectLine pattern = null; - if (expno < expected.size()) { - pattern = expected.get(expno); - } - - if (pattern == null) { - errors.append("Exhausted expected list before matching event " + gotno - + ": '" + eventLine + "'."); - mismatch = true; - break; - } - - if (pattern.matches(eventLine)) { - if (! pattern.repeatable()) { - ++expno; - } - ++gotno; - } else { - if (pattern.optional()) { - ++expno; - } else { - errors.append("Event " + gotno + ": '" + eventLine - + "' did not match regex " + expno + ": " + pattern); - mismatch = true; - break; - } - } - } - if (!mismatch && expno < expected.size()) { - errors.append("Too few entries in event log (only matched " - + expno + " of " + expected.size() + ")"); - mismatch = true; - } - if (mismatch) { - StringBuilder eventsGotten = new StringBuilder(); - for (Event e : events) { - String eventLine = e.toString(); - eventsGotten.append(eventLine).append("\n"); - } - errors.append("\nExpected events matching:\n" + exp + "\n"); - errors.append("but got the following events:\n" + eventsGotten.toString()); - fail(errors.toString()); - } - } - public static Set<ConfiguredNode> toNodes(Integer ... indexes) { return Arrays.stream(indexes) .map(i -> new ConfiguredNode(i, false)) diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java index 641f15467a6..6a7c75c629e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/RpcServerTest.java @@ -9,6 +9,7 @@ import com.yahoo.jrt.StringValue; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; +import com.yahoo.jrt.slobrok.api.BackOffPolicy; import com.yahoo.jrt.slobrok.server.Slobrok; import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.distribution.Distribution; @@ -595,4 +596,12 @@ public class RpcServerTest extends FleetControllerTest { return req; } + private static class BackOff implements BackOffPolicy { + private int counter = 0; + public void reset() { counter = 0; } + public double get() { ++counter; return 0.01; } + public boolean shouldWarn(double v) { return ((counter % 1000) == 10); } + public boolean shouldInform(double v) { return false; } + } + } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java index 6ad0430b5c3..426ac42fe9e 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java @@ -82,7 +82,7 @@ public class StateChangeTest extends FleetControllerTest { super.tearDown(); } - public void verifyNodeEvents(Node n, String correct) { + private void verifyNodeEvents(Node n, String correct) { String actual = ""; for (NodeEvent e : eventLog.getNodeEvents(n)) { actual += e.toString() + "\n"; diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java index 0a193766df3..6bc9f434ee6 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java @@ -6,11 +6,9 @@ import com.yahoo.vdslib.state.NodeState; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.database.DatabaseHandler; -import com.yahoo.vespa.clustercontroller.core.listeners.SlobrokListener; import com.yahoo.vespa.clustercontroller.core.listeners.NodeListener; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; - import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -69,11 +67,6 @@ public class SystemStateBroadcasterTest { } @Override - public SlobrokListener getNodeAddedOrRemovedListener() { - return null; - } - - @Override public NodeListener getNodeStateUpdateListener() { return null; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java index 55e640d4ec7..0c8c8946cbc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java @@ -18,6 +18,7 @@ public class SessionActiveResponse extends SlimeJsonResponse { Cursor root = metaData.get(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(sessionId)); root.setString("message", message); root.setString("url", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java index e436675fb59..617df3868ab 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java @@ -25,6 +25,7 @@ public class SessionPrepareAndActivateResponse extends SlimeJsonResponse { Cursor root = slime.get(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(result.sessionId())); root.setString("url", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName + "/application/" + applicationId.application().value() + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java index 563f50d0012..ac745f833f9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java @@ -31,6 +31,7 @@ public class SessionPrepareResponse extends SlimeJsonResponse { Cursor root = deployLog.get().type() != Type.NIX ? deployLog.get() : deployLog.setObject(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(sessionId)); root.setString("activate", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName.value() + "/session/" + sessionId + "/active"); root.setString("message", "Session " + sessionId + " for tenant '" + tenantName.value() + "' prepared."); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java index 8638a29cf75..1c71ef0b7fb 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -155,7 +155,10 @@ public class SessionActiveHandlerTest { private void assertActivationMessageOK(ActivateRequest activateRequest, String message) throws IOException { ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); new JsonFormat(true).encode(byteArrayOutputStream, activateRequest.getMetaData().getSlime()); - assertTrue(message.contains("\"tenant\":\"" + tenantName + "\",\"message\":\"Session " + activateRequest.getSessionId() + activatedMessage)); + long sessionId = activateRequest.getSessionId(); + assertTrue(message.contains("\"tenant\":\"" + tenantName)); + assertTrue(message.contains("\"session-id\":\"" + sessionId)); + assertTrue(message.contains("\"message\":\"Session " + sessionId + activatedMessage)); assertTrue(message.contains("/application/v2/tenant/" + tenantName + "/application/" + appName + "/environment/" + "prod" + diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index 66da009946e..2b07cffffce 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -61,7 +61,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { private ConfigserverConfig configserverConfig; private String preparedMessage = " prepared.\"}"; - private String tenantMessage = ""; private TenantRepository tenantRepository; @Rule @@ -90,7 +89,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { .build(); pathPrefix = "/application/v2/tenant/" + tenant + "/session/"; preparedMessage = " for tenant '" + tenant + "' prepared.\""; - tenantMessage = ",\"tenant\":\"" + tenant + "\""; } @Test @@ -123,13 +121,16 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void require_that_activate_url_is_returned_on_success() throws Exception { + public void require_that_response_has_all_fields() throws Exception { long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertNotNull(response); assertEquals(OK, response.getStatus()); - assertResponseContains(response, "\"activate\":\"http://foo:1337" + pathPrefix + sessionId + - "/active\",\"message\":\"Session " + sessionId + preparedMessage); + assertResponseContains(response, "\"activate\":\"http://foo:1337" + pathPrefix + sessionId + "/active\""); + assertResponseContains(response, "\"message\":\"Session " + sessionId + preparedMessage); + assertResponseContains(response, "\"tenant\":\"" + tenant + "\""); + assertResponseContains(response, "\"session-id\":\"" + sessionId + "\""); + assertResponseContains(response, "\"log\":[]"); } @Test @@ -186,15 +187,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void require_that_tenant_is_in_response() throws Exception { - long sessionId = createSession(applicationId()); - HttpResponse response = request(HttpRequest.Method.PUT, sessionId); - assertNotNull(response); - assertEquals(OK, response.getStatus()); - assertResponseContains(response, tenantMessage); - } - - @Test public void require_that_preparing_with_multiple_tenants_work() throws Exception { SessionHandler handler = createHandler(); @@ -307,7 +299,8 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } private static void assertResponseContains(HttpResponse response, String string) throws IOException { - assertTrue(SessionHandlerTest.getRenderedString(response).contains(string)); + String s = SessionHandlerTest.getRenderedString(response); + assertTrue(s, s.contains(string)); } private static void assertResponseNotContains(HttpResponse response, String string) throws IOException { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index ad1e8525b4b..38d09c3ddf4 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -142,6 +142,10 @@ public abstract class AbstractParser implements CustomParser { private Item parse(String queryToParse, String filterToParse, Language parsingLanguage, IndexFacts.Session indexFacts, String defaultIndex, Parsable parsable) { if (queryToParse == null) return null; + + if (defaultIndexName != null) + defaultIndexName = indexFacts.getCanonicName(defaultIndex); + tokenize(queryToParse, defaultIndex, indexFacts, parsingLanguage); if (parsingLanguage == null && parsable != null) { diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index f8a77445a40..a722552b7d2 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java @@ -452,10 +452,34 @@ public class QueryTestCase { @Test void testDefaultIndex() { - Query q = new Query("?query=hi%20hello%20keyword:kanoo%20" + - "default:munkz%20%22phrases+too%22&default-index=def"); + Query q = new Query("?query=hi%20hello%20keyword:kanoo%20default:munkz%20%22phrases+too%22&default-index=def"); assertEquals("WEAKAND(100) def:hi def:hello keyword:kanoo default:munkz def:\"phrases too\"", - q.getModel().getQueryTree().toString()); + q.getModel().getQueryTree().toString()); + } + + @Test + void testDefaultIndexAlias() { + SearchDefinition test = new SearchDefinition("test"); + Index year = new Index("year"); + year.setNumerical(true); + year.addAlias("yearalias"); + test.addIndex(year); + test.addAlias("yearalias", "year"); + IndexModel indexModel = new IndexModel(test); + + { + Query q = new Query("?default-index=year&type=all"); + q.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + q.getModel().setQueryString("2000"); + assertEquals("select * from sources * where year = 2000", q.yqlRepresentation()); + } + + { + Query q = new Query("?default-index=yearalias&type=all"); + q.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + q.getModel().setQueryString("2000"); + assertEquals("select * from sources * where year = 2000", q.yqlRepresentation()); + } } @Test diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java index 9cf508d37c9..3558d18f721 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.api.integration; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import java.io.InputStream; import java.util.Optional; /** @@ -29,4 +30,10 @@ public interface RunDataStore { /** Deletes all data associated with the given application. */ void delete(ApplicationId id); + /** Stores Vespa logs for the run. */ + void putLogs(RunId id, boolean tester, InputStream logs); + + /** Fetches Vespa logs for the run. */ + InputStream getLogs(RunId id, boolean tester); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java index 32591cfe4e2..cc1e084de8e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java @@ -5,10 +5,14 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import static com.yahoo.yolean.Exceptions.uncheck; + /** * @author jonmv */ @@ -16,6 +20,8 @@ public class MockRunDataStore implements RunDataStore { private final Map<RunId, byte[]> logs = new ConcurrentHashMap<>(); private final Map<RunId, byte[]> reports = new ConcurrentHashMap<>(); + private final Map<RunId, byte[]> vespaLogs = new ConcurrentHashMap<>(); + private final Map<RunId, byte[]> testerLogs = new ConcurrentHashMap<>(); @Override public Optional<byte[]> get(RunId id) { @@ -49,4 +55,14 @@ public class MockRunDataStore implements RunDataStore { reports.keySet().removeIf(runId -> runId.application().equals(id)); } + @Override + public void putLogs(RunId id, boolean tester, InputStream logs) { + (tester ? testerLogs : vespaLogs).put(id, uncheck(logs::readAllBytes)); + } + + @Override + public InputStream getLogs(RunId id, boolean tester) { + return new ByteArrayInputStream((tester ? testerLogs : vespaLogs).get(id)); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 881107fa0f9..7114402f824 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -38,6 +38,9 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; @@ -181,20 +184,30 @@ public class JobController { if ( ! run.hasStep(copyVespaLogs)) return run; + storeVespaLogs(id); + + // TODO jonmv: remove all the below around start of 2023. ZoneId zone = id.type().zone(); Optional<Deployment> deployment = Optional.ofNullable(controller.applications().requireInstance(id.application()) .deployments().get(zone)); if (deployment.isEmpty() || deployment.get().at().isBefore(run.start())) return run; - Instant deployedAt = run.stepInfo(installInitialReal).or(() -> run.stepInfo(installReal)).flatMap(StepInfo::startTime).orElseThrow(); - Instant from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); - List<LogEntry> log = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() - .getLogs(new DeploymentId(id.application(), zone), - Map.of("from", Long.toString(from.toEpochMilli()))), - from); + List<LogEntry> log; + Instant deployedAt; + Instant from; + if ( ! run.id().type().isProduction()) { + deployedAt = run.stepInfo(installInitialReal).or(() -> run.stepInfo(installReal)).flatMap(StepInfo::startTime).orElseThrow(); + from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); + log = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() + .getLogs(new DeploymentId(id.application(), zone), + Map.of("from", Long.toString(from.toEpochMilli()))), + from); + } + else + log = List.of(); - if (run.hasStep(installTester) && run.versions().targetPlatform().isAfter(new Version("7.590"))) { // todo jonmv: remove + if (id.type().isTest()) { deployedAt = run.stepInfo(installTester).flatMap(StepInfo::startTime).orElseThrow(); from = run.lastVespaLogTimestamp().isAfter(run.start()) ? run.lastVespaLogTimestamp() : deployedAt.minusSeconds(10); List<LogEntry> testerLog = LogEntry.parseVespaLog(controller.serviceRegistry().configServer() @@ -216,6 +229,47 @@ public class JobController { }); } + public InputStream getVespaLogs(RunId id, long fromMillis, boolean tester) { + Run run = run(id); + return run.stepStatus(copyVespaLogs).map(succeeded::equals).orElse(false) + ? controller.serviceRegistry().runDataStore().getLogs(id, tester) + : getVespaLogsFromLogserver(run, fromMillis, tester); + } + + public static Optional<Instant> deploymentCompletedAt(Run run, boolean tester) { + return (tester ? run.stepInfo(installTester) + : run.stepInfo(installInitialReal).or(() -> run.stepInfo(installReal))) + .flatMap(StepInfo::startTime); + } + + public void storeVespaLogs(RunId id) { + Run run = run(id); + if ( ! id.type().isProduction()) { + try (InputStream logs = getVespaLogsFromLogserver(run, 0, false)) { + controller.serviceRegistry().runDataStore().putLogs(id, false, logs); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + if (id.type().isTest()) { + try (InputStream logs = getVespaLogsFromLogserver(run, 0, true)) { + controller.serviceRegistry().runDataStore().putLogs(id, true, logs); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } + + private InputStream getVespaLogsFromLogserver(Run run, long fromMillis, boolean tester) { + long deploymentCompletedAtMillis = deploymentCompletedAt(run, tester).orElse(Instant.EPOCH).toEpochMilli(); + return controller.serviceRegistry().configServer().getLogs(new DeploymentId(tester ? run.id().tester().id() : run.id().application(), + run.id().type().zone()), + Map.of("from", Long.toString(Math.max(fromMillis, deploymentCompletedAtMillis)), + "to", Long.toString(run.end().orElse(controller.clock().instant()).toEpochMilli()))); + } + /** Fetches any new test log entries, and records the id of the last of these, for continuation. */ public void updateTestLog(RunId id) { locked(id, run -> { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java index 5f9207953ca..f0ec39b8d1c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java @@ -63,6 +63,7 @@ public enum JobProfile { installTester, startTests, endTests, + copyVespaLogs, deactivateTester, report)), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java index ddcfef23d86..2d0424e9f03 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java @@ -119,6 +119,7 @@ public class OsUpgradeScheduler extends ControllerMaintainer { Objects.requireNonNull(artifactRepository); } + @Override public Optional<Change> change(Version currentVersion, Instant instant) { OsRelease release = artifactRepository.osRelease(currentVersion.getMajor(), tag()); if (!release.version().isAfter(currentVersion)) return Optional.empty(); @@ -162,8 +163,8 @@ public class OsUpgradeScheduler extends ControllerMaintainer { public Optional<Change> change(Version currentVersion, Instant instant) { Version wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion); while (!wantedVersion.isAfter(currentVersion)) { - wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion); instant = instant.plus(Duration.ofDays(1)); + wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion); } return Optional.of(new Change(wantedVersion, upgradeBudget(), schedulingInstant(instant, system))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index e79aafbc783..a4bb9034a85 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -272,6 +272,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/diff/{number}")) return devApplicationPackageDiff(runIdFromPath(path)); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/test-config")) return testConfig(appIdFromPath(path), jobTypeFromPath(path)); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/run/{number}")) return JobControllerApiHandlerHelper.runDetailsResponse(controller.jobController(), runIdFromPath(path), request.getProperty("after")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/run/{number}/logs")) return JobControllerApiHandlerHelper.vespaLogsResponse(controller.jobController(), runIdFromPath(path), asLong(request.getProperty("from"), 0), request.getBooleanProperty("tester")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return getReindexing(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/suspended")) return suspended(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 25953c16bf0..60f65070557 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -8,7 +8,6 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.SlimeJsonResponse; -import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; @@ -16,16 +15,15 @@ import com.yahoo.text.Text; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.NotExistsException; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ConvergenceSummary; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.JobController; @@ -39,11 +37,13 @@ import com.yahoo.vespa.hosted.controller.deployment.Versions; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.URI; import java.time.Instant; import java.time.format.TextStyle; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Locale; @@ -53,6 +53,7 @@ import java.util.stream.Stream; import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.canary; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; +import static com.yahoo.vespa.hosted.controller.deployment.Step.copyVespaLogs; import static com.yahoo.vespa.hosted.controller.deployment.Step.installInitialReal; import static com.yahoo.vespa.hosted.controller.deployment.Step.installReal; import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.broken; @@ -154,9 +155,27 @@ class JobControllerApiHandlerHelper { .map(Slime::get) .ifPresent(reportArrayCursor -> SlimeUtils.copyArray(reportArrayCursor, detailsObject.setArray("testReports"))); + boolean logsStored = run.stepStatus(copyVespaLogs).map(succeeded::equals).orElse(false); + if (run.hasStep(copyVespaLogs) && ! runId.type().isProduction() && JobController.deploymentCompletedAt(run, false).isPresent()) + detailsObject.setBool("vespaLogsActive", ! logsStored); + + if (runId.type().isTest() && JobController.deploymentCompletedAt(run, true).isPresent()) + detailsObject.setBool("testerLogsActive", ! logsStored); + return new SlimeJsonResponse(slime); } + /** Proxies a Vespa log request for a run to S3 once logs have been copied, or to logserver before this. */ + static HttpResponse vespaLogsResponse(JobController jobController, RunId runId, long fromMillis, boolean tester) { + return new HttpResponse(200) { + @Override public void render(OutputStream out) throws IOException { + try (InputStream logs = jobController.getVespaLogs(runId, fromMillis, tester)) { + logs.transferTo(out); + } + } + }; + } + private static void toSlime(Cursor summaryObject, ConvergenceSummary summary) { summaryObject.setLong("nodes", summary.nodes()); summaryObject.setLong("down", summary.down()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index 493c0945ecc..784ea284de5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -470,28 +470,24 @@ public class InternalStepRunnerTest { new LogEntry(lastId + 4, tester.clock().instant().minusSeconds(4), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), - /* new LogEntry(lastId + 5, tester.clock().instant().minusSeconds(4), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), new LogEntry(lastId + 6, tester.clock().instant().minusSeconds(4), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), - */ - new LogEntry(lastId + 5, tester.clock().instant().minusSeconds(3), info, + new LogEntry(lastId + 7, tester.clock().instant().minusSeconds(3), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), - new LogEntry(lastId + 6, tester.clock().instant().minusSeconds(3), warning, + new LogEntry(lastId + 8, tester.clock().instant().minusSeconds(3), warning, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstderr\n" + - "java.lang.NullPointerException\n\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\n\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)")); - /* + "java.lang.NullPointerException\n\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\n\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)"), new LogEntry(lastId + 9, tester.clock().instant().minusSeconds(3), info, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstdout\n" + "ERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)"), new LogEntry(lastId + 10, tester.clock().instant().minusSeconds(3), warning, "17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\tcontainer\tstderr\n" + "java.lang.NullPointerException\n\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\n\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)")); - */ } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index 233b460333e..168b9b374f3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -7,6 +7,7 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TestReport; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; @@ -144,6 +145,10 @@ public class JobControllerApiHandlerHelperTest { assertResponse(JobControllerApiHandlerHelper.runResponse(app.application(), tester.jobs().runs(userApp.instanceId(), devAwsUsEast2a), Optional.empty(), URI.create("https://some.url:43/root")), "dev-aws-us-east-2a-runs.json"); assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), userApp.instanceId(), URI.create("https://some.url:43/root/")), "overview-user-instance.json"); assertResponse(JobControllerApiHandlerHelper.overviewResponse(tester.controller(), app.application().id(), URI.create("https://some.url:43/root/")), "deployment-overview-2.json"); + + tester.configServer().setLogStream(() -> "no more logs"); + assertResponse(JobControllerApiHandlerHelper.vespaLogsResponse(tester.jobs(), new RunId(app.instanceId(), stagingTest, 1), 0, false), "vespa.log"); + assertResponse(JobControllerApiHandlerHelper.vespaLogsResponse(tester.jobs(), new RunId(app.instanceId(), stagingTest, 1), 0, true), "vespa.log"); } @Test @@ -197,11 +202,17 @@ public class JobControllerApiHandlerHelperTest { Path path = Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/").resolve(fileName); ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); - byte[] actualJson = SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(baos.toByteArray()).get(), false); - // Files.write(path, actualJson); - byte[] expected = Files.readAllBytes(path); - assertEquals(new String(SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(expected).get(), false), UTF_8), - new String(actualJson, UTF_8)); + if (fileName.endsWith(".json")) { + byte[] actualJson = SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(baos.toByteArray()).get(), false); + // Files.write(path, actualJson); + byte[] expected = Files.readAllBytes(path); + assertEquals(new String(SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlimeOrThrow(expected).get(), false), UTF_8), + new String(actualJson, UTF_8)); + } + else { + assertEquals(Files.readString(path), + baos.toString(UTF_8)); + } } catch (Exception e) { throw new RuntimeException(e); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index a7e2d1913cf..a02fb1fb375 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -967,6 +967,10 @@ "status": "succeeded" }, { + "name": "copyVespaLogs", + "status": "succeeded" + }, + { "name": "deactivateTester", "status": "succeeded" }, @@ -1016,6 +1020,10 @@ "status": "succeeded" }, { + "name": "copyVespaLogs", + "status": "succeeded" + }, + { "name": "deactivateTester", "status": "succeeded" }, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json index 9b391196d55..3190dee06fa 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-first-part.json @@ -82,5 +82,6 @@ "copyVespaLogs": { "status": "unfinished" } - } + }, + "vespaLogsActive": true } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json index 4ffac2bf738..cee7e0f4c92 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-east-1-log-second-part.json @@ -39,5 +39,6 @@ "status": "succeeded", "startMillis": 0 } - } + }, + "vespaLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json index a2f62621f5b..0b8937c31a4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/staging-test-log.json @@ -232,5 +232,7 @@ "status": "succeeded", "startMillis": 14503000 } - } + }, + "vespaLogsActive": false, + "testerLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json index a691762c40b..638a7e24ee7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json @@ -401,5 +401,7 @@ "status": "succeeded", "startMillis": 1600000000000 } - } + }, + "vespaLogsActive": false, + "testerLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json index 4e8737d5f67..a8b38079b6f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json @@ -404,5 +404,7 @@ "failed": 0 } } - ] + ], + "vespaLogsActive": false, + "testerLogsActive": false } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log new file mode 100644 index 00000000000..25916d9e6df --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/vespa.log @@ -0,0 +1 @@ +INFO - All good
\ No newline at end of file diff --git a/document/src/vespa/document/datatype/datatype.cpp b/document/src/vespa/document/datatype/datatype.cpp index 3cc7034e336..7d53679bc28 100644 --- a/document/src/vespa/document/datatype/datatype.cpp +++ b/document/src/vespa/document/datatype/datatype.cpp @@ -155,7 +155,7 @@ DataType::buildFieldPath(FieldPath & path, vespalib::stringref remainFieldName) { if ( !remainFieldName.empty() ) { path.reserve(4); // Optimize for short paths - onBuildFieldPath(path,remainFieldName); + onBuildFieldPath(path, remainFieldName); } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 1b1a5493cc5..e349165b855 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -119,7 +119,7 @@ public class Flags { public static final UnboundBooleanFlag USE_THREE_PHASE_UPDATES = defineFeatureFlag( "use-three-phase-updates", false, - List.of("vekterli"), "2020-12-02", "2022-08-15", + List.of("vekterli"), "2020-12-02", "2022-10-01", "Whether to enable the use of three-phase updates when bucket replicas are out of sync.", "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); @@ -245,7 +245,7 @@ public class Flags { public static final UnboundIntFlag MAX_ACTIVATION_INHIBITED_OUT_OF_SYNC_GROUPS = defineIntFlag( "max-activation-inhibited-out-of-sync-groups", 0, - List.of("vekterli"), "2021-02-19", "2022-08-15", + List.of("vekterli"), "2021-02-19", "2022-10-01", "Allows replicas in up to N content groups to not be activated " + "for query visibility if they are out of sync with a majority of other replicas", "Takes effect at redeployment", @@ -253,14 +253,14 @@ public class Flags { public static final UnboundIntFlag MAX_CONCURRENT_MERGES_PER_NODE = defineIntFlag( "max-concurrent-merges-per-node", 16, - List.of("balder", "vekterli"), "2021-06-06", "2022-08-15", + List.of("balder", "vekterli"), "2021-06-06", "2022-10-01", "Specifies max concurrent merges per content node.", "Takes effect at redeploy", ZONE_ID, APPLICATION_ID); public static final UnboundIntFlag MAX_MERGE_QUEUE_SIZE = defineIntFlag( "max-merge-queue-size", 100, - List.of("balder", "vekterli"), "2021-06-06", "2022-08-15", + List.of("balder", "vekterli"), "2021-06-06", "2022-10-01", "Specifies max size of merge queue.", "Takes effect at redeploy", ZONE_ID, APPLICATION_ID); @@ -339,7 +339,7 @@ public class Flags { public static final UnboundStringFlag MERGE_THROTTLING_POLICY = defineStringFlag( "merge-throttling-policy", "STATIC", - List.of("vekterli"), "2022-01-25", "2022-08-15", + List.of("vekterli"), "2022-01-25", "2022-10-01", "Sets the policy used for merge throttling on the content nodes. " + "Valid values: STATIC, DYNAMIC", "Takes effect at redeployment", @@ -347,7 +347,7 @@ public class Flags { public static final UnboundDoubleFlag PERSISTENCE_THROTTLING_WS_DECREMENT_FACTOR = defineDoubleFlag( "persistence-throttling-ws-decrement-factor", 1.2, - List.of("vekterli"), "2022-01-27", "2022-08-15", + List.of("vekterli"), "2022-01-27", "2022-10-01", "Sets the dynamic throttle policy window size decrement factor for persistence " + "async throttling. Only applies if DYNAMIC policy is used.", "Takes effect on redeployment", @@ -355,7 +355,7 @@ public class Flags { public static final UnboundDoubleFlag PERSISTENCE_THROTTLING_WS_BACKOFF = defineDoubleFlag( "persistence-throttling-ws-backoff", 0.95, - List.of("vekterli"), "2022-01-27", "2022-08-15", + List.of("vekterli"), "2022-01-27", "2022-10-01", "Sets the dynamic throttle policy window size backoff for persistence " + "async throttling. Only applies if DYNAMIC policy is used. Valid range [0, 1]", "Takes effect on redeployment", diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java index e18ea374fe1..148f48136f4 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java @@ -4,6 +4,7 @@ package ai.vespa.metricsproxy.node; import ai.vespa.metricsproxy.core.MetricsManager; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; +import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.metric.model.ServiceId; @@ -18,6 +19,7 @@ import com.yahoo.container.jdisc.state.HostLifeGatherer; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static ai.vespa.metricsproxy.node.ServiceHealthGatherer.gatherServiceHealthMetrics; @@ -78,6 +80,7 @@ public class NodeMetricGatherer { builder.putMetric(MetricId.toMetricId(key), metrics.get(key).asLong()); } } + builder.addConsumers(Set.of(ConsumerId.toConsumerId("Vespa"))); builders.add(builder); } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java index c693e9690e8..0de3526b40d 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.metricsproxy.node; +import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import com.fasterxml.jackson.databind.JsonNode; @@ -10,6 +11,7 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; +import java.util.Set; import static org.junit.Assert.assertEquals; @@ -33,6 +35,7 @@ public class NodeMetricGathererTest { assertEquals(123, packet.timestamp); assertEquals(12l, packet.metrics().get(MetricId.toMetricId("uptime"))); assertEquals(1l, packet.metrics().get(MetricId.toMetricId("alive"))); + assertEquals(Set.of(ConsumerId.toConsumerId("Vespa")), packet.consumers()); } private JsonNode generateHostLifePacket() { diff --git a/screwdriver/release-container-image.sh b/screwdriver/release-container-image.sh index 3ae2a7feb1e..def2f037b63 100755 --- a/screwdriver/release-container-image.sh +++ b/screwdriver/release-container-image.sh @@ -26,27 +26,35 @@ cd $BUILD_DIR git clone --depth 1 https://github.com/vespa-engine/docker-image cd docker-image -docker build --build-arg VESPA_VERSION=$VESPA_VERSION --file Dockerfile \ - --tag docker.io/vespaengine/vespa:$VESPA_VERSION --tag docker.io/vespaengine/vespa:latest \ - --tag ghcr.io/vespa-engine/vespa:$VESPA_VERSION --tag ghcr.io/vespa-engine/vespa:latest . +docker info +docker version +docker buildx version +docker buildx install + +unset DOCKER_HOST +docker context create vespa-context --docker "host=tcp://localhost:2376,ca=/certs/client/ca.pem,cert=/certs/client/cert.pem,key=/certs/client/key.pem" +docker context use vespa-context + +docker buildx create --name vespa-builder --driver docker-container --use +docker buildx inspect --bootstrap # Push to Docker Hub -docker login --username aressem --password "$DOCKER_HUB_DEPLOY_KEY" if curl -fsSL https://index.docker.io/v1/repositories/vespaengine/vespa/tags/$VESPA_VERSION &> /dev/null; then echo "Container image docker.io/vespaengine/vespa:$VESPA_VERSION aldready exists." else - docker push docker.io/vespaengine/vespa:$VESPA_VERSION - docker push docker.io/vespaengine/vespa:latest + docker login --username aressem --password "$DOCKER_HUB_DEPLOY_KEY" + docker buildx build --progress plain --push --platform linux/amd64,linux/arm64 --build-arg VESPA_VERSION=$VESPA_VERSION \ + --tag docker.io/vespaengine/vespa:$VESPA_VERSION --tag docker.io/vespaengine/vespa:latest . fi # Push to GitHub Container Registry -docker login --username aressem --password "$GHCR_DEPLOY_KEY" ghcr.io JWT=$(curl -sSL -u aressem:$GHCR_DEPLOY_KEY "https://ghcr.io/token?service=ghcr.io&scope=repository:vespa-engine/vespa:pull" | jq -re '.token') IMAGE_TAGS=$(curl -sSL -H "Authorization: Bearer $JWT" https://ghcr.io/v2/vespa-engine/vespa/tags/list | jq -re '.tags[]') if grep $VESPA_VERSION <<< "$IMAGE_TAGS" &> /dev/null; then echo "Container image ghcr.io/vespa-engine/vespa:$VESPA_VERSION aldready exists." else - docker push ghcr.io/vespa-engine/vespa:$VESPA_VERSION - docker push ghcr.io/vespa-engine/vespa:latest + docker login --username aressem --password "$GHCR_DEPLOY_KEY" ghcr.io + docker buildx build --progress plain --push --platform linux/amd64,linux/arm64 --build-arg VESPA_VERSION=$VESPA_VERSION \ + --tag ghcr.io/vespa-engine/vespa:$VESPA_VERSION --tag ghcr.io/vespa-engine/vespa:latest . fi diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt index 6ca815cd216..5435e9eb93e 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/CMakeLists.txt @@ -17,18 +17,6 @@ vespa_add_executable(searchcore_documentbucketmover_test_app TEST ) vespa_add_test(NAME searchcore_documentbucketmover_test_app COMMAND searchcore_documentbucketmover_test_app) -vespa_add_executable(searchcore_scaniterator_test_app TEST - SOURCES - scaniterator_test.cpp - DEPENDS - searchcore_bucketmover_test - searchcore_server - searchcore_test - searchcore_feedoperation - GTest::GTest -) -vespa_add_test(NAME searchcore_scaniterator_test_app COMMAND searchcore_scaniterator_test_app) - vespa_add_executable(searchcore_documentmover_test_app TEST SOURCES documentmover_test.cpp diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp deleted file mode 100644 index 4ec38ff7170..00000000000 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/scaniterator_test.cpp +++ /dev/null @@ -1,185 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "bucketmover_common.h" -#include <vespa/searchcore/proton/bucketdb/bucketscaniterator.h> -#include <vespa/vespalib/gtest/gtest.h> - -#include <vespa/log/log.h> -LOG_SETUP("document_bucket_mover_test"); - -using namespace proton; -using namespace proton::move::test; -using document::BucketId; - -using ScanItr = bucketdb::ScanIterator; - -struct ScanTestBase : public ::testing::Test -{ - test::UserDocumentsBuilder _builder; - std::shared_ptr<bucketdb::BucketDBOwner> _bucketDB; - MySubDb _ready; - MySubDb _notReady; - ScanTestBase(); - ~ScanTestBase(); - - ScanItr getItr() { - return getItr(BucketId()); - } - - ScanItr getItr(BucketId bucket) { - return ScanItr(_bucketDB->takeGuard(), bucket); - } -}; - -ScanTestBase::ScanTestBase() - : _builder(), - _bucketDB(std::make_shared<bucketdb::BucketDBOwner>()), - _ready(_builder.getRepo(), _bucketDB, 1, SubDbType::READY), - _notReady(_builder.getRepo(), _bucketDB, 2, SubDbType::NOTREADY) -{} -ScanTestBase::~ScanTestBase() = default; - -struct ScanTest : public ScanTestBase -{ - ScanTest() : ScanTestBase() - { - _builder.createDocs(6, 1, 2); - _builder.createDocs(8, 2, 3); - _ready.insertDocs(_builder.getDocs()); - _builder.clearDocs(); - _builder.createDocs(2, 1, 2); - _builder.createDocs(4, 2, 3); - _notReady.insertDocs(_builder.getDocs()); - _builder.clearDocs(); - } -}; - -struct OnlyNotReadyScanTest : public ScanTestBase -{ - OnlyNotReadyScanTest() : ScanTestBase() - { - _builder.createDocs(2, 1, 2); - _builder.createDocs(4, 2, 3); - _notReady.insertDocs(_builder.getDocs()); - } -}; - -struct OnlyReadyScanTest : public ScanTestBase -{ - OnlyReadyScanTest() : ScanTestBase() - { - _builder.createDocs(6, 1, 2); - _builder.createDocs(8, 2, 3); - _ready.insertDocs(_builder.getDocs()); - } -}; - -struct BucketVector : public BucketId::List -{ - BucketVector() : BucketId::List() {} - BucketVector &add(const BucketId &bucket) { - push_back(bucket); - return *this; - } -}; - -void -advanceToFirstBucketWithDocs(ScanItr &itr, SubDbType subDbType) -{ - while (itr.valid()) { - if (subDbType == SubDbType::READY) { - if (itr.hasReadyBucketDocs()) - return; - } else { - if (itr.hasNotReadyBucketDocs()) - return; - } - ++itr; - } -} - -void -assertEquals(const BucketVector &exp, ScanItr &itr, SubDbType subDbType) -{ - for (size_t i = 0; i < exp.size(); ++i) { - advanceToFirstBucketWithDocs(itr, subDbType); - EXPECT_TRUE(itr.valid()); - EXPECT_EQ(exp[i], itr.getBucket()); - ++itr; - } - advanceToFirstBucketWithDocs(itr, subDbType); - EXPECT_FALSE(itr.valid()); -} - -TEST_F(ScanTest, require_that_we_can_iterate_all_buckets_from_start_to_end) -{ - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_notReady.bucket(2)). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); - } - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); - } -} - -TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_not_ready_buckets) -{ - BucketId bucket = _notReady.bucket(4); - { - ScanItr itr = getItr(bucket); - assertEquals(BucketVector(). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); - } - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); - } -} - -TEST_F(ScanTest, require_that_we_can_iterate_from_the_middle_of_ready_buckets) -{ - { - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_notReady.bucket(2)). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); - } - { - BucketId bucket = _ready.bucket(6); - ScanItr itr = getItr(bucket); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); - } -} - -TEST_F(OnlyNotReadyScanTest, require_that_we_can_iterate_only_not_ready_buckets) -{ - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_notReady.bucket(2)). - add(_notReady.bucket(4)), itr, SubDbType::NOTREADY); -} - -TEST_F(OnlyReadyScanTest, require_that_we_can_iterate_only_ready_buckets) -{ - ScanItr itr = getItr(); - assertEquals(BucketVector(). - add(_ready.bucket(6)). - add(_ready.bucket(8)), itr, SubDbType::READY); -} - -TEST_F(ScanTestBase, require_that_we_can_iterate_zero_buckets) -{ - ScanItr itr = getItr(); - EXPECT_FALSE(itr.valid()); -} - -GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp index b326db47b5c..9d9bb64e6b1 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp @@ -143,7 +143,7 @@ struct MyIndexWriter : public test::MockIndexWriter uint32_t _wantedLidLimit; MyTracer &_tracer; MyIndexWriter(MyTracer &tracer) - : test::MockIndexWriter(IIndexManager::SP(new test::MockIndexManager())), + : test::MockIndexWriter(std::make_shared<test::MockIndexManager>()), _removes(), _heartBeatCount(0), _commitCount(0), @@ -224,7 +224,7 @@ struct MyDocumentStore : public test::DummyDocumentStore DocMap _docs; uint64_t _lastSyncToken; uint32_t _compactLidSpaceLidLimit; - MyDocumentStore(const document::DocumentTypeRepo & repo) + MyDocumentStore(const document::DocumentTypeRepo & repo) noexcept : test::DummyDocumentStore("."), _repo(repo), _docs(), @@ -266,42 +266,47 @@ MyDocumentStore::~MyDocumentStore() = default; struct MySummaryManager : public test::DummySummaryManager { MyDocumentStore _store; - MySummaryManager(const document::DocumentTypeRepo & repo) : _store(repo) {} - virtual search::IDocumentStore &getBackingStore() override { return _store; } + MySummaryManager(const document::DocumentTypeRepo & repo) noexcept : _store(repo) {} + ~MySummaryManager() override; + search::IDocumentStore &getBackingStore() override { return _store; } }; +MySummaryManager::~MySummaryManager() = default; + struct MySummaryAdapter : public test::MockSummaryAdapter { ISummaryManager::SP _sumMgr; MyDocumentStore &_store; MyLidVector _removes; - MySummaryAdapter(const document::DocumentTypeRepo & repo) - : _sumMgr(new MySummaryManager(repo)), + MySummaryAdapter(const document::DocumentTypeRepo & repo) noexcept + : _sumMgr(std::make_shared<MySummaryManager>(repo)), _store(static_cast<MyDocumentStore &>(_sumMgr->getBackingStore())), _removes() {} - virtual void put(SerialNum serialNum, DocumentIdT lid, const Document &doc) override { + ~MySummaryAdapter() override; + void put(SerialNum serialNum, DocumentIdT lid, const Document &doc) override { _store.write(serialNum, lid, doc); } - virtual void put(SerialNum serialNum, DocumentIdT lid, const vespalib::nbostream & os) override { + void put(SerialNum serialNum, DocumentIdT lid, const vespalib::nbostream & os) override { _store.write(serialNum, lid, os); } - virtual void remove(SerialNum serialNum, const DocumentIdT lid) override { + void remove(SerialNum serialNum, const DocumentIdT lid) override { LOG(info, "MySummaryAdapter::remove(): serialNum(%" PRIu64 "), docId(%u)", serialNum, lid); _store.remove(serialNum, lid); _removes.push_back(lid); } - virtual const search::IDocumentStore &getDocumentStore() const override { + const search::IDocumentStore &getDocumentStore() const override { return _store; } - virtual std::unique_ptr<Document> get(const DocumentIdT lid, const DocumentTypeRepo &repo) override { + std::unique_ptr<Document> get(const DocumentIdT lid, const DocumentTypeRepo &repo) override { return _store.read(lid, repo); } - virtual void compactLidSpace(uint32_t wantedDocIdLimit) override { + void compactLidSpace(uint32_t wantedDocIdLimit) override { _store.compactLidSpace(wantedDocIdLimit); } }; +MySummaryAdapter::~MySummaryAdapter() = default; struct MyAttributeWriter : public IAttributeWriter { @@ -362,7 +367,7 @@ struct MyAttributeWriter : public IAttributeWriter _updateLid = lid; for (const auto & fieldUpdate : upd.getUpdates()) { search::AttributeVector * attr = getWritableAttribute(fieldUpdate.getField().getName()); - onUpdate.onUpdateField(fieldUpdate.getField().getName(), attr); + onUpdate.onUpdateField(fieldUpdate.getField(), attr); } } void update(SerialNum serialNum, const document::Document &doc, DocumentIdT lid, OnWriteDoneType) override { @@ -434,7 +439,7 @@ struct SchemaContext }; SchemaContext::SchemaContext() : - _schema(new Schema()), + _schema(std::make_shared<Schema>()), _builder() { _schema->addIndexField(Schema::IndexField("i1", DataType::STRING, CollectionType::SINGLE)); @@ -442,7 +447,7 @@ SchemaContext::SchemaContext() : _schema->addAttributeField(Schema::AttributeField("a2", DataType::BOOLEANTREE, CollectionType::SINGLE)); _schema->addAttributeField(Schema::AttributeField("a3", DataType::TENSOR, CollectionType::SINGLE)); _schema->addSummaryField(Schema::SummaryField("s1", DataType::STRING, CollectionType::SINGLE)); - _builder.reset(new DocBuilder(*_schema)); + _builder = std::make_unique<DocBuilder>(*_schema); } SchemaContext::~SchemaContext() = default; @@ -464,7 +469,7 @@ struct DocumentContext DocumentContext::DocumentContext(const vespalib::string &docId, uint64_t timestamp, DocBuilder &builder) : doc(builder.startDocument(docId).startSummaryField("s1").addStr(docId).endField().endDocument().release()), - upd(new DocumentUpdate(*builder.getDocumentTypeRepo(), builder.getDocumentType(), doc->getId())), + upd(std::make_shared<DocumentUpdate>(*builder.getDocumentTypeRepo(), builder.getDocumentType(), doc->getId())), bid(BucketFactory::getNumBucketBits(), doc->getId().getGlobalId().convertToBucketId().getRawId()), ts(timestamp) {} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index bb25b3da7be..021fc4717af 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -764,9 +764,9 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document for (const auto &fupd : upd.getUpdates()) { LOG(debug, "Retrieving guard for attribute vector '%s'.", fupd.getField().getName().data()); - auto itr = _attrMap.find(fupd.getField().getName()); - AttributeVector * attrp = (itr != _attrMap.end()) ? itr->second.attribute : nullptr; - onUpdate.onUpdateField(fupd.getField().getName(), attrp); + auto found = _attrMap.find(fupd.getField().getName()); + AttributeVector * attrp = (found != _attrMap.end()) ? found->second.attribute : nullptr; + onUpdate.onUpdateField(fupd.getField(), attrp); if (__builtin_expect(attrp == nullptr, false)) { LOG(spam, "Failed to find attribute vector %s", fupd.getField().getName().data()); continue; @@ -776,16 +776,15 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document if (__builtin_expect(attrp->getStatus().getLastSyncToken() >= serialNum, false)) { continue; } - if (itr->second.use_two_phase_put_for_assign_updates && - is_single_assign_update(fupd)) { + if (found->second.use_two_phase_put_for_assign_updates && is_single_assign_update(fupd)) { auto prepare_task = std::make_unique<PreparePutTask>(serialNum, lid, *attrp, get_single_assign_update_field_value(fupd)); auto complete_task = std::make_unique<CompletePutTask>(*prepare_task, onWriteDone); LOG(debug, "About to handle assign update as two phase put for docid %u in attribute vector '%s'", lid, attrp->getName().c_str()); _shared_executor.execute(CpuUsage::wrap(std::move(prepare_task), CpuUsage::Category::WRITE)); - _attributeFieldWriter.executeTask(itr->second.executor_id, std::move(complete_task)); + _attributeFieldWriter.executeTask(found->second.executor_id, std::move(complete_task)); } else { - args[itr->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); + args[found->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd); LOG(debug, "About to apply update for docId %u in attribute vector '%s'.", lid, attrp->getName().c_str()); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h index d8872607b44..d3ab970fb39 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h @@ -5,16 +5,17 @@ #include <vespa/vespalib/stllike/string.h> namespace search { class AttributeVector; } +namespace document { class Field; } namespace proton { struct IFieldUpdateCallback { - virtual ~IFieldUpdateCallback() { } - virtual void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) = 0; + virtual ~IFieldUpdateCallback() = default; + virtual void onUpdateField(const document::Field & field, const search::AttributeVector * attr) = 0; }; struct DummyFieldUpdateCallback : IFieldUpdateCallback { - void onUpdateField(vespalib::stringref, const search::AttributeVector *) override {} + void onUpdateField(const document::Field & , const search::AttributeVector *) override {} }; } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt index 810fe53236d..a542f44f27e 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/CMakeLists.txt @@ -6,7 +6,6 @@ vespa_add_library(searchcore_bucketdb STATIC bucket_db_owner.cpp bucketdb.cpp bucketdbhandler.cpp - bucketscaniterator.cpp bucketsessionbase.cpp bucketstate.cpp checksumaggregators.cpp diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp index b68892be60f..9d28d88ba4a 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucket_db_explorer.cpp @@ -33,10 +33,11 @@ checksumToString(storage::spi::BucketChecksum checksum) void convertBucketsToSlime(const BucketDB &bucketDb, Cursor &array) { - for (auto itr = bucketDb.begin(); itr != bucketDb.end(); ++itr) { + BucketId::List buckets = bucketDb.getBuckets(); + for (BucketId bucketId : buckets) { Cursor &object = array.addObject(); - object.setString("id", bucketIdToString(itr->first)); - const bucketdb::BucketState &state = itr->second; + object.setString("id", bucketIdToString(bucketId)); + bucketdb::BucketState state = bucketDb.get(bucketId); object.setString("checksum", checksumToString(state.getChecksum())); object.setLong("readyCount", state.getReadyCount()); object.setLong("notReadyCount", state.getNotReadyCount()); diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp index ecdc679e4bc..a99d5cbd573 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.cpp @@ -2,6 +2,7 @@ #include "bucketdb.h" #include "remove_batch_entry.h" +#include <vespa/vespalib/stllike/hash_map.hpp> #include <cassert> #include <algorithm> #include <optional> @@ -27,22 +28,19 @@ BucketDB::~BucketDB() } void -BucketDB::add(const BucketId &bucketId, const BucketState & state) { +BucketDB::add(BucketId bucketId, const BucketState & state) { _map[bucketId] += state; } bucketdb::BucketState * -BucketDB::getBucketStatePtr(const BucketId &bucket) +BucketDB::getBucketStatePtr(BucketId bucket) { auto it(_map.find(bucket)); - if (it != _map.end()) { - return &it->second; - } - return nullptr; + return (it != _map.end()) ? &it->second : nullptr; } void -BucketDB::unloadBucket(const BucketId &bucket, const BucketState &delta) +BucketDB::unloadBucket(BucketId bucket, const BucketState &delta) { BucketState *state = getBucketStatePtr(bucket); assert(state); @@ -51,7 +49,7 @@ BucketDB::unloadBucket(const BucketId &bucket, const BucketState &delta) const bucketdb::BucketState & BucketDB::add(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType) { BucketState &state = _map[bucketId]; @@ -61,7 +59,7 @@ BucketDB::add(const GlobalId &gid, void BucketDB::remove(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType) { BucketState &state = _map[bucketId]; @@ -84,8 +82,8 @@ BucketDB::remove_batch(const std::vector<RemoveBatchEntry> &removed, SubDbType s void BucketDB::modify(const GlobalId &gid, - const BucketId &oldBucketId, const Timestamp &oldTimestamp, uint32_t oldDocSize, - const BucketId &newBucketId, const Timestamp &newTimestamp, uint32_t newDocSize, + BucketId oldBucketId, Timestamp oldTimestamp, uint32_t oldDocSize, + BucketId newBucketId, Timestamp newTimestamp, uint32_t newDocSize, SubDbType subDbType) { if (oldBucketId == newBucketId) { @@ -99,17 +97,14 @@ BucketDB::modify(const GlobalId &gid, bucketdb::BucketState -BucketDB::get(const BucketId &bucketId) const +BucketDB::get(BucketId bucketId) const { auto itr = _map.find(bucketId); - if (itr != _map.end()) { - return itr->second; - } - return BucketState(); + return (itr != _map.end()) ? itr->second : BucketState(); } void -BucketDB::cacheBucket(const BucketId &bucketId) +BucketDB::cacheBucket(BucketId bucketId) { _cachedBucketId = bucketId; _cachedBucketState = get(bucketId); @@ -123,13 +118,13 @@ BucketDB::uncacheBucket() } bool -BucketDB::isCachedBucket(const BucketId &bucketId) const +BucketDB::isCachedBucket(BucketId bucketId) const { return _cachedBucketId == bucketId; } bucketdb::BucketState -BucketDB::cachedGet(const BucketId &bucketId) const +BucketDB::cachedGet(BucketId bucketId) const { if (isCachedBucket(bucketId)) { return _cachedBucketState; @@ -138,27 +133,23 @@ BucketDB::cachedGet(const BucketId &bucketId) const } storage::spi::BucketInfo -BucketDB::cachedGetBucketInfo(const BucketId &bucketId) const +BucketDB::cachedGetBucketInfo(BucketId bucketId) const { if (isCachedBucket(bucketId)) { return _cachedBucketState; } - auto itr = _map.find(bucketId); - if (itr != _map.end()) { - return itr->second; - } - return BucketState(); + return get(bucketId); } bool -BucketDB::hasBucket(const BucketId &bucketId) const +BucketDB::hasBucket(BucketId bucketId) const { return (_map.find(bucketId) != _map.end()); } bool -BucketDB::isActiveBucket(const BucketId &bucketId) const +BucketDB::isActiveBucket(BucketId bucketId) const { auto itr = _map.find(bucketId); return (itr != _map.end()) && itr->second.isActive(); @@ -172,6 +163,7 @@ BucketDB::getBuckets() const for (const auto & entry : _map) { buckets.push_back(entry.first); } + std::sort(buckets.begin(), buckets.end()); return buckets; } @@ -199,7 +191,7 @@ BucketDB::checkEmpty() const void -BucketDB::setBucketState(const BucketId &bucketId, bool active) +BucketDB::setBucketState(BucketId bucketId, bool active) { BucketState &state = _map[bucketId]; state.setActive(active); @@ -207,7 +199,7 @@ BucketDB::setBucketState(const BucketId &bucketId, bool active) void -BucketDB::createBucket(const BucketId &bucketId) +BucketDB::createBucket(BucketId bucketId) { BucketState &state = _map[bucketId]; (void) state; @@ -215,7 +207,7 @@ BucketDB::createBucket(const BucketId &bucketId) void -BucketDB::deleteEmptyBucket(const BucketId &bucketId) +BucketDB::deleteEmptyBucket(BucketId bucketId) { auto itr = _map.find(bucketId); if (itr == _map.end()) { @@ -231,11 +223,13 @@ document::BucketId::List BucketDB::getActiveBuckets() const { BucketId::List buckets; + buckets.reserve(_map.size()); for (const auto & entry : _map) { if (entry.second.isActive()) { buckets.push_back(entry.first); } } + std::sort(buckets.begin(), buckets.end()); return buckets; } @@ -247,11 +241,12 @@ BucketDB::populateActiveBuckets(BucketId::List buckets) std::sort(buckets.begin(), buckets.end()); auto si = buckets.begin(); auto se = buckets.end(); - for (const auto & entry : _map) { - for (; si != se && !(entry.first < *si); ++si) { - if (*si < entry.first) { + BucketId::List currentBuckets = getBuckets(); + for (BucketId bucketId : currentBuckets) { + for (; si != se && !(bucketId < *si); ++si) { + if (*si < bucketId) { toAdd.push_back(*si); - } else if (!entry.second.isActive()) { + } else if (!isActiveBucket(bucketId)) { fixupBuckets.push_back(*si); setBucketState(*si, true); } @@ -262,8 +257,8 @@ BucketDB::populateActiveBuckets(BucketId::List buckets) } BucketState activeState; activeState.setActive(true); - for (const BucketId & bucketId : toAdd) { - auto [itr, inserted] = _map.emplace(bucketId, activeState); + for (BucketId bucketId : toAdd) { + auto [itr, inserted] = _map.insert(std::make_pair(bucketId, activeState)); assert(inserted); } return fixupBuckets; diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h index da475f2969a..bd6aa16504b 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketdb.h @@ -5,7 +5,7 @@ #include "bucketstate.h" #include <vespa/document/bucket/bucketid.h> #include <vespa/persistence/spi/result.h> -#include <map> +#include <vespa/vespalib/stllike/hash_map.h> namespace proton::bucketdb { class RemoveBatchEntry; } @@ -19,8 +19,7 @@ public: typedef storage::spi::Timestamp Timestamp; typedef storage::spi::BucketChecksum BucketChecksum; typedef bucketdb::BucketState BucketState; - typedef std::map<BucketId, BucketState> Map; - typedef Map::const_iterator ConstMapIterator; + typedef vespalib::hash_map<BucketId, BucketState, BucketId::hash> Map; private: Map _map; @@ -34,43 +33,39 @@ public: ~BucketDB(); const BucketState & add(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType); - void add(const BucketId &bucketId, const BucketState & state); + void add(BucketId bucketId, const BucketState & state); void remove(const GlobalId &gid, - const BucketId &bucketId, const Timestamp ×tamp, uint32_t docSize, + BucketId bucketId, Timestamp timestamp, uint32_t docSize, SubDbType subDbType); void remove_batch(const std::vector<bucketdb::RemoveBatchEntry> &removed, SubDbType sub_db_type); void modify(const GlobalId &gid, - const BucketId &oldBucketId, const Timestamp &oldTimestamp, uint32_t oldDocSize, - const BucketId &newBucketId, const Timestamp &newTimestamp, uint32_t newDocSize, + BucketId oldBucketId, Timestamp oldTimestamp, uint32_t oldDocSize, + BucketId newBucketId, Timestamp newTimestamp, uint32_t newDocSize, SubDbType subDbType); - BucketState get(const BucketId &bucketId) const; - void cacheBucket(const BucketId &bucketId); + BucketState get(BucketId bucketId) const; + void cacheBucket(BucketId bucketId); void uncacheBucket(); - bool isCachedBucket(const BucketId &bucketId) const; - storage::spi::BucketInfo cachedGetBucketInfo(const BucketId &bucketId) const; - BucketState cachedGet(const BucketId &bucketId) const; - bool hasBucket(const BucketId &bucketId) const; + bool isCachedBucket(BucketId bucketId) const; + storage::spi::BucketInfo cachedGetBucketInfo(BucketId bucketId) const; + BucketState cachedGet(BucketId bucketId) const; + bool hasBucket(BucketId bucketId) const; BucketId::List getBuckets() const; bool empty() const; - void setBucketState(const BucketId &bucketId, bool active); - void createBucket(const BucketId &bucketId); - void deleteEmptyBucket(const BucketId &bucketId); + void setBucketState(BucketId bucketId, bool active); + void createBucket(BucketId bucketId); + void deleteEmptyBucket(BucketId bucketId); BucketId::List getActiveBuckets() const; BucketId::List populateActiveBuckets(BucketId::List buckets); - - ConstMapIterator begin() const { return _map.begin(); } - ConstMapIterator end() const { return _map.end(); } - ConstMapIterator lowerBound(const BucketId &bucket) const { return _map.lower_bound(bucket); } size_t size() const { return _map.size(); } - bool isActiveBucket(const BucketId &bucketId) const; - BucketState *getBucketStatePtr(const BucketId &bucket); - void unloadBucket(const BucketId &bucket, const BucketState &delta); + bool isActiveBucket(BucketId bucketId) const; + BucketState *getBucketStatePtr(BucketId bucket); + void unloadBucket(BucketId bucket, const BucketState &delta); }; } diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp deleted file mode 100644 index c691e786add..00000000000 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.cpp +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "bucketscaniterator.h" - -using document::BucketId; - -namespace proton::bucketdb { - -ScanIterator::ScanIterator(const Guard & db, BucketId bucket) - : _db(std::move(db)), - _itr(_db->lowerBound(bucket)), - _end(_db->end()) -{ } - -} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h deleted file mode 100644 index 1b68cfc1e59..00000000000 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketscaniterator.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include "bucket_db_owner.h" -#include "bucketdb.h" - -namespace proton::bucketdb { - - -class ScanIterator { -private: - using BucketId = document::BucketId; - using BucketIterator = BucketDB::ConstMapIterator; - const Guard &_db; - BucketIterator _itr; - BucketIterator _end; - -public: - ScanIterator(const Guard & db, BucketId bucket); - ScanIterator(const ScanIterator &) = delete; - ScanIterator(ScanIterator &&rhs) = delete; - ScanIterator &operator=(const ScanIterator &) = delete; - ScanIterator &operator=(ScanIterator &&rhs) = delete; - - bool valid() const { return _itr != _end; } - bool isActive() const { return _itr->second.isActive(); } - BucketId getBucket() const { return _itr->first; } - bool hasReadyBucketDocs() const { return _itr->second.getReadyCount() != 0; } - bool hasNotReadyBucketDocs() const { return _itr->second.getNotReadyCount() != 0; } - - ScanIterator & operator++() { - ++_itr; - return *this; - } -}; - -} diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h b/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h index 217fa2db1f5..3f50bf9faec 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_loop_communicator.h @@ -8,7 +8,7 @@ namespace proton::matching { -class MatchLoopCommunicator : public IMatchLoopCommunicator +class MatchLoopCommunicator final : public IMatchLoopCommunicator { private: using IDiversifier = search::queryeval::IDiversifier; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp index 985176bcef5..84ff0672fa0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp @@ -24,7 +24,7 @@ using vespalib::Issue; namespace { -struct TimedMatchLoopCommunicator : IMatchLoopCommunicator { +struct TimedMatchLoopCommunicator final : IMatchLoopCommunicator { IMatchLoopCommunicator &communicator; vespalib::Timer timer; vespalib::duration elapsed; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index e0449236af0..eecbf116e9d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -14,15 +14,12 @@ #include <vespa/vespalib/data/slime/inserter.h> #include <vespa/vespalib/util/issue.h> -using search::attribute::IAttributeContext; -using search::queryeval::IRequestContext; using search::queryeval::IDiversifier; using search::attribute::diversity::DiversityFilter; using search::attribute::BasicType; using search::attribute::AttributeBlueprintParams; using vespalib::Issue; -using namespace search::fef; using namespace search::fef::indexproperties::matchphase; using namespace search::fef::indexproperties::matching; using namespace search::fef::indexproperties; @@ -33,6 +30,9 @@ namespace proton::matching { namespace { +using search::fef::Properties; +using search::fef::RankSetup; + bool contains_all(const HandleRecorder::HandleMap &old_map, const HandleRecorder::HandleMap &new_map) { @@ -70,7 +70,7 @@ extractDiversityParams(const RankSetup &rankSetup, const Properties &rankPropert } // namespace proton::matching::<unnamed> void -MatchTools::setup(search::fef::RankProgram::UP rank_program, double termwise_limit) +MatchTools::setup(std::unique_ptr<RankProgram> rank_program, double termwise_limit) { if (_search) { _match_data->soft_reset(); @@ -240,12 +240,12 @@ std::unique_ptr<IDiversifier> MatchToolsFactory::createDiversifier(uint32_t heapSize) const { if ( !_diversityParams.enabled() ) { - return std::unique_ptr<IDiversifier>(); + return {}; } auto attr = _requestContext.getAttribute(_diversityParams.attribute); if ( !attr) { Issue::report("Skipping diversity due to no %s attribute.", _diversityParams.attribute.c_str()); - return std::unique_ptr<IDiversifier>(); + return {}; } size_t max_per_group = heapSize/_diversityParams.min_groups; return DiversityFilter::create(*attr, heapSize, max_per_group, _diversityParams.min_groups, @@ -302,10 +302,8 @@ MatchToolsFactory::get_feature_rename_map() const } AttributeBlueprintParams -MatchToolsFactory::extract_global_filter_params(const search::fef::RankSetup& rank_setup, - const search::fef::Properties& rank_properties, - uint32_t active_docids, - uint32_t docid_limit) +MatchToolsFactory::extract_global_filter_params(const RankSetup& rank_setup, const Properties& rank_properties, + uint32_t active_docids, uint32_t docid_limit) { double lower_limit = GlobalFilterLowerLimit::lookup(rank_properties, rank_setup.get_global_filter_lower_limit()); double upper_limit = GlobalFilterUpperLimit::lookup(rank_properties, rank_setup.get_global_filter_upper_limit()); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h index 11e943957ef..d7254d4b958 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h @@ -30,19 +30,25 @@ class MatchTools { private: using IRequestContext = search::queryeval::IRequestContext; - QueryLimiter &_queryLimiter; - const vespalib::Doom &_doom; - const Query &_query; - MaybeMatchPhaseLimiter &_match_limiter; - const QueryEnvironment &_queryEnv; - const search::fef::RankSetup &_rankSetup; - const search::fef::Properties &_featureOverrides; - std::unique_ptr<search::fef::MatchData> _match_data; - std::unique_ptr<search::fef::RankProgram> _rank_program; - search::queryeval::SearchIterator::UP _search; - HandleRecorder::HandleMap _used_handles; - bool _search_has_changed; - void setup(std::unique_ptr<search::fef::RankProgram>, double termwise_limit = 1.0); + using SearchIterator = search::queryeval::SearchIterator; + using MatchData = search::fef::MatchData; + using MatchDataLayout = search::fef::MatchDataLayout; + using Properties = search::fef::Properties; + using RankProgram = search::fef::RankProgram; + using RankSetup = search::fef::RankSetup; + QueryLimiter &_queryLimiter; + const vespalib::Doom &_doom; + const Query &_query; + MaybeMatchPhaseLimiter &_match_limiter; + const QueryEnvironment &_queryEnv; + const RankSetup &_rankSetup; + const Properties &_featureOverrides; + std::unique_ptr<MatchData> _match_data; + std::unique_ptr<RankProgram> _rank_program; + std::unique_ptr<SearchIterator> _search; + HandleRecorder::HandleMap _used_handles; + bool _search_has_changed; + void setup(std::unique_ptr<RankProgram>, double termwise_limit = 1.0); public: typedef std::unique_ptr<MatchTools> UP; MatchTools(const MatchTools &) = delete; @@ -52,19 +58,19 @@ public: const Query &query, MaybeMatchPhaseLimiter &match_limiter_in, const QueryEnvironment &queryEnv, - const search::fef::MatchDataLayout &mdl, - const search::fef::RankSetup &rankSetup, - const search::fef::Properties &featureOverrides); + const MatchDataLayout &mdl, + const RankSetup &rankSetup, + const Properties &featureOverrides); ~MatchTools(); const vespalib::Doom &getDoom() const { return _doom; } QueryLimiter & getQueryLimiter() { return _queryLimiter; } MaybeMatchPhaseLimiter &match_limiter() { return _match_limiter; } bool has_second_phase_rank() const; - const search::fef::MatchData &match_data() const { return *_match_data; } - search::fef::RankProgram &rank_program() { return *_rank_program; } - search::queryeval::SearchIterator &search() { return *_search; } - search::queryeval::SearchIterator::UP borrow_search() { return std::move(_search); } - void give_back_search(search::queryeval::SearchIterator::UP search_in) { _search = std::move(search_in); } + const MatchData &match_data() const { return *_match_data; } + RankProgram &rank_program() { return *_rank_program; } + SearchIterator &search() { return *_search; } + std::unique_ptr<SearchIterator> borrow_search() { return std::move(_search); } + void give_back_search(std::unique_ptr<SearchIterator> search_in) { _search = std::move(search_in); } void tag_search_as_changed() { _search_has_changed = true; } void setup_first_phase(); void setup_second_phase(); @@ -92,17 +98,25 @@ class MatchToolsFactory { private: using IAttributeFunctor = search::attribute::IAttributeFunctor; - QueryLimiter & _queryLimiter; - search::attribute::AttributeBlueprintParams _global_filter_params; - Query _query; - MaybeMatchPhaseLimiter::UP _match_limiter; - QueryEnvironment _queryEnv; - RequestContext _requestContext; - search::fef::MatchDataLayout _mdl; - const search::fef::RankSetup & _rankSetup; - const search::fef::Properties & _featureOverrides; - DiversityParams _diversityParams; - bool _valid; + using IAttributeContext = search::attribute::IAttributeContext; + using AttributeBlueprintParams = search::attribute::AttributeBlueprintParams; + using MatchDataLayout = search::fef::MatchDataLayout; + using Properties = search::fef::Properties; + using RankProgram = search::fef::RankProgram; + using RankSetup = search::fef::RankSetup; + using IIndexEnvironment = search::fef::IIndexEnvironment; + using IDiversifier = search::queryeval::IDiversifier; + QueryLimiter & _queryLimiter; + AttributeBlueprintParams _global_filter_params; + Query _query; + MaybeMatchPhaseLimiter::UP _match_limiter; + QueryEnvironment _queryEnv; + RequestContext _requestContext; + MatchDataLayout _mdl; + const RankSetup & _rankSetup; + const Properties & _featureOverrides; + DiversityParams _diversityParams; + bool _valid; std::unique_ptr<AttributeOperationTask> createTask(vespalib::stringref attribute, vespalib::stringref operation) const; @@ -114,23 +128,23 @@ public: MatchToolsFactory(QueryLimiter & queryLimiter, const vespalib::Doom & softDoom, ISearchContext &searchContext, - search::attribute::IAttributeContext &attributeContext, + IAttributeContext &attributeContext, search::engine::Trace & root_trace, vespalib::stringref queryStack, const vespalib::string &location, const ViewResolver &viewResolver, const search::IDocumentMetaStore &metaStore, - const search::fef::IIndexEnvironment &indexEnv, - const search::fef::RankSetup &rankSetup, - const search::fef::Properties &rankProperties, - const search::fef::Properties &featureOverrides, + const IIndexEnvironment &indexEnv, + const RankSetup &rankSetup, + const Properties &rankProperties, + const Properties &featureOverrides, bool is_search); ~MatchToolsFactory(); bool valid() const { return _valid; } const MaybeMatchPhaseLimiter &match_limiter() const { return *_match_limiter; } MatchTools::UP createMatchTools() const; bool should_diversify() const { return _diversityParams.enabled(); } - std::unique_ptr<search::queryeval::IDiversifier> createDiversifier(uint32_t heapSize) const; + std::unique_ptr<IDiversifier> createDiversifier(uint32_t heapSize) const; search::queryeval::Blueprint::HitEstimate estimate() const { return _query.estimate(); } bool has_first_phase_rank() const; bool has_match_features() const; @@ -151,11 +165,9 @@ public: * When searchable-copies > 1, we must scale the parameters to match the effective range of the estimated hit ratio. * This is done by multiplying with the active hit ratio (active docids / docid limit). */ - static search::attribute::AttributeBlueprintParams - extract_global_filter_params(const search::fef::RankSetup& rank_setup, - const search::fef::Properties& rank_properties, - uint32_t active_docids, - uint32_t docid_limit); + static AttributeBlueprintParams + extract_global_filter_params(const RankSetup& rank_setup, const Properties& rank_properties, + uint32_t active_docids, uint32_t docid_limit); }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp index 45cac1d02e9..415000c3f9b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp @@ -9,6 +9,7 @@ #include "document_db_maintenance_config.h" #include <vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h> #include <vespa/searchcore/proton/bucketdb/i_bucket_create_notifier.h> +#include <vespa/searchcore/proton/bucketdb/bucket_db_owner.h> #include <vespa/searchcore/proton/feedoperation/moveoperation.h> #include <vespa/searchcore/proton/documentmetastore/i_document_meta_store.h> #include <vespa/searchcorespi/index/i_thread_service.h> @@ -141,7 +142,7 @@ BucketMoveJob::create(const std::shared_ptr<IBucketStateCalculator> &calc, } BucketMoveJob::NeedResult -BucketMoveJob::needMove(const ScanIterator &itr) const { +BucketMoveJob::needMove(BucketId bucketId, const BucketStateWrapper &itr) const { NeedResult noMove(false, false); const bool hasReadyDocs = itr.hasReadyBucketDocs(); const bool hasNotReadyDocs = itr.hasNotReadyBucketDocs(); @@ -154,13 +155,13 @@ BucketMoveJob::needMove(const ScanIterator &itr) const { if (!_calc || (_calc->nodeRetired() && !isActive)) { return noMove; } - const Trinary shouldBeReady = _calc->shouldBeReady(document::Bucket(_bucketSpace, itr.getBucket())); + const Trinary shouldBeReady = _calc->shouldBeReady(document::Bucket(_bucketSpace, bucketId)); if (shouldBeReady == Trinary::Undefined) { return noMove; } const bool wantReady = (shouldBeReady == Trinary::True) || isActive; LOG(spam, "needMove(): bucket(%s), shouldBeReady(%s), active(%s)", - itr.getBucket().toString().c_str(), toStr(shouldBeReady), toStr(isActive)); + bucketId.toString().c_str(), toStr(shouldBeReady), toStr(isActive)); if (wantReady) { if (!hasNotReadyDocs) { return noMove; // No notready bucket to make ready @@ -301,8 +302,7 @@ BucketMoveJob::considerBucket(const bucketdb::Guard & guard, BucketId bucket) { void BucketMoveJob::reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket) { assert( ! _bucketsInFlight.contains(bucket)); - ScanIterator itr(guard, bucket); - auto [mustMove, wantReady] = needMove(itr); + auto [mustMove, wantReady] = needMove(bucket, guard->get(bucket)); if (mustMove) { _buckets2Move[bucket] = wantReady; } else { @@ -322,10 +322,11 @@ BucketMoveJob::BucketMoveSet BucketMoveJob::computeBuckets2Move(const bucketdb::Guard & guard) { BucketMoveJob::BucketMoveSet toMove; - for (ScanIterator itr(guard, BucketId()); itr.valid(); ++itr) { - auto [mustMove, wantReady] = needMove(itr); + BucketId::List buckets = guard->getBuckets(); + for (BucketId bucketId : buckets) { + auto [mustMove, wantReady] = needMove(bucketId, guard->get(bucketId)); if (mustMove) { - toMove[itr.getBucket()] = wantReady; + toMove[bucketId] = wantReady; } } return toMove; diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h index d4438fcd411..9885b581a24 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h @@ -8,9 +8,9 @@ #include "ibucketstatechangedhandler.h" #include "iclusterstatechangedhandler.h" #include "maintenancedocumentsubdb.h" -#include <vespa/searchcore/proton/bucketdb/bucketscaniterator.h> #include <vespa/searchcore/proton/bucketdb/i_bucket_create_listener.h> #include <vespa/vespalib/util/retain_guard.h> +#include <map> namespace storage::spi { struct BucketExecutor; } @@ -49,7 +49,6 @@ private: using IDestructorCallbackSP = std::shared_ptr<IDestructorCallback>; using IThreadService = searchcorespi::index::IThreadService; using BucketId = document::BucketId; - using ScanIterator = bucketdb::ScanIterator; using BucketMoveSet = std::map<BucketId, bool>; using NeedResult = std::pair<bool, bool>; using ActiveState = storage::spi::BucketInfo::ActiveState; @@ -79,6 +78,17 @@ private: IBucketStateChangedNotifier &_bucketStateChangedNotifier; IDiskMemUsageNotifier &_diskMemUsageNotifier; + class BucketStateWrapper { + private: + const bucketdb::BucketState & _state; + + public: + BucketStateWrapper(const bucketdb::BucketState & state) noexcept : _state(state) {} + + bool isActive() const noexcept { return _state.isActive(); } + bool hasReadyBucketDocs() const noexcept { return _state.getReadyCount() != 0; } + bool hasNotReadyBucketDocs() const noexcept { return _state.getNotReadyCount() != 0; } + }; BucketMoveJob(const std::shared_ptr<IBucketStateCalculator> &calc, vespalib::RetainGuard dbRetainer, IDocumentMoveHandler &moveHandler, @@ -103,7 +113,7 @@ private: void reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket); void updatePending(); void cancelBucket(BucketId bucket); // True if something to cancel - NeedResult needMove(const ScanIterator &itr) const; + NeedResult needMove(BucketId bucketId, const BucketStateWrapper &itr) const; BucketMoveSet computeBuckets2Move(const bucketdb::Guard & guard); BucketMoverSP createMover(BucketId bucket, bool wantReady); BucketMoverSP greedyCreateMover(); diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 3fdf82ea61d..783282b71c1 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -1025,11 +1025,12 @@ notifyBucketsChanged(const documentmetastore::IBucketHandler &metaStore, IBucketModifiedHandler &handler, const vespalib::string &name) { - bucketdb::Guard buckets = metaStore.getBucketDB().takeGuard(); - for (const auto &kv : *buckets) { - handler.notifyBucketModified(kv.first); + bucketdb::Guard guard = metaStore.getBucketDB().takeGuard(); + document::BucketId::List buckets = guard->getBuckets(); + for (document::BucketId bucketId : buckets) { + handler.notifyBucketModified(bucketId); } - LOG(debug, "notifyBucketsChanged(%s, %zu)", name.c_str(), buckets->size()); + LOG(debug, "notifyBucketsChanged(%s, %zu)", name.c_str(), buckets.size()); } } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp index 207f1d813d8..7a78b4ba82a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp @@ -34,7 +34,7 @@ SearchableFeedView::SearchableFeedView(StoreOnlyFeedView::Context storeOnlyCtx, const FastAccessFeedView::Context &fastUpdateCtx, Context ctx) : Parent(std::move(storeOnlyCtx), params, fastUpdateCtx), _indexWriter(ctx._indexWriter), - _hasIndexedFields(_schema->getNumIndexFields() > 0) + _hasIndexedFields(getSchema()->getNumIndexFields() > 0) { } SearchableFeedView::~SearchableFeedView() = default; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index a537742b79b..a9850b5c2b7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -7,14 +7,15 @@ #include "putdonecontext.h" #include "removedonecontext.h" #include "updatedonecontext.h" -#include <vespa/document/datatype/documenttype.h> -#include <vespa/document/fieldvalue/document.h> -#include <vespa/document/repo/documenttyperepo.h> #include <vespa/searchcore/proton/attribute/ifieldupdatecallback.h> #include <vespa/searchcore/proton/common/feedtoken.h> #include <vespa/searchcore/proton/feedoperation/operations.h> #include <vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h> #include <vespa/searchcore/proton/reference/i_pending_gid_to_lid_changes.h> +#include <vespa/searchlib/index/uri_field.h> +#include <vespa/document/repo/documenttyperepo.h> +#include <vespa/document/datatype/documenttype.h> +#include <vespa/document/fieldvalue/document.h> #include <vespa/vespalib/util/cpu_usage.h> #include <vespa/vespalib/util/destructor_callbacks.h> #include <vespa/vespalib/util/exceptions.h> @@ -38,7 +39,7 @@ using vespalib::CpuUsage; using vespalib::IDestructorCallback; using vespalib::IllegalStateException; using vespalib::makeLambdaTask; -using vespalib::make_string; +using vespalib::make_string_short::fmt; namespace proton { @@ -88,8 +89,8 @@ SummaryPutDoneContext::SummaryPutDoneContext(FeedToken token, IPendingLidTracker SummaryPutDoneContext::~SummaryPutDoneContext() = default; -std::vector<document::GlobalId> getGidsToRemove(const IDocumentMetaStore &metaStore, - const LidVectorContext::LidVector &lidsToRemove) +std::vector<document::GlobalId> +getGidsToRemove(const IDocumentMetaStore &metaStore, const LidVectorContext::LidVector &lidsToRemove) { std::vector<document::GlobalId> gids; gids.reserve(lidsToRemove.size()); @@ -102,23 +103,24 @@ std::vector<document::GlobalId> getGidsToRemove(const IDocumentMetaStore &metaSt return gids; } -void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, - const DocumentOperation &op, bool is_removed_doc) +void +putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, + const DocumentOperation &op, bool is_removed_doc) { documentmetastore::IStore::Result putRes( meta_store.put(doc_id.getGlobalId(), op.getBucketId(), op.getTimestamp(), op.getSerializedDocSize(), op.getLid(), op.get_prepare_serial_num())); if (!putRes.ok()) { - throw IllegalStateException( - make_string("Could not put <lid, gid> pair for %sdocument with id '%s' and gid '%s'", - is_removed_doc ? "removed " : "", doc_id.toString().c_str(), - doc_id.getGlobalId().toString().c_str())); + throw IllegalStateException(fmt("Could not put <lid, gid> pair for %sdocument with id '%s' and gid '%s'", + is_removed_doc ? "removed " : "", doc_id.toString().c_str(), + doc_id.getGlobalId().toString().c_str())); } assert(op.getLid() == putRes._lid); } -void removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, - const DocumentOperation &op, bool is_removed_doc) +void +removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id, + const DocumentOperation &op, bool is_removed_doc) { assert(meta_store.validLid(op.getPrevLid())); assert(is_removed_doc == op.getPrevMarkedAsRemoved()); @@ -127,9 +129,8 @@ void removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, (void) meta; if (!meta_store.remove(op.getPrevLid(), op.get_prepare_serial_num())) { throw IllegalStateException( - make_string("Could not remove <lid, gid> pair for %sdocument with id '%s' and gid '%s'", - is_removed_doc ? "removed " : "", doc_id.toString().c_str(), - gid.toString().c_str())); + fmt("Could not remove <lid, gid> pair for %sdocument with id '%s' and gid '%s'", + is_removed_doc ? "removed " : "", doc_id.toString().c_str(), gid.toString().c_str())); } } @@ -147,6 +148,37 @@ moveMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, c meta_store.move(op.getPrevLid(), op.getLid(), op.get_prepare_serial_num()); } +class UpdateScope final : public IFieldUpdateCallback +{ +private: + const vespalib::hash_set<int32_t> & _indexedFields; + bool _nonAttributeFields; +public: + bool _hasIndexedFields; + + UpdateScope(const vespalib::hash_set<int32_t> & indexedFields, const DocumentUpdate & upd); + bool hasIndexOrNonAttributeFields() const { + return _hasIndexedFields || _nonAttributeFields; + } + void onUpdateField(const document::Field & field, const search::AttributeVector * attr) override; +}; + +UpdateScope::UpdateScope(const vespalib::hash_set<int32_t> & indexedFields, const DocumentUpdate & upd) + : _indexedFields(indexedFields), + _nonAttributeFields(!upd.getFieldPathUpdates().empty()), + _hasIndexedFields(false) +{} + +void +UpdateScope::onUpdateField(const document::Field & field, const search::AttributeVector * attr) { + if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { + _nonAttributeFields = true; + } + if (!_hasIndexedFields && (_indexedFields.find(field.getId()) != _indexedFields.end())) { + _hasIndexedFields = true; + } +} + } // namespace StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms) @@ -160,12 +192,26 @@ StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams ¶ms _pendingLidsForDocStore(), _pendingLidsForCommit(std::move(ctx._pendingLidsForCommit)), _schema(std::move(ctx._schema)), + _indexedFields(), _writeService(ctx._writeService), _params(params), _metaStore(_documentMetaStoreContext->get()), _gidToLidChangeHandler(ctx._gidToLidChangeHandler) { _docType = _repo->getDocumentType(_params._docTypeName.getName()); + if (_schema && _docType) { + for (const auto &indexField : _schema->getIndexFields()) { + size_t dotPos = indexField.getName().find('.'); + if ((dotPos == vespalib::string::npos) || search::index::UriField::mightBePartofUri(indexField.getName())) { + document::FieldPath fieldPath; + _docType->buildFieldPath(fieldPath, indexField.getName().substr(0, dotPos)); + _indexedFields.insert(fieldPath.back().getFieldRef().getId()); + } else { + throw IllegalStateException("Field '%s' is not a valid index name", indexField.getName().c_str()); + } + + } + } } StoreOnlyFeedView::~StoreOnlyFeedView() = default; @@ -207,7 +253,7 @@ void StoreOnlyFeedView::putAttributes(SerialNum, Lid, const Document &, OnPutDoneType) {} void -StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const Document::SP &, OnOperationDoneType) {} +StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const std::shared_ptr<Document> &, OnOperationDoneType) {} void StoreOnlyFeedView::preparePut(PutOperation &putOp) @@ -285,7 +331,7 @@ StoreOnlyFeedView::updateAttributes(SerialNum, Lid, const DocumentUpdate & upd, OnOperationDoneType, IFieldUpdateCallback & onUpdate) { for (const auto & fieldUpdate : upd.getUpdates()) { - onUpdate.onUpdateField(fieldUpdate.getField().getName(), nullptr); + onUpdate.onUpdateField(fieldUpdate.getField(), nullptr); } } @@ -387,22 +433,6 @@ StoreOnlyFeedView::heartBeatSummary(SerialNum serialNum, DoneCallback onDone) { })); } -StoreOnlyFeedView::UpdateScope::UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd) - : _schema(&schema), - _indexedFields(false), - _nonAttributeFields(!upd.getFieldPathUpdates().empty()) -{} - -void -StoreOnlyFeedView::UpdateScope::onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) { - if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) { - _nonAttributeFields = true; - } - if (!_indexedFields && _schema->isIndexField(fieldName)) { - _indexedFields = true; - } -} - void StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) { if ( ! updOp.getUpdate()) { @@ -432,7 +462,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) } auto onWriteDone = createUpdateDoneContext(std::move(token), get_pending_lid_token(updOp), updOp.getUpdate()); - UpdateScope updateScope(*_schema, upd); + UpdateScope updateScope(_indexedFields, upd); updateAttributes(serialNum, lid, upd, onWriteDone, updateScope); if (updateScope.hasIndexOrNonAttributeFields()) { @@ -440,7 +470,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) FutureDoc futureDoc = promisedDoc.get_future().share(); onWriteDone->setDocument(futureDoc); _pendingLidsForDocStore.waitComplete(lid); - if (updateScope._indexedFields) { + if (updateScope._hasIndexedFields) { updateIndexedFields(serialNum, lid, futureDoc, onWriteDone); } PromisedStream promisedStream; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h index 2822aa70525..25a98da7ce7 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h @@ -9,7 +9,6 @@ #include "searchcontext.h" #include <vespa/searchcore/proton/common/pendinglidtracker.h> #include <vespa/searchcore/proton/common/doctypename.h> -#include <vespa/searchcore/proton/attribute/ifieldupdatecallback.h> #include <vespa/searchcore/proton/common/feeddebugger.h> #include <vespa/searchcore/proton/documentmetastore/documentmetastore.h> #include <vespa/searchcore/proton/documentmetastore/documentmetastorecontext.h> @@ -18,9 +17,10 @@ #include <vespa/searchcore/proton/persistenceengine/resulthandler.h> #include <vespa/searchcorespi/index/ithreadingservice.h> #include <vespa/searchlib/query/base.h> +#include <vespa/searchcore/proton/feedoperation/operations.h> #include <vespa/vespalib/util/threadstackexecutorbase.h> +#include <vespa/vespalib/stllike/hash_set.h> #include <future> -#include <vespa/searchcore/proton/feedoperation/operations.h> namespace vespalib { class IDestructorCallback; } @@ -118,22 +118,6 @@ public: {} }; -protected: - class UpdateScope : public IFieldUpdateCallback - { - private: - const search::index::Schema *_schema; - public: - bool _indexedFields; - bool _nonAttributeFields; - - UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd); - bool hasIndexOrNonAttributeFields() const { - return _indexedFields || _nonAttributeFields; - } - void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) override; - }; - private: const ISummaryAdapter::SP _summaryAdapter; const IDocumentMetaStoreContext::SP _documentMetaStoreContext; @@ -142,9 +126,9 @@ private: LidReuseDelayer _lidReuseDelayer; PendingLidTracker _pendingLidsForDocStore; std::shared_ptr<PendingLidTrackerBase> _pendingLidsForCommit; - + const search::index::Schema::SP _schema; + vespalib::hash_set<int32_t> _indexedFields; protected: - const search::index::Schema::SP _schema; searchcorespi::index::IThreadingService &_writeService; PersistentParams _params; IDocumentMetaStore &_metaStore; diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h b/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h index 9e0e8f66dc0..d8d6e119d9b 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h @@ -11,7 +11,7 @@ struct DummyDocumentStore : public search::IDocumentStore vespalib::string _baseDir; DummyDocumentStore() = default; - DummyDocumentStore(const vespalib::string &baseDir) + DummyDocumentStore(const vespalib::string &baseDir) noexcept : _baseDir(baseDir) {} ~DummyDocumentStore() = default; diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h b/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h index 4d3513c52f5..f826cbbe921 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_summary_manager.h @@ -9,7 +9,7 @@ namespace test { struct DummySummaryManager : public ISummaryManager { - virtual ISummarySetup::SP + ISummarySetup::SP createSummarySetup(const vespa::config::search::SummaryConfig &, const vespa::config::search::SummarymapConfig &, const vespa::config::search::summary::JuniperrcConfig &, diff --git a/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp b/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp index 75cccb0d573..9f1027d0522 100644 --- a/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp +++ b/searchlib/src/tests/index/docbuilder/docbuilder_test.cpp @@ -13,8 +13,7 @@ LOG_SETUP("docbuilder_test"); using namespace document; using search::index::schema::CollectionType; -namespace search { -namespace index { +namespace search::index { namespace { @@ -26,15 +25,8 @@ namespace linguistics const vespalib::string SPANTREE_NAME("linguistics"); } -class Test : public vespalib::TestApp { -private: - void testBuilder(); -public: - int Main() override; -}; -void -Test::testBuilder() +TEST("test docBuilder") { Schema s; s.addIndexField(Schema::IndexField("ia", schema::DataType::STRING)); @@ -415,7 +407,7 @@ Test::testBuilder() EXPECT_EQUAL("</af>", *itr++); EXPECT_EQUAL("</document>", *itr++); EXPECT_TRUE(itr == lines.end()); -#if 1 +#if 0 std::cout << "onedoc xml start -----" << std::endl << xml << std::endl << "-------" << std::endl; @@ -479,7 +471,7 @@ Test::testBuilder() expSpans.push_back(Span(15, 9)); expSpans.push_back(Span(15, 9)); ASSERT_TRUE(expSpans == spans); -#if 1 +#if 0 std::cout << "onedoc xml start -----" << std::endl << xml << std::endl << "-------" << std::endl; @@ -490,18 +482,21 @@ Test::testBuilder() } } -int -Test::Main() -{ - TEST_INIT("docbuilder_test"); - - testBuilder(); - - TEST_DONE(); +TEST("test if index names are valid uri parts") { + EXPECT_FALSE(UriField::mightBePartofUri("all")); + EXPECT_FALSE(UriField::mightBePartofUri("fragment")); + EXPECT_FALSE(UriField::mightBePartofUri(".all")); + EXPECT_FALSE(UriField::mightBePartofUri("all.b")); + EXPECT_TRUE(UriField::mightBePartofUri("b.all")); + EXPECT_TRUE(UriField::mightBePartofUri("b.scheme")); + EXPECT_TRUE(UriField::mightBePartofUri("b.host")); + EXPECT_TRUE(UriField::mightBePartofUri("b.port")); + EXPECT_TRUE(UriField::mightBePartofUri("b.hostname")); + EXPECT_TRUE(UriField::mightBePartofUri("b.path")); + EXPECT_TRUE(UriField::mightBePartofUri("b.query")); + EXPECT_TRUE(UriField::mightBePartofUri("b.fragment")); } } -} - -TEST_APPHOOK(search::index::Test); +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp b/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp index 77cc7920ea8..0fef721a6fa 100644 --- a/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp +++ b/searchlib/src/tests/queryeval/dot_product/dot_product_test.cpp @@ -263,8 +263,11 @@ TEST_F("test Eager Matching Children", MockFixture(5)) { class IteratorChildrenVerifier : public search::test::IteratorChildrenVerifier { private: - SearchIterator::UP create(const std::vector<SearchIterator*> &children) const override { - std::vector<fef::TermFieldMatchData*> no_child_match; + SearchIterator::UP + create(const std::vector<SearchIterator*> &children) const override { + // This is a pragmatic and dirty workaround to make IteratorVerifier test + // not fail on unpack when accessing child match weights + std::vector<fef::TermFieldMatchData*> no_child_match(children.size(), &_tfmd); MatchData::UP no_match_data; return DotProductSearch::create(children, _tfmd, false, no_child_match, _weights, std::move(no_match_data)); } @@ -272,8 +275,9 @@ private: class WeightIteratorChildrenVerifier : public search::test::DwaIteratorChildrenVerifier { private: - SearchIterator::UP create(std::vector<DocumentWeightIterator> && children) const override { - return SearchIterator::UP(DotProductSearch::create(_tfmd, false, _weights, std::move(children))); + SearchIterator::UP + create(std::vector<DocumentWeightIterator> && children) const override { + return DotProductSearch::create(_tfmd, false, _weights, std::move(children)); } }; diff --git a/searchlib/src/vespa/searchlib/index/uri_field.cpp b/searchlib/src/vespa/searchlib/index/uri_field.cpp index 380f67210d3..070afc94837 100644 --- a/searchlib/src/vespa/searchlib/index/uri_field.cpp +++ b/searchlib/src/vespa/searchlib/index/uri_field.cpp @@ -18,9 +18,7 @@ UriField::UriField() } bool -UriField::valid(const Schema &schema, - uint32_t fieldId, - const Schema::CollectionType &collectionType) +UriField::valid(const Schema &schema, uint32_t fieldId, const Schema::CollectionType &collectionType) { if (fieldId == Schema::UNKNOWN_FIELD_ID) { return false; @@ -36,9 +34,7 @@ UriField::valid(const Schema &schema, } bool -UriField::broken(const Schema &schema, - const Schema::CollectionType & - collectionType) const +UriField::broken(const Schema &schema, const Schema::CollectionType & collectionType) const { return !valid(schema, _all, collectionType) && valid(schema, _scheme, collectionType) && @@ -50,9 +46,7 @@ UriField::broken(const Schema &schema, } bool -UriField::valid(const Schema &schema, - const Schema::CollectionType & - collectionType) const +UriField::valid(const Schema &schema, const Schema::CollectionType & collectionType) const { return valid(schema, _all, collectionType) && valid(schema, _scheme, collectionType) && @@ -64,8 +58,7 @@ UriField::valid(const Schema &schema, } void -UriField::setup(const Schema &schema, - const vespalib::string &field) +UriField::setup(const Schema &schema, const vespalib::string &field) { _all = schema.getIndexFieldId(field); _scheme = schema.getIndexFieldId(field + ".scheme"); @@ -77,9 +70,19 @@ UriField::setup(const Schema &schema, _hostname = schema.getIndexFieldId(field + ".hostname"); } +bool +UriField::mightBePartofUri(vespalib::stringref name) { + size_t dotPos = name.find('.'); + if ((dotPos != 0) && (dotPos != vespalib::string::npos)) { + vespalib::stringref suffix = name.substr(dotPos + 1); + return ((suffix == "all") || (suffix == "scheme") || (suffix == "host") || (suffix == "port") || + (suffix == "path") || (suffix == "query") || (suffix == "fragment") || (suffix == "hostname")); + } + return false; +} + void -UriField::markUsed(UsedFieldsMap &usedFields, - uint32_t field) +UriField::markUsed(UsedFieldsMap &usedFields, uint32_t field) { if (field == Schema::UNKNOWN_FIELD_ID) { return; diff --git a/searchlib/src/vespa/searchlib/index/uri_field.h b/searchlib/src/vespa/searchlib/index/uri_field.h index 9b8e4e72b7c..70bf8c01a8c 100644 --- a/searchlib/src/vespa/searchlib/index/uri_field.h +++ b/searchlib/src/vespa/searchlib/index/uri_field.h @@ -35,6 +35,7 @@ public: bool valid(const Schema &schema, const Schema::CollectionType &collectionType) const; void setup(const Schema &schema, const vespalib::string &field); void markUsed(UsedFieldsMap &usedFields) const; + static bool mightBePartofUri(vespalib::stringref name); }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp b/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp index 393140784b0..34cc343b16d 100644 --- a/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/dot_product_search.cpp @@ -169,9 +169,9 @@ DotProductSearch::create(const std::vector<SearchIterator*> &children, *childMatch[0], weights[0], std::move(md)); } if (childMatch.size() < 128) { - return SearchIterator::UP(new ArrayHeapImpl(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md)))); + return std::make_unique<ArrayHeapImpl>(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md))); } - return SearchIterator::UP(new HeapImpl(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md)))); + return std::make_unique<HeapImpl>(tmd, field_is_filter, weights, SearchIteratorPack(children, childMatch, std::move(md))); } //----------------------------------------------------------------------------- @@ -186,9 +186,9 @@ DotProductSearch::create(TermFieldMatchData &tmd, typedef DotProductSearchImpl<vespalib::LeftHeap, AttributeIteratorPack> HeapImpl; if (iterators.size() < 128) { - return SearchIterator::UP(new ArrayHeapImpl(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators)))); + return std::make_unique<ArrayHeapImpl>(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators))); } - return SearchIterator::UP(new HeapImpl(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators)))); + return std::make_unique<HeapImpl>(tmd, field_is_filter, weights, AttributeIteratorPack(std::move(iterators))); } //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/test/initrange.cpp b/searchlib/src/vespa/searchlib/test/initrange.cpp index 5154a8abb08..d082bfd4e78 100644 --- a/searchlib/src/vespa/searchlib/test/initrange.cpp +++ b/searchlib/src/vespa/searchlib/test/initrange.cpp @@ -72,7 +72,7 @@ InitRangeVerifier::InitRangeVerifier() : } } -InitRangeVerifier::~InitRangeVerifier() { } +InitRangeVerifier::~InitRangeVerifier() = default; InitRangeVerifier::DocIds InitRangeVerifier::invert(const DocIds & docIds, uint32_t docIdlimit) @@ -173,6 +173,7 @@ InitRangeVerifier::searchRelaxed(SearchIterator & it, Range range) for (uint32_t docid = range.first; docid < range.second; ++docid) { if (it.seek(docid)) { result.emplace_back(docid); + it.unpack(docid); } } return result; @@ -185,6 +186,7 @@ InitRangeVerifier::searchStrict(SearchIterator & it, Range range) it.initRange(range.first, range.second); for (uint32_t docId = it.seekFirst(range.first); docId < range.second; docId = it.seekNext(docId + 1)) { result.push_back(docId); + it.unpack(docId); } return result; } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp index 024046b679b..d9878cd2057 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp @@ -375,40 +375,35 @@ DynamicTeaserDFW::makeDynamicTeaser(uint32_t docid, vespalib::stringref input, G state->_dynteaser._query = _juniper->CreateQueryHandle(iq, nullptr); } - if (docid != state->_dynteaser._docid || - _inputFieldEnumValue != state->_dynteaser._input || - _langFieldEnumValue != state->_dynteaser._lang || - !juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config)) { - LOG(debug, "makeDynamicTeaser: docid (%d,%d), fieldenum (%d,%d), lang (%d,%d) analyse %s", - docid, state->_dynteaser._docid, - _inputFieldEnumValue, state->_dynteaser._input, - _langFieldEnumValue, state->_dynteaser._lang, - (juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config) ? "no" : "yes")); - - if (state->_dynteaser._result != nullptr) - juniper::ReleaseResult(state->_dynteaser._result); - - state->_dynteaser._docid = docid; - state->_dynteaser._input = _inputFieldEnumValue; - state->_dynteaser._lang = _langFieldEnumValue; - state->_dynteaser._config = _juniperConfig.get(); - state->_dynteaser._result = nullptr; - - if (state->_dynteaser._query != nullptr) { - - if (LOG_WOULD_LOG(spam)) { - std::ostringstream hexDump; - hexDump << vespalib::HexDump(input.data(), input.length()); - LOG(spam, "makeDynamicTeaser: docid=%d, input='%s', hexdump:\n%s", - docid, std::string(input.data(), input.length()).c_str(), hexDump.str().c_str()); - } + LOG(debug, "makeDynamicTeaser: docid (%d,%d), fieldenum (%d,%d), lang (%d,%d) analyse %s", + docid, state->_dynteaser._docid, + _inputFieldEnumValue, state->_dynteaser._input, + _langFieldEnumValue, state->_dynteaser._lang, + (juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config) ? "no" : "yes")); + + if (state->_dynteaser._result != nullptr) + juniper::ReleaseResult(state->_dynteaser._result); + + state->_dynteaser._docid = docid; + state->_dynteaser._input = _inputFieldEnumValue; + state->_dynteaser._lang = _langFieldEnumValue; + state->_dynteaser._config = _juniperConfig.get(); + state->_dynteaser._result = nullptr; + + if (state->_dynteaser._query != nullptr) { + + if (LOG_WOULD_LOG(spam)) { + std::ostringstream hexDump; + hexDump << vespalib::HexDump(input.data(), input.length()); + LOG(spam, "makeDynamicTeaser: docid=%d, input='%s', hexdump:\n%s", + docid, std::string(input.data(), input.length()).c_str(), hexDump.str().c_str()); + } - auto langid = static_cast<uint32_t>(-1); + auto langid = static_cast<uint32_t>(-1); - state->_dynteaser._result = - juniper::Analyse(_juniperConfig.get(), state->_dynteaser._query, - input.data(), input.length(), docid, _inputFieldEnumValue, langid); - } + state->_dynteaser._result = + juniper::Analyse(_juniperConfig.get(), state->_dynteaser._query, + input.data(), input.length(), docid, _inputFieldEnumValue, langid); } juniper::Summary *teaser = (state->_dynteaser._result != nullptr) diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index ddd2457ec49..c2bbf4d09da 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -936,7 +936,6 @@ SearchVisitor::handleDocument(StorageDocument & document) group(document.docDoc(), rp.getRankScore(), false); if (amongTheBest) { - document.saveCachedFields(); needToKeepDocument = true; } diff --git a/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp b/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp index a0d666268f5..b73b6be2862 100644 --- a/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp +++ b/streamingvisitors/src/vespa/vsm/common/storagedocument.cpp @@ -17,28 +17,31 @@ StorageDocument::StorageDocument(document::Document::UP doc, const SharedFieldPa _fieldMap(fim), _cachedFields(getFieldCount()), _backedFields() -{ } +{ + _backedFields.reserve(getFieldCount()); +} -StorageDocument::~StorageDocument() { } +StorageDocument::~StorageDocument() = default; namespace { FieldPath _emptyFieldPath; - StorageDocument::SubDocument _empySubDocument(NULL, _emptyFieldPath.getFullRange()); + StorageDocument::SubDocument _empySubDocument(nullptr, _emptyFieldPath.getFullRange()); } const StorageDocument::SubDocument & StorageDocument::getComplexField(FieldIdT fId) const { - if (_cachedFields[fId].getFieldValue() == NULL) { + if (_cachedFields[fId].getFieldValue() == nullptr) { const FieldPath & fp = (*_fieldMap)[fId]; if ( ! fp.empty() ) { const document::StructuredFieldValue * sfv = _doc.get(); NestedIterator nested = fp.getFullRange(); const document::FieldPathEntry& fvInfo = nested.cur(); - bool ok = sfv->getValue(fvInfo.getFieldRef(), fvInfo.getFieldValueToSet()); - if (ok) { - SubDocument tmp(&fvInfo.getFieldValueToSet(), nested.next()); + document::FieldValue::UP fv = sfv->getValue(fvInfo.getFieldRef()); + if (fv) { + SubDocument tmp(fv.get(), nested.next()); _cachedFields[fId].swap(tmp); + _backedFields.push_back(std::move(fv)); } } else { LOG(debug, "Failed getting field fId %d.", fId); @@ -48,18 +51,6 @@ StorageDocument::getComplexField(FieldIdT fId) const return _cachedFields[fId]; } -void StorageDocument::saveCachedFields() const -{ - size_t m(_cachedFields.size()); - _backedFields.reserve(m); - for (size_t i(0); i < m; i++) { - if (_cachedFields[i].getFieldValue() != 0) { - _backedFields.emplace_back(document::FieldValue::UP(_cachedFields[i].getFieldValue()->clone())); - _cachedFields[i].setFieldValue(_backedFields.back().get()); - } - } -} - const document::FieldValue * StorageDocument::getField(FieldIdT fId) const { diff --git a/streamingvisitors/src/vespa/vsm/common/storagedocument.h b/streamingvisitors/src/vespa/vsm/common/storagedocument.h index a7f21cb052f..0ac94157462 100644 --- a/streamingvisitors/src/vespa/vsm/common/storagedocument.h +++ b/streamingvisitors/src/vespa/vsm/common/storagedocument.h @@ -47,7 +47,6 @@ public: const SubDocument &getComplexField(FieldIdT fId) const; const document::FieldValue *getField(FieldIdT fId) const override; bool setField(FieldIdT fId, document::FieldValue::UP fv) override ; - void saveCachedFields() const; private: document::Document::UP _doc; SharedFieldPathMap _fieldMap; |