diff options
84 files changed, 1933 insertions, 809 deletions
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 948e416eb53..3137dfff606 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 @@ -183,10 +183,10 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd options.nodeStateRequestTimeoutEarliestPercentage, options.nodeStateRequestTimeoutLatestPercentage, options.nodeStateRequestRoundTripTimeMaxSeconds); - var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(), timer, options.zooKeeperServerAddress, timer); - var lookUp = new SlobrokClient(timer); - var stateGenerator = new StateChangeHandler(timer, log); - var stateBroadcaster = new SystemStateBroadcaster(timer, timer); + var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(context), timer, options.zooKeeperServerAddress, timer); + var lookUp = new SlobrokClient(context, timer); + var stateGenerator = new StateChangeHandler(context, timer, log); + var stateBroadcaster = new SystemStateBroadcaster(context, timer, timer); var masterElectionHandler = new MasterElectionHandler(context, options.fleetControllerIndex, options.fleetControllerCount, timer, timer); var controller = new FleetController(context, timer, log, cluster, stateGatherer, communicator, statusPageServer, null, lookUp, database, stateGenerator, diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContext.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContext.java index cc94dd88e60..d1aadf9d217 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContext.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContext.java @@ -16,4 +16,13 @@ public interface FleetControllerContext { default void log(Logger logger, Level level, String message) { log(logger, level, () -> message); } void log(Logger logger, Level level, String message, Throwable t); void log(Logger logger, Level level, Supplier<String> message); + + default void log(Logger logger, Level level, String format, Object first, Object... rest) { + log(logger, level, () -> { + var args = new Object[1 + rest.length]; + args[0] = first; + System.arraycopy(rest, 0, args, 1, rest.length); + return String.format(format, args); + }); + } } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandler.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandler.java index 5035ed1aa88..46fafddfade 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandler.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandler.java @@ -28,6 +28,7 @@ public class StateChangeHandler { private static final Logger log = Logger.getLogger(StateChangeHandler.class.getName()); + private final FleetControllerContext context; private final Timer timer; private final EventLogInterface eventLog; private boolean stateMayHaveChanged = false; @@ -40,7 +41,8 @@ public class StateChangeHandler { private int maxSlobrokDisconnectGracePeriod = 1000; private static final boolean disableUnstableNodes = true; - public StateChangeHandler(Timer timer, EventLogInterface eventLog) { + public StateChangeHandler(FleetControllerContext context, Timer timer, EventLogInterface eventLog) { + this.context = context; this.timer = timer; this.eventLog = eventLog; maxTransitionTime.put(NodeType.DISTRIBUTOR, 5000); @@ -52,7 +54,7 @@ public class StateChangeHandler { final DatabaseHandler database, final DatabaseHandler.DatabaseContext dbContext) throws InterruptedException { int startTimestampsReset = 0; - log.log(Level.FINE, "handleAllDistributorsInSync invoked for state version %d", currentState.getVersion()); + context.log(log, Level.FINE, "handleAllDistributorsInSync invoked for state version %d", currentState.getVersion()); for (NodeType nodeType : NodeType.getTypes()) { for (ConfiguredNode configuredNode : nodes) { final Node node = new Node(nodeType, configuredNode.index()); diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java index bc8d84c4634..d061f7edbea 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java @@ -18,8 +18,9 @@ import java.util.stream.Collectors; public class SystemStateBroadcaster { - public static Logger log = Logger.getLogger(SystemStateBroadcaster.class.getName()); + private static Logger log = Logger.getLogger(SystemStateBroadcaster.class.getName()); + private final FleetControllerContext context; private final Timer timer; private final Object monitor; private ClusterStateBundle clusterStateBundle; @@ -37,7 +38,8 @@ public class SystemStateBroadcaster { private final SetClusterStateWaiter setClusterStateWaiter = new SetClusterStateWaiter(); private final ActivateClusterStateVersionWaiter activateClusterStateVersionWaiter = new ActivateClusterStateVersionWaiter(); - public SystemStateBroadcaster(Timer timer, Object monitor) { + public SystemStateBroadcaster(FleetControllerContext context, Timer timer, Object monitor) { + this.context = context; this.timer = timer; this.monitor = monitor; } @@ -70,7 +72,7 @@ public class SystemStateBroadcaster { long time = timer.getCurrentTimeInMillis(); Long lastReported = lastErrorReported.get(info.getNode()); boolean alreadySeen = (lastReported != null && time - lastReported < minTimeBetweenNodeErrorLogging); - log.log((nodeOk && !alreadySeen) ? Level.WARNING : Level.FINE, message); + context.log(log, nodeOk && !alreadySeen ? Level.WARNING : Level.FINE, message); if (!alreadySeen) { lastErrorReported.put(info.getNode(), time); } @@ -96,12 +98,17 @@ public class SystemStateBroadcaster { // NO_SUCH_METHOD implies node is on a version that does not understand explicit activations // and it has already merrily started using the state version. Treat as if it had been ACKed. if (reply.getReturnCode() != ErrorCode.NO_SUCH_METHOD) { - log.log(Level.FINE, () -> String.format("Activation NACK for node %s with version %d, message %s", - info, version, reply.getReturnMessage())); + context.log(log, + Level.FINE, + () -> String.format("Activation NACK for node %s with version %d, message %s", + info, version, reply.getReturnMessage())); success = false; } else { - log.log(Level.FINE, () -> String.format("Node %s did not understand state activation RPC; " + - "implicitly treating state %d as activated on node", info, version)); + context.log(log, + Level.FINE, + () -> String.format("Node %s did not understand state activation RPC; " + + "implicitly treating state %d as activated on node", + info, version)); } } else if (reply.getActualVersion() != version) { boolean nodeOk = nodeReportsSelfAsAvailable(info); @@ -113,8 +120,10 @@ public class SystemStateBroadcaster { version, info, reply.getActualVersion())); success = false; } else { - log.log(Level.FINE, () -> String.format("Node %s reports successful activation of state " + - "version %d", info, version)); + context.log(log, + Level.FINE, + () -> String.format("Node %s reports successful activation of state version %d", + info, version)); } info.setSystemStateVersionActivationAcked(version, success); // TODO we currently don't invoke reportNodeError here.. We assume that node errors will be reported @@ -144,7 +153,7 @@ public class SystemStateBroadcaster { } } else { info.setClusterStateBundleVersionAcknowledged(version, true); - log.log(Level.FINE, () -> String.format("Node %s ACKed system state version %d.", info, version)); + context.log(log, Level.FINE, () -> String.format("Node %s ACKed system state version %d.", info, version)); lastErrorReported.remove(info.getNode()); } } @@ -220,8 +229,10 @@ public class SystemStateBroadcaster { if (!anyDistributorsNeedStateBundle && (currentStateVersion > lastStateVersionBundleAcked)) { markCurrentClusterStateBundleAsReceivedByAllDistributors(); if (clusterStateBundle.deferredActivation()) { - log.log(Level.FINE, () -> String.format("All distributors have ACKed cluster state " + - "version %d, sending activation", currentStateVersion)); + context.log(log, + Level.FINE, + () -> String.format("All distributors have ACKed cluster state " + + "version %d, sending activation", currentStateVersion)); } else { markCurrentClusterStateAsConverged(database, dbContext, fleetController); } @@ -239,8 +250,10 @@ public class SystemStateBroadcaster { if (!anyDistributorsNeedActivation && (currentStateVersion > lastClusterStateVersionConverged)) { markCurrentClusterStateAsConverged(database, dbContext, fleetController); } else { - log.log(Level.FINE, () -> String.format("distributors still need activation in state %d (last converged: %d)", - currentStateVersion, lastClusterStateVersionConverged)); + context.log(log, + Level.FINE, + () -> String.format("distributors still need activation in state %d (last converged: %d)", + currentStateVersion, lastClusterStateVersionConverged)); } } @@ -249,7 +262,7 @@ public class SystemStateBroadcaster { } private void markCurrentClusterStateAsConverged(DatabaseHandler database, DatabaseHandler.DatabaseContext dbContext, FleetController fleetController) throws InterruptedException { - log.log(Level.FINE, "All distributors have newest clusterstate, updating start timestamps in zookeeper and clearing them from cluster state"); + context.log(log, Level.FINE, "All distributors have newest clusterstate, updating start timestamps in zookeeper and clearing them from cluster state"); lastClusterStateVersionConverged = clusterStateBundle.getVersion(); lastClusterStateBundleConverged = clusterStateBundle; fleetController.handleAllDistributorsInSync(database, dbContext); @@ -279,7 +292,7 @@ public class SystemStateBroadcaster { ClusterState baselineState = clusterStateBundle.getBaselineClusterState(); if (!currentBundleVersionIsTaggedOfficial()) { - log.log(Level.INFO, String.format("Publishing cluster state version %d", baselineState.getVersion())); + context.log(log, Level.INFO, "Publishing cluster state version " + baselineState.getVersion()); tagCurrentBundleVersionAsOfficial(); } @@ -288,13 +301,17 @@ public class SystemStateBroadcaster { if (nodeNeedsToObserveStartupTimestamps(node)) { // TODO this is the same for all nodes, compute only once ClusterStateBundle modifiedBundle = clusterStateBundle.cloneWithMapper(state -> buildModifiedClusterState(state, dbContext)); - log.log(Level.FINE, () -> String.format("Sending modified cluster state version %d" + - " to node %s: %s", baselineState.getVersion(), node, modifiedBundle)); + context.log(log, + Level.FINE, + () -> "Sending modified cluster state version " + baselineState.getVersion() + + " to node " + node + ": " + modifiedBundle); communicator.setSystemState(modifiedBundle, node, setClusterStateWaiter); } else { - log.log(Level.FINE, () -> String.format("Sending system state version %d to node %s. " + - "(went down time %d, node start time %d)", baselineState.getVersion(), node, - node.getWentDownWithStartTime(), node.getStartTimestamp())); + context.log(log, + Level.FINE, + () -> "Sending system state version " + baselineState.getVersion() + + " to node " + node + ". (went down time " + node.getWentDownWithStartTime() + + ", node start time " + node.getStartTimestamp() + ")"); communicator.setSystemState(clusterStateBundle, node, setClusterStateWaiter); } } @@ -313,8 +330,10 @@ public class SystemStateBroadcaster { var recipients = resolveStateActivationSendSet(dbContext); for (NodeInfo node : recipients) { - log.log(Level.FINE, () -> String.format("Sending cluster state activation to node %s for version %d", - node, clusterStateBundle.getVersion())); + context.log(log, + Level.FINE, + () -> "Sending cluster state activation to node " + node + " for version " + + clusterStateBundle.getVersion()); communicator.activateClusterStateVersion(clusterStateBundle.getVersion(), node, activateClusterStateVersionWaiter); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java index fe716eea288..4285ef83782 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabase.java @@ -7,6 +7,7 @@ import com.yahoo.vdslib.state.State; import com.yahoo.vespa.clustercontroller.core.AnnotatedClusterState; 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.rpc.EnvelopedClusterStateBundleCodec; import com.yahoo.vespa.clustercontroller.core.rpc.SlimeClusterStateBundleCodec; import com.yahoo.vespa.zookeeper.client.ZkClientConfigBuilder; @@ -42,18 +43,14 @@ public class ZooKeeperDatabase extends Database { private final ZooKeeperWatcher watcher = new ZooKeeperWatcher(); private final ZooKeeper session; private boolean sessionOpen = true; + private final FleetControllerContext context; private final int nodeIndex; private final MasterDataGatherer masterDataGatherer; - private boolean reportErrors = true; // Expected ZK znode versions. Note: these are _not_ -1 as that would match anything. // We expect the caller to invoke the load methods prior to calling any store methods. private int lastKnownStateBundleZNodeVersion = -2; private int lastKnownStateVersionZNodeVersion = -2; - public void stopErrorReporting() { - reportErrors = false; - } - private class ZooKeeperWatcher implements Watcher { private Event.KeeperState state = null; @@ -62,50 +59,51 @@ public class ZooKeeperDatabase extends Database { public void process(WatchedEvent watchedEvent) { // Shouldn't get events after we expire, but just be sure we stop them here. if (state != null && state.equals(Event.KeeperState.Expired)) { - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got event from ZooKeeper session after it expired"); + context.log(log, Level.WARNING, "Got event from ZooKeeper session after it expired"); return; } Event.KeeperState newState = watchedEvent.getState(); if (state == null || !state.equals(newState)) switch (newState) { case Expired: - log.log(Level.INFO, "Fleetcontroller " + nodeIndex + ": Zookeeper session expired"); + context.log(log, Level.INFO, "Zookeeper session expired"); sessionOpen = false; listener.handleZooKeeperSessionDown(); break; case Disconnected: - log.log(Level.INFO, "Fleetcontroller " + nodeIndex + ": Lost connection to zookeeper server"); + context.log(log, Level.INFO, "Lost connection to zookeeper server"); sessionOpen = false; listener.handleZooKeeperSessionDown(); break; case SyncConnected: - log.log(Level.INFO, "Fleetcontroller " + nodeIndex + ": Connection to zookeeper server established. Refetching master data"); + context.log(log, Level.INFO, "Connection to zookeeper server established. Refetching master data"); if (masterDataGatherer != null) { masterDataGatherer.restart(); } } switch (watchedEvent.getType()) { case NodeChildrenChanged: // Fleetcontrollers have either connected or disconnected to ZooKeeper - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got unexpected ZooKeeper event NodeChildrenChanged"); + context.log(log, Level.WARNING, "Got unexpected ZooKeeper event NodeChildrenChanged"); break; case NodeDataChanged: // A fleetcontroller have changed what node it is voting for - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got unexpected ZooKeeper event NodeDataChanged"); + context.log(log, Level.WARNING, "Got unexpected ZooKeeper event NodeDataChanged"); break; case NodeCreated: // How can this happen? Can one leave watches on non-existing nodes? - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got unexpected ZooKeeper event NodeCreated"); + context.log(log, Level.WARNING, "Got unexpected ZooKeeper event NodeCreated"); break; case NodeDeleted: // We're not watching any nodes for whether they are deleted or not. - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got unexpected ZooKeeper event NodeDeleted"); + context.log(log, Level.WARNING, "Got unexpected ZooKeeper event NodeDeleted"); break; case None: if (state != null && state.equals(watchedEvent.getState())) { - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got None type event that didn't even alter session state. What does that indicate?"); + context.log(log, Level.WARNING, "Got None type event that didn't even alter session state. What does that indicate?"); } } state = watchedEvent.getState(); } } - public ZooKeeperDatabase(ContentCluster cluster, int nodeIndex, String address, int timeout, Database.DatabaseListener zksl) throws IOException, KeeperException, InterruptedException { + public ZooKeeperDatabase(FleetControllerContext context, ContentCluster cluster, int nodeIndex, String address, int timeout, DatabaseListener zksl) throws IOException, KeeperException, InterruptedException { + this.context = context; this.nodeIndex = nodeIndex; zooKeeperRoot = "/vespa/fleetcontroller/" + cluster.getName() + "/"; session = new ZooKeeper(address, timeout, watcher, new ZkClientConfigBuilder().toConfig()); @@ -113,7 +111,7 @@ public class ZooKeeperDatabase extends Database { try{ this.listener = zksl; setupRoot(); - log.log(Level.FINEST, () -> "Fleetcontroller " + nodeIndex + ": Asking for initial data on master election"); + context.log(log, Level.FINEST, "Asking for initial data on master election"); masterDataGatherer = new MasterDataGatherer(session, zooKeeperRoot, listener, nodeIndex); completedOk = true; } finally { @@ -124,14 +122,14 @@ public class ZooKeeperDatabase extends Database { private void createNode(String prefix, String nodename, byte[] value) throws KeeperException, InterruptedException { try{ if (session.exists(prefix + nodename, false) != null) { - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Zookeeper node '" + prefix + nodename + "' already exists. Not creating it"); + context.log(log, Level.FINE, () -> "Zookeeper node '" + prefix + nodename + "' already exists. Not creating it"); return; } session.create(prefix + nodename, value, acl, CreateMode.PERSISTENT); - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Created zookeeper node '" + prefix + nodename + "'"); + context.log(log, Level.FINE, () -> "Created zookeeper node '" + prefix + nodename + "'"); } catch (KeeperException.NodeExistsException e) { - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Node to create existed, " - + "but this is normal as other nodes may create them at the same time."); + context.log(log, Level.FINE, "Node to create existed, but this is normal as other nodes " + + "may create them at the same time."); } } @@ -149,14 +147,13 @@ public class ZooKeeperDatabase extends Database { createNode(zooKeeperRoot, "published_state_bundle", new byte[0]); // TODO dedupe string constants byte[] val = String.valueOf(nodeIndex).getBytes(utf8); deleteNodeIfExists(getMyIndexPath()); - log.log(Level.INFO, "Fleetcontroller " + nodeIndex + - ": Creating ephemeral master vote node with vote to self."); + context.log(log, Level.INFO, "Creating ephemeral master vote node with vote to self."); session.create(getMyIndexPath(), val, acl, CreateMode.EPHEMERAL); } private void deleteNodeIfExists(String path) throws KeeperException, InterruptedException { if (session.exists(path, false) != null) { - log.log(Level.INFO, "Fleetcontroller " + nodeIndex + ": Removing master vote node."); + context.log(log, Level.INFO, "Removing master vote node at " + path); session.delete(path, -1); } } @@ -172,11 +169,11 @@ public class ZooKeeperDatabase extends Database { public void close() { sessionOpen = false; try{ - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Trying to close ZooKeeper session 0x" + context.log(log, Level.FINE, () -> "Trying to close ZooKeeper session 0x" + Long.toHexString(session.getSessionId())); session.close(); } catch (InterruptedException e) { - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Got interrupt exception while closing session: " + e); + context.log(log, Level.WARNING, "Got interrupt exception while closing session: " + e); } } @@ -185,11 +182,10 @@ public class ZooKeeperDatabase extends Database { } private void maybeLogExceptionWarning(Exception e, String message) { - if (sessionOpen && reportErrors) { + if (sessionOpen) { StringWriter sw = new StringWriter(); e.printStackTrace(new PrintWriter(sw)); - log.log(Level.WARNING, String.format("Fleetcontroller %s: %s. Exception: %s\n%s", - nodeIndex, message, e.getMessage(), sw.toString())); + context.log(log, Level.WARNING, message + ". Exception: " + e.getMessage() + "\n" + sw); } } @@ -197,7 +193,7 @@ public class ZooKeeperDatabase extends Database { byte[] val = String.valueOf(wantedMasterIndex).getBytes(utf8); try{ session.setData(getMyIndexPath(), val, -1); - log.log(Level.INFO, "Fleetcontroller " + nodeIndex + ": Stored new vote in ephemeral node. " + nodeIndex + " -> " + wantedMasterIndex); + context.log(log, Level.INFO, "Stored new vote in ephemeral node. " + nodeIndex + " -> " + wantedMasterIndex); return true; } catch (InterruptedException e) { throw new RuntimeException(e); @@ -209,7 +205,7 @@ public class ZooKeeperDatabase extends Database { public boolean storeLatestSystemStateVersion(int version) { byte[] data = Integer.toString(version).getBytes(utf8); try{ - log.log(Level.INFO, String.format("Fleetcontroller %d: Storing new cluster state version in ZooKeeper: %d", nodeIndex, version)); + context.log(log, Level.INFO, "Storing new cluster state version in ZooKeeper: " + version); var stat = session.setData(zooKeeperRoot + "latestversion", data, lastKnownStateVersionZNodeVersion); lastKnownStateVersionZNodeVersion = stat.getVersion(); return true; @@ -227,13 +223,11 @@ public class ZooKeeperDatabase extends Database { public Integer retrieveLatestSystemStateVersion() { Stat stat = new Stat(); try{ - log.log(Level.FINE, () -> String.format("Fleetcontroller %d: Fetching latest cluster state at '%slatestversion'", - nodeIndex, zooKeeperRoot)); + context.log(log, Level.FINE, "Fetching latest cluster state at '%slatestversion'", zooKeeperRoot); byte[] data = session.getData(zooKeeperRoot + "latestversion", false, stat); lastKnownStateVersionZNodeVersion = stat.getVersion(); final Integer versionNumber = Integer.valueOf(new String(data, utf8)); - log.log(Level.INFO, String.format("Fleetcontroller %d: Read cluster state version %d from ZooKeeper " + - "(znode version %d)", nodeIndex, versionNumber, stat.getVersion())); + context.log(log, Level.INFO, "Read cluster state version %d from ZooKeeper (znode version %d)", versionNumber, stat.getVersion()); return versionNumber; } catch (InterruptedException e) { throw new RuntimeException(e); @@ -262,7 +256,7 @@ public class ZooKeeperDatabase extends Database { } byte[] val = sb.toString().getBytes(utf8); try{ - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Storing wanted states at '" + zooKeeperRoot + "wantedstates'"); + context.log(log, Level.FINE, () -> "Storing wanted states at '" + zooKeeperRoot + "wantedstates'"); session.setData(zooKeeperRoot + "wantedstates", val, -1); return true; } catch (InterruptedException e) { @@ -275,7 +269,7 @@ public class ZooKeeperDatabase extends Database { public Map<Node, NodeState> retrieveWantedStates() { try{ - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Fetching wanted states at '" + zooKeeperRoot + "wantedstates'"); + context.log(log, Level.FINE, () -> "Fetching wanted states at '" + zooKeeperRoot + "wantedstates'"); Stat stat = new Stat(); byte[] data = session.getData(zooKeeperRoot + "wantedstates", false, stat); Map<Node, NodeState> wanted = new TreeMap<>(); @@ -290,7 +284,7 @@ public class ZooKeeperDatabase extends Database { NodeState nodeState = NodeState.deserialize(node.getType(), token.substring(colon + 1)); wanted.put(node, nodeState); } catch (Exception e) { - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Ignoring invalid wantedstate line in zookeeper '" + token + "'."); + context.log(log, Level.WARNING, "Ignoring invalid wantedstate line in zookeeper '" + token + "'."); } } } @@ -313,7 +307,7 @@ public class ZooKeeperDatabase extends Database { } byte val[] = sb.toString().getBytes(utf8); try{ - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Storing start timestamps at '" + zooKeeperRoot + "starttimestamps"); + context.log(log, Level.FINE, () -> "Storing start timestamps at '" + zooKeeperRoot + "starttimestamps"); session.setData(zooKeeperRoot + "starttimestamps", val, -1); return true; } catch (InterruptedException e) { @@ -327,7 +321,7 @@ public class ZooKeeperDatabase extends Database { @Override public Map<Node, Long> retrieveStartTimestamps() { try{ - log.log(Level.FINE, () -> "Fleetcontroller " + nodeIndex + ": Fetching start timestamps at '" + zooKeeperRoot + "starttimestamps'"); + context.log(log, Level.FINE, () -> "Fetching start timestamps at '" + zooKeeperRoot + "starttimestamps'"); Stat stat = new Stat(); byte[] data = session.getData(zooKeeperRoot + "starttimestamps", false, stat); Map<Node, Long> wanted = new TreeMap<Node, Long>(); @@ -342,7 +336,7 @@ public class ZooKeeperDatabase extends Database { Long timestamp = Long.valueOf(token.substring(colon + 1)); wanted.put(n, timestamp); } catch (Exception e) { - log.log(Level.WARNING, "Fleetcontroller " + nodeIndex + ": Ignoring invalid starttimestamp line in zookeeper '" + token + "'."); + context.log(log, Level.WARNING, "Ignoring invalid starttimestamp line in zookeeper '" + token + "'."); } } } @@ -360,9 +354,11 @@ public class ZooKeeperDatabase extends Database { EnvelopedClusterStateBundleCodec envelopedBundleCodec = new SlimeClusterStateBundleCodec(); byte[] encodedBundle = envelopedBundleCodec.encodeWithEnvelope(stateBundle); try{ - log.log(Level.FINE, () -> String.format("Fleetcontroller %d: Storing published state bundle %s at " + - "'%spublished_state_bundle' with expected znode version %d", - nodeIndex, stateBundle, zooKeeperRoot, lastKnownStateBundleZNodeVersion)); + context.log(log, + Level.FINE, + () -> String.format("Storing published state bundle %s at " + + "'%spublished_state_bundle' with expected znode version %d", + stateBundle, zooKeeperRoot, lastKnownStateBundleZNodeVersion)); var stat = session.setData(zooKeeperRoot + "published_state_bundle", encodedBundle, lastKnownStateBundleZNodeVersion); lastKnownStateBundleZNodeVersion = stat.getVersion(); } catch (InterruptedException e) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabaseFactory.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabaseFactory.java index 0f739eec1d0..71f39135609 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabaseFactory.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/database/ZooKeeperDatabaseFactory.java @@ -1,12 +1,20 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.core.database; +import com.yahoo.vespa.clustercontroller.core.FleetControllerContext; + public class ZooKeeperDatabaseFactory implements DatabaseFactory { + private final FleetControllerContext context; + + public ZooKeeperDatabaseFactory(FleetControllerContext context) { + this.context = context; + } + @Override public Database create(Params params) throws Exception { - return new ZooKeeperDatabase(params.cluster, params.nodeIndex, params.dbAddress, - params.dbSessionTimeout, params.listener); + return new ZooKeeperDatabase(context, params.cluster, params.nodeIndex, params.dbAddress, + params.dbSessionTimeout, params.listener); } } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java index c4894e41747..7487f9546b7 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/rpc/SlobrokClient.java @@ -2,17 +2,17 @@ package com.yahoo.vespa.clustercontroller.core.rpc; -import com.yahoo.jrt.slobrok.api.SlobrokList; -import com.yahoo.jrt.slobrok.api.Mirror; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Transport; -import com.yahoo.vdslib.state.NodeType; +import com.yahoo.jrt.slobrok.api.Mirror; +import com.yahoo.jrt.slobrok.api.SlobrokList; import com.yahoo.vdslib.state.Node; -import java.util.logging.Level; +import com.yahoo.vdslib.state.NodeType; +import com.yahoo.vespa.clustercontroller.core.ContentCluster; +import com.yahoo.vespa.clustercontroller.core.FleetControllerContext; import com.yahoo.vespa.clustercontroller.core.NodeInfo; import com.yahoo.vespa.clustercontroller.core.NodeLookup; import com.yahoo.vespa.clustercontroller.core.Timer; -import com.yahoo.vespa.clustercontroller.core.ContentCluster; import com.yahoo.vespa.clustercontroller.core.listeners.NodeAddedOrRemovedListener; import java.util.Iterator; @@ -21,19 +21,22 @@ import java.util.List; import java.util.Map; import java.util.StringTokenizer; import java.util.TreeMap; +import java.util.logging.Level; import java.util.logging.Logger; public class SlobrokClient implements NodeLookup { public static final Logger log = Logger.getLogger(SlobrokClient.class.getName()); + private final FleetControllerContext context; private final Timer timer; private String[] connectionSpecs; private Mirror mirror; private Supervisor supervisor; private boolean freshMirror = false; - public SlobrokClient(Timer timer) { + public SlobrokClient(FleetControllerContext context, Timer timer) { + this.context = context; this.timer = timer; } @@ -81,9 +84,7 @@ public class SlobrokClient implements NodeLookup { if (freshMirror) { freshMirror = false; } else if (cluster.getSlobrokGenerationCount() == mirrorVersion) { - if (log.isLoggable(Level.FINEST)) { - log.log(Level.FINEST, "Slobrok still at generation count " + cluster.getSlobrokGenerationCount() + ". Not updating."); - } + context.log(log, Level.FINEST, () -> "Slobrok still at generation count " + cluster.getSlobrokGenerationCount() + ". Not updating."); return false; } @@ -150,16 +151,18 @@ public class SlobrokClient implements NodeLookup { cluster.setSlobrokGenerationCount(mirrorVersion); for (NodeInfo nodeInfo : cluster.getNodeInfo()) { if (slobrokNodes.containsKey(nodeInfo.getNode()) && nodeInfo.isRpcAddressOutdated()) { - log.log(Level.WARNING, "Node " + nodeInfo - + " was tagged NOT in slobrok even though it is. It was in the following lists:" - + (newNodes.contains(nodeInfo.getNode()) ? " newNodes" : "") - + (missingNodeInfos.contains(nodeInfo) ? " missingNodes" : "") - + (alteredRpcAddressNodes.contains(nodeInfo.getNode()) ? " alteredNodes" : "") - + (returningNodeInfos.contains(nodeInfo) ? " returningNodes" : "")); + context.log(log, + Level.WARNING, + "Node " + nodeInfo + + " was tagged NOT in slobrok even though it is. It was in the following lists:" + + (newNodes.contains(nodeInfo.getNode()) ? " newNodes" : "") + + (missingNodeInfos.contains(nodeInfo) ? " missingNodes" : "") + + (alteredRpcAddressNodes.contains(nodeInfo.getNode()) ? " alteredNodes" : "") + + (returningNodeInfos.contains(nodeInfo) ? " returningNodes" : "")); nodeInfo.markRpcAddressLive(); } } - log.log(Level.FINEST, "Slobrok information updated to generation " + cluster.getSlobrokGenerationCount()); + context.log(log, Level.FINEST, () -> "Slobrok information updated to generation " + cluster.getSlobrokGenerationCount()); return true; } @@ -204,7 +207,7 @@ public class SlobrokClient implements NodeLookup { private Map<Node, SlobrokData> getSlobrokData(String pattern) { Map<Node, SlobrokData> result = new TreeMap<>(); List<Mirror.Entry> entries = mirror.lookup(pattern); - log.log(Level.FINEST, "Looking for slobrok entries with pattern '" + pattern + "'. Found " + entries.size() + " entries."); + context.log(log, Level.FINEST, () -> "Looking for slobrok entries with pattern '" + pattern + "'. Found " + entries.size() + " entries."); for (Mirror.Entry entry : entries) { StringTokenizer st = new StringTokenizer(entry.getName(), "/"); String addressType = st.nextToken(); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java index a52370a0654..f0b91102e8f 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFeedBlockTest.java @@ -54,9 +54,9 @@ public class ClusterFeedBlockTest extends FleetControllerTest { var eventLog = new EventLog(timer, metricUpdater); var cluster = new ContentCluster(options.clusterName, options.nodes, options.storageDistribution); var stateGatherer = new NodeStateGatherer(timer, timer, eventLog); - var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(), timer, options.zooKeeperServerAddress, timer); - var stateGenerator = new StateChangeHandler(timer, eventLog); - var stateBroadcaster = new SystemStateBroadcaster(timer, timer); + var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(context), timer, options.zooKeeperServerAddress, timer); + var stateGenerator = new StateChangeHandler(context, timer, eventLog); + var stateBroadcaster = new SystemStateBroadcaster(context, timer, timer); var masterElectionHandler = new MasterElectionHandler(context, options.fleetControllerIndex, options.fleetControllerCount, timer, timer); ctrl = new FleetController(context, timer, eventLog, cluster, stateGatherer, communicator, null, null, communicator, database, stateGenerator, stateBroadcaster, masterElectionHandler, metricUpdater, options); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java index 75c31898408..4ce32484098 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java @@ -33,14 +33,11 @@ public class ClusterFixture { this.distribution = distribution; this.timer = new FakeTimer(); this.eventLog = mock(EventLogInterface.class); - this.nodeStateChangeHandler = createNodeStateChangeHandlerForCluster(); + var context = new FleetControllerContextImpl(new FleetControllerId(cluster.getName(), 0)); + this.nodeStateChangeHandler = new StateChangeHandler(context, timer, eventLog); this.params.cluster(this.cluster); } - private StateChangeHandler createNodeStateChangeHandlerForCluster() { - return new StateChangeHandler(timer, eventLog); - } - public ClusterFixture bringEntireClusterUp() { cluster.clusterInfo().getConfiguredNodes().forEach((idx, node) -> { reportStorageNodeState(idx, State.UP); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContextImplTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContextImplTest.java new file mode 100644 index 00000000000..450975076bb --- /dev/null +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerContextImplTest.java @@ -0,0 +1,43 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.clustercontroller.core; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import static org.junit.Assert.assertEquals; + +/** + * @author hakonhall + */ +public class FleetControllerContextImplTest { + private final MockLogger logger = new MockLogger(); + public final FleetControllerId id = new FleetControllerId("clustername", 1); + private final FleetControllerContextImpl context = new FleetControllerContextImpl(id); + + @Test + public void verify() { + context.log(logger, Level.INFO, "A %s message", "log"); + + assertEquals(1, logger.records.size()); + assertEquals(Level.INFO, logger.records.get(0).getLevel()); + assertEquals("Cluster 'clustername': A log message", logger.records.get(0).getMessage()); + } + + private static class MockLogger extends Logger { + public List<LogRecord> records = new ArrayList<>(); + + public MockLogger() { + super(MockLogger.class.getName(), null); + } + + @Override + public void log(LogRecord record) { + records.add(record); + } + } +} 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 d115f9f0060..c56b3bbdc69 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 @@ -174,18 +174,18 @@ public abstract class FleetControllerTest implements Waiter { options.nodeStateRequestTimeoutEarliestPercentage, options.nodeStateRequestTimeoutLatestPercentage, options.nodeStateRequestRoundTripTimeMaxSeconds); - var lookUp = new SlobrokClient(timer); + var lookUp = new SlobrokClient(context, timer); lookUp.setSlobrokConnectionSpecs(new String[0]); var rpcServer = new RpcServer(timer, timer, options.clusterName, options.fleetControllerIndex, options.slobrokBackOffPolicy); - var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(), timer, options.zooKeeperServerAddress, timer); + var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(context), timer, options.zooKeeperServerAddress, timer); // Setting this <1000 ms causes ECONNREFUSED on socket trying to connect to ZK server, in ZooKeeper, // after creating a new ZooKeeper (session). This causes ~10s extra time to connect after connection loss. // Reasons unknown. Larger values like the default 10_000 causes that much additional running time for some tests. database.setMinimumWaitBetweenFailedConnectionAttempts(2_000); - var stateGenerator = new StateChangeHandler(timer, log); - var stateBroadcaster = new SystemStateBroadcaster(timer, timer); + var stateGenerator = new StateChangeHandler(context, timer, log); + var stateBroadcaster = new SystemStateBroadcaster(context, timer, timer); var masterElectionHandler = new MasterElectionHandler(context, options.fleetControllerIndex, options.fleetControllerCount, timer, timer); var controller = new FleetController(context, timer, log, cluster, stateGatherer, communicator, status, rpcServer, lookUp, database, stateGenerator, stateBroadcaster, masterElectionHandler, metricUpdater, options); if (startThread) { diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java index b370c29537d..95c097c5920 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java @@ -81,7 +81,8 @@ public class StateChangeHandlerTest { this.config = config; for (int i=0; i<config.nodeCount; ++i) configuredNodes.add(new ConfiguredNode(i, false)); cluster = new ContentCluster("testcluster", configuredNodes, distribution); - nodeStateChangeHandler = new StateChangeHandler(clock, eventLog); + var context = new FleetControllerContextImpl(new FleetControllerId(cluster.getName(), 0)); + nodeStateChangeHandler = new StateChangeHandler(context, clock, eventLog); params.minStorageNodesUp(1).minDistributorNodesUp(1) .minRatioOfStorageNodesUp(0.0).minRatioOfDistributorNodesUp(0.0) .maxPrematureCrashes(config.maxPrematureCrashes) 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 b601412ecc4..a5bb65e11d0 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 @@ -55,9 +55,9 @@ public class StateChangeTest extends FleetControllerTest { eventLog = new EventLog(timer, metricUpdater); var cluster = new ContentCluster(options.clusterName, options.nodes, options.storageDistribution); var stateGatherer = new NodeStateGatherer(timer, timer, eventLog); - var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(), timer, options.zooKeeperServerAddress, timer); - var stateGenerator = new StateChangeHandler(timer, eventLog); - var stateBroadcaster = new SystemStateBroadcaster(timer, timer); + var database = new DatabaseHandler(context, new ZooKeeperDatabaseFactory(context), timer, options.zooKeeperServerAddress, timer); + var stateGenerator = new StateChangeHandler(context, timer, eventLog); + var stateBroadcaster = new SystemStateBroadcaster(context, timer, timer); var masterElectionHandler = new MasterElectionHandler(context, options.fleetControllerIndex, options.fleetControllerCount, timer, timer); ctrl = new FleetController(context, timer, eventLog, cluster, stateGatherer, communicator, null, null, communicator, database, stateGenerator, stateBroadcaster, masterElectionHandler, metricUpdater, options); 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 84b479cfc29..45593375c0b 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 @@ -26,7 +26,8 @@ public class SystemStateBroadcasterTest { private static class Fixture { FakeTimer timer = new FakeTimer(); final Object monitor = new Object(); - SystemStateBroadcaster broadcaster = new SystemStateBroadcaster(timer, monitor); + FleetControllerContext context = mock(FleetControllerContext.class); + SystemStateBroadcaster broadcaster = new SystemStateBroadcaster(context, timer, monitor); Communicator mockCommunicator = mock(Communicator.class); DatabaseHandler mockDatabaseHandler = mock(DatabaseHandler.class); FleetController mockFleetController = mock(FleetController.class); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/TestFleetControllerContext.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/TestFleetControllerContext.java index b9d8474affb..6fe8f92ac97 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/TestFleetControllerContext.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/TestFleetControllerContext.java @@ -9,6 +9,10 @@ public class TestFleetControllerContext extends FleetControllerContextImpl { super(options); } + public TestFleetControllerContext(FleetControllerId id) { + super(id); + } + @Override protected String withLogPrefix(String message) { // Include fleet controller index in prefix in tests, since many may be running diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java index d2407541680..a71665fb364 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ZooKeeperDatabaseTest.java @@ -35,8 +35,10 @@ public class ZooKeeperDatabaseTest { void createDatabase() throws Exception { closeDatabaseIfOpen(); - zkDatabase = new ZooKeeperDatabase(clusterFixture.cluster(), nodeIndex, zkServer.getAddress(), - (int)sessionTimeout.toMillis(), mockListener); + var id = new FleetControllerId(clusterFixture.cluster.getName(), nodeIndex); + var context = new TestFleetControllerContext(id); + zkDatabase = new ZooKeeperDatabase(context, clusterFixture.cluster(), nodeIndex, zkServer.getAddress(), + (int)sessionTimeout.toMillis(), mockListener); } ZooKeeperDatabase db() { return zkDatabase; } diff --git a/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java b/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java index 19805adc637..27286a7dbbe 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java @@ -78,9 +78,11 @@ public class ApplicationConfigProducerRoot extends AbstractConfigProducer<Abstra } private boolean useV8GeoPositions = false; + private boolean useV8DocManagerCfg = false; public void useFeatureFlags(ModelContext.FeatureFlags featureFlags) { this.useV8GeoPositions = featureFlags.useV8GeoPositions(); + this.useV8DocManagerCfg = featureFlags.useV8DocManagerCfg(); } /** @@ -160,6 +162,7 @@ public class ApplicationConfigProducerRoot extends AbstractConfigProducer<Abstra public void getConfig(DocumentmanagerConfig.Builder builder) { new DocumentManager() .useV8GeoPositions(this.useV8GeoPositions) + .useV8DocManagerCfg(this.useV8DocManagerCfg) .produce(documentModel, builder); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java index 170753a6ff1..55f24123940 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java @@ -195,6 +195,11 @@ public class DocumentModelBuilder { } } + private static String descT(DataType type) { + if (type == null) { return "<null>"; } + return "'" + type.getName() + "' [" + type.getId() + "] {"+type.getClass() + "}"; + } + private void addDocumentTypes(List<SDDocumentType> docList) { LinkedList<NewDocumentType> lst = new LinkedList<>(); for (SDDocumentType doc : docList) { @@ -235,13 +240,11 @@ public class DocumentModelBuilder { if (other == null || other == type) { other = getDocumentType(docs, type.getId()); } - // maybe warning if null here? if (other != null) { type = other; } } else if (type instanceof DocumentType || type instanceof NewDocumentType) { DataType other = getDocumentType(docs, type.getId()); - // maybe warning if null here? if (other != null) { type = other; } @@ -387,6 +390,13 @@ public class DocumentModelBuilder { throw new IllegalArgumentException("Data type '" + sdoc.getName() + "' is not a struct => tostring='" + sdoc.toString() + "'."); } } + for (SDDocumentType type : sdoc.getTypes()) { + for (SDDocumentType proxy : type.getInheritedTypes()) { + var inherited = dt.getDataTypeRecursive(proxy.getName()); + var converted = (StructDataType) dt.getDataType(type.getName()); + converted.inherit((StructDataType) inherited); + } + } for (AnnotationType annotation : sdoc.getAnnotations().values()) { dt.add(annotation); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java index 2ff4d2d44d0..098426865fb 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java @@ -218,6 +218,8 @@ public class SchemaBuilder { public void build(boolean validate) { if (isBuilt) throw new IllegalStateException("Application already built"); + new TemporarySDTypeResolver(application.schemas().values(), deployLogger).process(); + if (validate) application.validate(deployLogger); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/TemporarySDTypeResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/TemporarySDTypeResolver.java new file mode 100644 index 00000000000..2eaf0d5e5ba --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/TemporarySDTypeResolver.java @@ -0,0 +1,79 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition; + +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.searchdefinition.document.SDDocumentType; +import com.yahoo.searchdefinition.document.TemporarySDDocumentType; + +import java.util.Collection; +import java.util.LinkedList; +import java.util.List; +import java.util.logging.Level; + +/** + * @author arnej + */ +public class TemporarySDTypeResolver { + + private final DeployLogger deployLogger; + private final Collection<Schema> toProcess; + private final List<SDDocumentType> docTypes = new LinkedList<>(); + + public TemporarySDTypeResolver(Collection<Schema> schemas, DeployLogger deployLogger) { + this.deployLogger = deployLogger; + this.toProcess = schemas; + } + + private SDDocumentType findDocType(String name) { + assert(name != null); + for (var doc : docTypes) { + if (doc.getName().equals(name)) { + return doc; + } + } + deployLogger.logApplicationPackage(Level.WARNING, "No document type in application matching name: "+name); + return null; + } + + public void process() { + docTypes.add(SDDocumentType.VESPA_DOCUMENT); + for (Schema schema : toProcess) { + if (schema.hasDocument()) { + docTypes.add(schema.getDocument()); + } + } + // first, fix inheritance + for (SDDocumentType doc : docTypes) { + for (SDDocumentType inherited : doc.getInheritedTypes()) { + if (inherited instanceof TemporarySDDocumentType) { + var actual = findDocType(inherited.getName()); + if (actual != null) { + doc.inherit(actual); + } else { + deployLogger.logApplicationPackage(Level.WARNING, "Unresolved inherit '"+inherited.getName() +"' for document "+doc.getName()); + } + } + } + } + // next, check owned types (structs only?) + for (SDDocumentType doc : docTypes) { + for (SDDocumentType owned : doc.getTypes()) { + if (owned instanceof TemporarySDDocumentType) { + deployLogger.logApplicationPackage(Level.WARNING, "Schema '"+doc.getName()+"' owned type '"+owned.getName()+"' is temporary, should not happen"); + continue; + } + for (SDDocumentType inherited : owned.getInheritedTypes()) { + if (inherited instanceof TemporarySDDocumentType) { + var actual = doc.getType(inherited.getName()); + if (actual != null) { + owned.inherit(actual); + } else { + deployLogger.logApplicationPackage(Level.WARNING, "Unresolved inherit '"+inherited.getName() +"' for type '"+owned.getName()+"' in document "+doc.getName()); + } + } + } + } + } + } + +} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java index 51dc9834f20..14e303522e0 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java @@ -38,6 +38,12 @@ public class Deriver { return new DocumentManager().produce(getSearchBuilder(sds).getModel(), new DocumentmanagerConfig.Builder()); } + public static DocumentmanagerConfig.Builder getDocumentManagerConfig(List<String> sds, boolean useV8DocManagerCfg) { + return new DocumentManager() + .useV8DocManagerCfg(useV8DocManagerCfg) + .produce(getSearchBuilder(sds).getModel(), new DocumentmanagerConfig.Builder()); + } + public static DocumenttypesConfig.Builder getDocumentTypesConfig(String sd) { return getDocumentTypesConfig(Collections.singletonList(sd)); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java index a484476e978..1d4b39dfcc5 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java @@ -62,6 +62,7 @@ public class Processing { MultifieldIndexHarmonizer::new, FilterFieldNames::new, MatchConsistency::new, + ValidateStructTypeInheritance::new, ValidateFieldTypes::new, SummaryDiskAccessValidator::new, DisallowComplexMapAndWsetKeyTypes::new, diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ValidateStructTypeInheritance.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ValidateStructTypeInheritance.java new file mode 100644 index 00000000000..d99832e3df6 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ValidateStructTypeInheritance.java @@ -0,0 +1,76 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition.processing; + +import com.yahoo.searchdefinition.Schema; +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.searchdefinition.RankProfileRegistry; +import com.yahoo.vespa.model.container.search.QueryProfiles; + +import com.yahoo.document.DataType; +import com.yahoo.document.Field; +import com.yahoo.document.StructDataType; +import com.yahoo.searchdefinition.document.SDDocumentType; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; +import java.util.HashSet; +import java.util.Set; + +/** + * @author arnej + */ +public class ValidateStructTypeInheritance extends Processor { + + public ValidateStructTypeInheritance(Schema schema, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) { + super(schema, deployLogger, rankProfileRegistry, queryProfiles); + } + + @Override + public void process(boolean validate, boolean documentsOnly) { + if (!validate) return; + verifyNoRedeclarations(schema.getDocument()); + } + + void fail(Field field, String message) { + throw newProcessException(schema, field, message); + } + + void verifyNoRedeclarations(SDDocumentType docType) { + for (SDDocumentType type : docType.allTypes().values()) { + if (type.isStruct()) { + var inheritedTypes = new ArrayList<SDDocumentType>(type.getInheritedTypes()); + for (int i = 0; i < inheritedTypes.size(); i++) { + SDDocumentType inherit = inheritedTypes.get(i); + for (var extra : inherit.getInheritedTypes()) { + if (! inheritedTypes.contains(extra)) { + inheritedTypes.add(extra); + } + } + } + if (inheritedTypes.isEmpty()) continue; + var seenFieldNames = new HashSet<>(); + for (var field : type.getDocumentType().contentStruct().getFieldsThisTypeOnly()) { + if (seenFieldNames.contains(field.getName())) { + // cannot happen? + fail(field, "struct "+type.getName()+" has multiple fields with same name: "+field.getName()); + } + seenFieldNames.add(field.getName()); + } + for (SDDocumentType inherit : inheritedTypes) { + if (inherit.isStruct()) { + for (var field : inherit.getDocumentType().contentStruct().getFieldsThisTypeOnly()) { + if (seenFieldNames.contains(field.getName())) { + fail(field, "struct "+type.getName()+" cannot inherit from "+inherit.getName()+" and redeclare field "+field.getName()); + } + seenFieldNames.add(field.getName()); + } + } else { + fail(new Field("no field"), "struct cannot inherit from non-struct "+inherit.getName()+" class "+inherit.getClass()); + } + } + } + } + } + +} diff --git a/config-model/src/main/java/com/yahoo/vespa/configmodel/producers/DocumentManager.java b/config-model/src/main/java/com/yahoo/vespa/configmodel/producers/DocumentManager.java index 4cfd5c84550..9b4b3eba3a7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/configmodel/producers/DocumentManager.java +++ b/config-model/src/main/java/com/yahoo/vespa/configmodel/producers/DocumentManager.java @@ -14,24 +14,44 @@ import com.yahoo.vespa.documentmodel.DocumentModel; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import java.util.Set; /** - * @author baldersheim + * @author baldersheim + * @author arnej */ public class DocumentManager { private boolean useV8GeoPositions = false; + private boolean useV8DocManagerCfg = false; public DocumentManager useV8GeoPositions(boolean value) { this.useV8GeoPositions = value; return this; } + public DocumentManager useV8DocManagerCfg(boolean value) { + this.useV8DocManagerCfg = value; + return this; + } public DocumentmanagerConfig.Builder produce(DocumentModel model, - DocumentmanagerConfig.Builder documentConfigBuilder) { + DocumentmanagerConfig.Builder documentConfigBuilder) + { + if (useV8DocManagerCfg) { + return produceDocTypes(model, documentConfigBuilder); + } else { + return produceDataTypes(model, documentConfigBuilder); + } + } + + public DocumentmanagerConfig.Builder produceDataTypes(DocumentModel model, + DocumentmanagerConfig.Builder documentConfigBuilder) + { documentConfigBuilder.enablecompression(false); documentConfigBuilder.usev8geopositions(this.useV8GeoPositions); Set<DataType> handled = new HashSet<>(); @@ -99,14 +119,14 @@ public class DocumentManager { } else if (type instanceof WeightedSetDataType) { WeightedSetDataType dt = (WeightedSetDataType) type; builder.weightedsettype(new Datatype.Weightedsettype.Builder(). - datatype(dt.getNestedType().getId()). - createifnonexistant(dt.createIfNonExistent()). - removeifzero(dt.removeIfZero())); + datatype(dt.getNestedType().getId()). + createifnonexistant(dt.createIfNonExistent()). + removeifzero(dt.removeIfZero())); } else if (type instanceof MapDataType) { MapDataType mtype = (MapDataType) type; builder.maptype(new Datatype.Maptype.Builder(). - keytype(mtype.getKeyType().getId()). - valtype(mtype.getValueType().getId())); + keytype(mtype.getKeyType().getId()). + valtype(mtype.getValueType().getId())); } else if (type instanceof DocumentType) { throw new IllegalArgumentException("Can not create config for unadorned document type: " + type.getName()); } else if (type instanceof NewDocumentType) { @@ -154,7 +174,7 @@ public class DocumentManager { ReferenceDataType refType = (ReferenceDataType) type; builder.referencetype(new Datatype.Referencetype.Builder().target_type_id(refType.getTargetType().getId())); } else { - throw new IllegalArgumentException("Can not create config for data type '" + type.getName()); + throw new IllegalArgumentException("Can not create config for data type " + type + " of class " + type.getClass()); } } @@ -176,4 +196,271 @@ public class DocumentManager { } } + + // Alternate (new) way to build config: + + public DocumentmanagerConfig.Builder produceDocTypes(DocumentModel model, DocumentmanagerConfig.Builder builder) { + builder.usev8geopositions(this.useV8GeoPositions); + Map<NewDocumentType.Name, NewDocumentType> produced = new HashMap<>(); + var indexMap = new IdxMap(); + for (NewDocumentType documentType : model.getDocumentManager().getTypes()) { + docTypeInheritOrder(documentType, builder, produced, indexMap); + } + indexMap.verifyAllDone(); + return builder; + } + + private void docTypeInheritOrder(NewDocumentType documentType, + DocumentmanagerConfig.Builder builder, + Map<NewDocumentType.Name, NewDocumentType> produced, + IdxMap indexMap) + { + if (! produced.containsKey(documentType.getFullName())) { + for (NewDocumentType inherited : documentType.getInherited()) { + docTypeInheritOrder(inherited, builder, produced, indexMap); + } + docTypeBuild(documentType, builder, indexMap); + produced.put(documentType.getFullName(), documentType); + } + } + + static private class IdxMap { + private Map<Integer, Boolean> doneMap = new HashMap<>(); + private Map<Object, Integer> map = new IdentityHashMap<>(); + void add(Object someType) { + assert(someType != null); + // the adding of "10000" here is mostly to make it more + // unique to grep for when debugging + int nextIdx = 10000 + map.size(); + map.computeIfAbsent(someType, k -> nextIdx); + } + int idxOf(Object someType) { + if (someType instanceof DocumentType) { + var dt = (DocumentType) someType; + if (dt.getId() == 8) { + return idxOf(VespaDocumentType.INSTANCE); + } + } + add(someType); + return map.get(someType); + } + boolean isDone(Object someType) { + return doneMap.computeIfAbsent(idxOf(someType), k -> false); + } + void setDone(Object someType) { + assert(! isDone(someType)); + doneMap.put(idxOf(someType), true); + } + void verifyAllDone() { + for (var entry : map.entrySet()) { + Object needed = entry.getKey(); + if (! isDone(needed)) { + throw new IllegalArgumentException("Could not generate config for all needed types, missing: " + + needed + " of class " + needed.getClass()); + } + } + } + } + + private void docTypeBuild(NewDocumentType documentType, DocumentmanagerConfig.Builder builder, IdxMap indexMap) { + DocumentmanagerConfig.Doctype.Builder db = new DocumentmanagerConfig.Doctype.Builder(); + db. + idx(indexMap.idxOf(documentType)). + name(documentType.getName()). + contentstruct(indexMap.idxOf(documentType.getHeader())); + docTypeBuildFieldSets(documentType.getFieldSets(), db); + docTypeBuildImportedFields(documentType.getImportedFieldNames(), db); + for (NewDocumentType inherited : documentType.getInherited()) { + db.inherits(b -> b.idx(indexMap.idxOf(inherited))); + } + docTypeBuildAnyType(documentType.getHeader(), db, indexMap); + for (DataType dt : documentType.getAllTypes().getTypes()) { + docTypeBuildAnyType(dt, db, indexMap); + } + for (AnnotationType annotation : documentType.getAnnotations()) { + docTypeBuildAnnotationType(annotation, db, indexMap); + } + builder.doctype(db); + indexMap.setDone(documentType); + } + + private void docTypeBuildFieldSets(Set<FieldSet> fieldSets, DocumentmanagerConfig.Doctype.Builder db) { + for (FieldSet fs : fieldSets) { + docTypeBuildOneFieldSet(fs, db); + } + } + + private void docTypeBuildOneFieldSet(FieldSet fs, DocumentmanagerConfig.Doctype.Builder db) { + db.fieldsets(fs.getName(), new DocumentmanagerConfig.Doctype.Fieldsets.Builder().fields(fs.getFieldNames())); + } + + private void docTypeBuildAnnotationType(AnnotationType annotation, DocumentmanagerConfig.Doctype.Builder builder, IdxMap indexMap) { + if (indexMap.isDone(annotation)) { + return; + } + indexMap.setDone(annotation); + var annBuilder = new DocumentmanagerConfig.Doctype.Annotationtype.Builder(); + annBuilder + .idx(indexMap.idxOf(annotation)) + .name(annotation.getName()) + .internalid(annotation.getId()); + DataType nested = annotation.getDataType(); + if (nested != null) { + annBuilder.datatype(indexMap.idxOf(nested)); + docTypeBuildAnyType(nested, builder, indexMap); + } + for (AnnotationType inherited : annotation.getInheritedTypes()) { + annBuilder.inherits(inhBuilder -> inhBuilder.idx(indexMap.idxOf(inherited))); + + } + builder.annotationtype(annBuilder); + } + + @SuppressWarnings("deprecation") + private void docTypeBuildAnyType(DataType type, DocumentmanagerConfig.Doctype.Builder documentBuilder, IdxMap indexMap) { + if (indexMap.isDone(type)) { + return; + } + if (type instanceof NewDocumentType) { + // should be in the top-level list and handled there + return; + } + if ((type instanceof DocumentType) && (type.getId() == 8)) { + // special handling + return; + } + indexMap.setDone(type); + if (type instanceof TemporaryStructuredDataType) { + throw new IllegalArgumentException("Can not create config for temporary data type: " + type.getName()); + } if (type instanceof StructDataType) { + docTypeBuildOneType((StructDataType) type, documentBuilder, indexMap); + } else if (type instanceof ArrayDataType) { + docTypeBuildOneType((ArrayDataType) type, documentBuilder, indexMap); + } else if (type instanceof WeightedSetDataType) { + docTypeBuildOneType((WeightedSetDataType) type, documentBuilder, indexMap); + } else if (type instanceof MapDataType) { + docTypeBuildOneType((MapDataType) type, documentBuilder, indexMap); + } else if (type instanceof AnnotationReferenceDataType) { + docTypeBuildOneType((AnnotationReferenceDataType) type, documentBuilder, indexMap); + } else if (type instanceof TensorDataType) { + docTypeBuildOneType((TensorDataType) type, documentBuilder, indexMap); + } else if (type instanceof ReferenceDataType) { + docTypeBuildOneType((ReferenceDataType) type, documentBuilder, indexMap); + } else if (type instanceof PrimitiveDataType) { + docTypeBuildOneType((PrimitiveDataType) type, documentBuilder, indexMap); + } else if (type instanceof DocumentType) { + throw new IllegalArgumentException("Can not create config for unadorned document type: " + type.getName() + " id "+type.getId()); + } else { + throw new IllegalArgumentException("Can not create config for data type " + type + " of class " + type.getClass()); + } + } + + private void docTypeBuildImportedFields(Collection<String> fieldNames, DocumentmanagerConfig.Doctype.Builder builder) { + for (String fieldName : fieldNames) { + builder.importedfield(ib -> ib.name(fieldName)); + } + } + + private void docTypeBuildOneType(StructDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + var structBuilder = new DocumentmanagerConfig.Doctype.Structtype.Builder(); + structBuilder + .idx(indexMap.idxOf(type)) + .name(type.getName()); + for (DataType inherited : type.getInheritedTypes()) { + structBuilder.inherits(inheritBuilder -> inheritBuilder + .type(indexMap.idxOf(inherited))); + docTypeBuildAnyType(inherited, builder, indexMap); + } + for (com.yahoo.document.Field field : type.getFieldsThisTypeOnly()) { + DataType fieldType = field.getDataType(); + structBuilder.field(fieldBuilder -> fieldBuilder + .name(field.getName()) + .internalid(field.getId()) + .type(indexMap.idxOf(fieldType))); + docTypeBuildAnyType(fieldType, builder, indexMap); + } + builder.structtype(structBuilder); + } + + private void docTypeBuildOneType(PrimitiveDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + builder.primitivetype(primBuilder -> primBuilder + .idx(indexMap.idxOf(type)) + .name(type.getName())); + } + + private void docTypeBuildOneType(TensorDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + var tt = type.getTensorType(); + String detailed = (tt != null) ? tt.toString() : "tensor"; + builder.tensortype(tensorBuilder -> tensorBuilder + .idx(indexMap.idxOf(type)) + .detailedtype(detailed)); + + } + + private void docTypeBuildOneType(ArrayDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + DataType nested = type.getNestedType(); + builder.arraytype(arrayBuilder -> arrayBuilder + .idx(indexMap.idxOf(type)) + .elementtype(indexMap.idxOf(nested))); + docTypeBuildAnyType(nested, builder, indexMap); + } + + private void docTypeBuildOneType(WeightedSetDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + DataType nested = type.getNestedType(); + builder.wsettype(wsetBuilder -> wsetBuilder + .idx(indexMap.idxOf(type)) + .elementtype(indexMap.idxOf(nested)) + .createifnonexistent(type.createIfNonExistent()) + .removeifzero(type.removeIfZero())); + docTypeBuildAnyType(nested, builder, indexMap); + } + + private void docTypeBuildOneType(MapDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + DataType keytype = type.getKeyType(); + DataType valtype = type.getValueType(); + builder.maptype(mapBuilder -> mapBuilder + .idx(indexMap.idxOf(type)) + .keytype(indexMap.idxOf(keytype)) + .valuetype(indexMap.idxOf(valtype))); + docTypeBuildAnyType(keytype, builder, indexMap); + docTypeBuildAnyType(valtype, builder, indexMap); + } + + private void docTypeBuildOneType(AnnotationReferenceDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + builder.annotationref(arefBuilder -> arefBuilder + .idx(indexMap.idxOf(type)) + .annotationtype(indexMap.idxOf(type.getAnnotationType()))); + } + + private void docTypeBuildOneType(ReferenceDataType type, + DocumentmanagerConfig.Doctype.Builder builder, + IdxMap indexMap) + { + builder.documentref(docrefBuilder -> docrefBuilder + .idx(indexMap.idxOf(type)) + .targettype(indexMap.idxOf(type.getTargetType()))); + + } + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java index d5500e7d040..13c3c229acb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java @@ -435,13 +435,7 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster> redundancy.getConfig(builder); } - if ((feedSequencerType == ProtonConfig.Indexing.Optimize.Enum.THROUGHPUT) && (visibilityDelay == 0.0)) { - // THROUGHPUT and zero visibilityDelay is inconsistent and currently a suboptimal combination, defaulting to LATENCY. - // TODO: Once we have figured out optimal combination this limitation will be cleaned up. - builder.indexing.optimize(ProtonConfig.Indexing.Optimize.Enum.LATENCY); - } else { - builder.indexing.optimize(feedSequencerType); - } + builder.indexing.optimize(feedSequencerType); builder.indexing.tasklimit(feedTaskLimit); builder.feeding.master_task_limit(feedMasterTaskLimit); builder.feeding.shared_field_writer_executor(sharedFieldWriterExecutor); diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index d3d992c11f5..92633f61e67 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -835,7 +835,7 @@ DataType dataType() : String typeName = null; boolean isArrayOldStyle = false; DataType mapType = null; - DataType arrayType = null; + DataType arrayType = null; DataType wsetType = null; TensorType tensorType; TemporaryStructuredDataType referenceType; diff --git a/config-model/src/test/derived/inheritance/documentmanager.cfg b/config-model/src/test/derived/inheritance/documentmanager.cfg index 49bf53bce8d..4a25f8c3a64 100644 --- a/config-model/src/test/derived/inheritance/documentmanager.cfg +++ b/config-model/src/test/derived/inheritance/documentmanager.cfg @@ -1,116 +1,106 @@ enablecompression false usev8geopositions false -datatype[].id 1381038251 -datatype[].structtype[].name "position" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "x" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "y" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 990971719 -datatype[].structtype[].name "grandparent.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "onlygrandparent" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "overridden" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id -154107656 -datatype[].documenttype[].name "grandparent" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 990971719 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlygrandparent" -datatype[].documenttype[].fieldsets{[document]}.fields[] "overridden" -datatype[].id 1306663898 -datatype[].structtype[].name "mother.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "onlymother" -datatype[].structtype[].field[].datatype 2 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "overridden" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id -158393403 -datatype[].documenttype[].name "mother" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "grandparent" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 1306663898 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlygrandparent" -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlymother" -datatype[].documenttype[].fieldsets{[document]}.fields[] "overridden" -datatype[].id 2126589281 -datatype[].structtype[].name "father.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "onlyfather" -datatype[].structtype[].field[].datatype 2 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "overridden" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 986686494 -datatype[].documenttype[].name "father" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "grandparent" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 2126589281 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlyfather" -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlygrandparent" -datatype[].documenttype[].fieldsets{[document]}.fields[] "overridden" -datatype[].id 81425825 -datatype[].structtype[].name "child.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "onlychild" -datatype[].structtype[].field[].datatype 2 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "overridden" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 746267614 -datatype[].documenttype[].name "child" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "father" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "mother" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 81425825 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlychild" -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlyfather" -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlygrandparent" -datatype[].documenttype[].fieldsets{[document]}.fields[] "onlymother" -datatype[].documenttype[].fieldsets{[document]}.fields[] "overridden" +doctype[0].name "document" +doctype[0].idx 10000 +doctype[0].contentstruct 10001 +doctype[0].primitivetype[0].idx 10002 +doctype[0].primitivetype[0].name "byte" +doctype[0].primitivetype[1].idx 10003 +doctype[0].primitivetype[1].name "int" +doctype[0].primitivetype[2].idx 10004 +doctype[0].primitivetype[2].name "long" +doctype[0].primitivetype[3].idx 10005 +doctype[0].primitivetype[3].name "string" +doctype[0].primitivetype[4].idx 10006 +doctype[0].primitivetype[4].name "raw" +doctype[0].primitivetype[5].idx 10008 +doctype[0].primitivetype[5].name "float" +doctype[0].primitivetype[6].idx 10009 +doctype[0].primitivetype[6].name "double" +doctype[0].primitivetype[7].idx 10011 +doctype[0].primitivetype[7].name "uri" +doctype[0].primitivetype[8].idx 10012 +doctype[0].primitivetype[8].name "predicate" +doctype[0].primitivetype[9].idx 10013 +doctype[0].primitivetype[9].name "bool" +doctype[0].primitivetype[10].idx 10014 +doctype[0].primitivetype[10].name "float16" +doctype[0].wsettype[0].idx 10007 +doctype[0].wsettype[0].elementtype 10005 +doctype[0].wsettype[0].createifnonexistent true +doctype[0].wsettype[0].removeifzero true +doctype[0].structtype[0].idx 10001 +doctype[0].structtype[0].name "document.header" +doctype[0].structtype[1].idx 10010 +doctype[0].structtype[1].name "position" +doctype[0].structtype[1].field[0].name "x" +doctype[0].structtype[1].field[0].internalid 914677694 +doctype[0].structtype[1].field[0].type 10003 +doctype[0].structtype[1].field[1].name "y" +doctype[0].structtype[1].field[1].internalid 900009410 +doctype[0].structtype[1].field[1].type 10003 +doctype[1].name "grandparent" +doctype[1].idx 10015 +doctype[1].inherits[0].idx 10000 +doctype[1].contentstruct 10016 +doctype[1].fieldsets{[document]}.fields[0] "onlygrandparent" +doctype[1].fieldsets{[document]}.fields[1] "overridden" +doctype[1].structtype[0].idx 10016 +doctype[1].structtype[0].name "grandparent.header" +doctype[1].structtype[0].field[0].name "onlygrandparent" +doctype[1].structtype[0].field[0].internalid 1456982690 +doctype[1].structtype[0].field[0].type 10003 +doctype[1].structtype[0].field[1].name "overridden" +doctype[1].structtype[0].field[1].internalid 1314355415 +doctype[1].structtype[0].field[1].type 10003 +doctype[2].name "mother" +doctype[2].idx 10017 +doctype[2].inherits[0].idx 10015 +doctype[2].inherits[1].idx 10000 +doctype[2].contentstruct 10018 +doctype[2].fieldsets{[document]}.fields[0] "onlygrandparent" +doctype[2].fieldsets{[document]}.fields[1] "onlymother" +doctype[2].fieldsets{[document]}.fields[2] "overridden" +doctype[2].structtype[0].idx 10018 +doctype[2].structtype[0].name "mother.header" +doctype[2].structtype[0].field[0].name "onlymother" +doctype[2].structtype[0].field[0].internalid 1390999339 +doctype[2].structtype[0].field[0].type 10005 +doctype[2].structtype[0].field[1].name "overridden" +doctype[2].structtype[0].field[1].internalid 1314355415 +doctype[2].structtype[0].field[1].type 10003 +doctype[3].name "father" +doctype[3].idx 10019 +doctype[3].inherits[0].idx 10015 +doctype[3].inherits[1].idx 10000 +doctype[3].contentstruct 10020 +doctype[3].fieldsets{[document]}.fields[0] "onlyfather" +doctype[3].fieldsets{[document]}.fields[1] "onlygrandparent" +doctype[3].fieldsets{[document]}.fields[2] "overridden" +doctype[3].structtype[0].idx 10020 +doctype[3].structtype[0].name "father.header" +doctype[3].structtype[0].field[0].name "onlyfather" +doctype[3].structtype[0].field[0].internalid 1083094308 +doctype[3].structtype[0].field[0].type 10005 +doctype[3].structtype[0].field[1].name "overridden" +doctype[3].structtype[0].field[1].internalid 1314355415 +doctype[3].structtype[0].field[1].type 10003 +doctype[4].name "child" +doctype[4].idx 10021 +doctype[4].inherits[0].idx 10000 +doctype[4].inherits[1].idx 10019 +doctype[4].inherits[2].idx 10017 +doctype[4].contentstruct 10022 +doctype[4].fieldsets{[document]}.fields[0] "onlychild" +doctype[4].fieldsets{[document]}.fields[1] "onlyfather" +doctype[4].fieldsets{[document]}.fields[2] "onlygrandparent" +doctype[4].fieldsets{[document]}.fields[3] "onlymother" +doctype[4].fieldsets{[document]}.fields[4] "overridden" +doctype[4].structtype[0].idx 10022 +doctype[4].structtype[0].name "child.header" +doctype[4].structtype[0].field[0].name "onlychild" +doctype[4].structtype[0].field[0].internalid 1737375598 +doctype[4].structtype[0].field[0].type 10005 +doctype[4].structtype[0].field[1].name "overridden" +doctype[4].structtype[0].field[1].internalid 1314355415 +doctype[4].structtype[0].field[1].type 10003 diff --git a/config-model/src/test/derived/inheritfromgrandparent/documentmanager.cfg b/config-model/src/test/derived/inheritfromgrandparent/documentmanager.cfg index 8fa93b61569..cc76fe939b0 100644 --- a/config-model/src/test/derived/inheritfromgrandparent/documentmanager.cfg +++ b/config-model/src/test/derived/inheritfromgrandparent/documentmanager.cfg @@ -1,75 +1,70 @@ enablecompression false usev8geopositions false -datatype[].id 1381038251 -datatype[].structtype[].name "position" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "x" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "y" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 1246084544 -datatype[].structtype[].name "grandparent_struct" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "grandparent_field" -datatype[].structtype[].field[].datatype 2 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 990971719 -datatype[].structtype[].name "grandparent.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].id -154107656 -datatype[].documenttype[].name "grandparent" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 990971719 -datatype[].documenttype[].bodystruct 0 -datatype[].id 836075987 -datatype[].structtype[].name "parent.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].id 1175161836 -datatype[].documenttype[].name "parent" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "grandparent" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 836075987 -datatype[].documenttype[].bodystruct 0 -datatype[].id 81425825 -datatype[].structtype[].name "child.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "child_field" -datatype[].structtype[].field[].datatype 1246084544 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 746267614 -datatype[].documenttype[].name "child" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "parent" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 81425825 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[document]}.fields[] "child_field" +doctype[0].name "document" +doctype[0].idx 10000 +doctype[0].contentstruct 10001 +doctype[0].primitivetype[0].idx 10002 +doctype[0].primitivetype[0].name "byte" +doctype[0].primitivetype[1].idx 10003 +doctype[0].primitivetype[1].name "int" +doctype[0].primitivetype[2].idx 10004 +doctype[0].primitivetype[2].name "long" +doctype[0].primitivetype[3].idx 10005 +doctype[0].primitivetype[3].name "string" +doctype[0].primitivetype[4].idx 10006 +doctype[0].primitivetype[4].name "raw" +doctype[0].primitivetype[5].idx 10008 +doctype[0].primitivetype[5].name "float" +doctype[0].primitivetype[6].idx 10009 +doctype[0].primitivetype[6].name "double" +doctype[0].primitivetype[7].idx 10011 +doctype[0].primitivetype[7].name "uri" +doctype[0].primitivetype[8].idx 10012 +doctype[0].primitivetype[8].name "predicate" +doctype[0].primitivetype[9].idx 10013 +doctype[0].primitivetype[9].name "bool" +doctype[0].primitivetype[10].idx 10014 +doctype[0].primitivetype[10].name "float16" +doctype[0].wsettype[0].idx 10007 +doctype[0].wsettype[0].elementtype 10005 +doctype[0].wsettype[0].createifnonexistent true +doctype[0].wsettype[0].removeifzero true +doctype[0].structtype[0].idx 10001 +doctype[0].structtype[0].name "document.header" +doctype[0].structtype[1].idx 10010 +doctype[0].structtype[1].name "position" +doctype[0].structtype[1].field[0].name "x" +doctype[0].structtype[1].field[0].internalid 914677694 +doctype[0].structtype[1].field[0].type 10003 +doctype[0].structtype[1].field[1].name "y" +doctype[0].structtype[1].field[1].internalid 900009410 +doctype[0].structtype[1].field[1].type 10003 +doctype[1].name "grandparent" +doctype[1].idx 10015 +doctype[1].inherits[0].idx 10000 +doctype[1].contentstruct 10016 +doctype[1].structtype[0].idx 10016 +doctype[1].structtype[0].name "grandparent.header" +doctype[1].structtype[1].idx 10017 +doctype[1].structtype[1].name "grandparent_struct" +doctype[1].structtype[1].field[0].name "grandparent_field" +doctype[1].structtype[1].field[0].internalid 18801796 +doctype[1].structtype[1].field[0].type 10005 +doctype[2].name "parent" +doctype[2].idx 10018 +doctype[2].inherits[0].idx 10015 +doctype[2].inherits[1].idx 10000 +doctype[2].contentstruct 10019 +doctype[2].structtype[0].idx 10019 +doctype[2].structtype[0].name "parent.header" +doctype[3].name "child" +doctype[3].idx 10020 +doctype[3].inherits[0].idx 10000 +doctype[3].inherits[1].idx 10018 +doctype[3].contentstruct 10021 +doctype[3].fieldsets{[document]}.fields[0] "child_field" +doctype[3].structtype[0].idx 10021 +doctype[3].structtype[0].name "child.header" +doctype[3].structtype[0].field[0].name "child_field" +doctype[3].structtype[0].field[0].internalid 129089854 +doctype[3].structtype[0].field[0].type 10017 diff --git a/config-model/src/test/derived/inheritfromparent/documentmanager.cfg b/config-model/src/test/derived/inheritfromparent/documentmanager.cfg index e3b6ca87689..3c7280094be 100644 --- a/config-model/src/test/derived/inheritfromparent/documentmanager.cfg +++ b/config-model/src/test/derived/inheritfromparent/documentmanager.cfg @@ -1,67 +1,71 @@ enablecompression false usev8geopositions false -datatype[].id 1381038251 -datatype[].structtype[].name "position" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "x" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "y" -datatype[].structtype[].field[].datatype 0 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 1091188812 -datatype[].structtype[].name "parent_struct" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "parent_field" -datatype[].structtype[].field[].datatype 2 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 836075987 -datatype[].structtype[].name "parent.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "weight_src" -datatype[].structtype[].field[].datatype 1 -datatype[].structtype[].field[].detailedtype "" -datatype[].structtype[].field[].name "weight" -datatype[].structtype[].field[].datatype 1 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 1175161836 -datatype[].documenttype[].name "parent" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 836075987 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[]}.fields[] "weight_src" -datatype[].id 81425825 -datatype[].structtype[].name "child.header" -datatype[].structtype[].version 0 -datatype[].structtype[].compresstype NONE -datatype[].structtype[].compresslevel 0 -datatype[].structtype[].compressthreshold 95 -datatype[].structtype[].compressminsize 800 -datatype[].structtype[].field[].name "child_field" -datatype[].structtype[].field[].datatype 1091188812 -datatype[].structtype[].field[].detailedtype "" -datatype[].id 746267614 -datatype[].documenttype[].name "child" -datatype[].documenttype[].version 0 -datatype[].documenttype[].inherits[].name "document" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].inherits[].name "parent" -datatype[].documenttype[].inherits[].version 0 -datatype[].documenttype[].headerstruct 81425825 -datatype[].documenttype[].bodystruct 0 -datatype[].documenttype[].fieldsets{[]}.fields[] "child_field" -datatype[].documenttype[].fieldsets{[]}.fields[] "weight_src" +doctype[0].name "document" +doctype[0].idx 10000 +doctype[0].contentstruct 10001 +doctype[0].primitivetype[0].idx 10002 +doctype[0].primitivetype[0].name "byte" +doctype[0].primitivetype[1].idx 10003 +doctype[0].primitivetype[1].name "int" +doctype[0].primitivetype[2].idx 10004 +doctype[0].primitivetype[2].name "long" +doctype[0].primitivetype[3].idx 10005 +doctype[0].primitivetype[3].name "string" +doctype[0].primitivetype[4].idx 10006 +doctype[0].primitivetype[4].name "raw" +doctype[0].primitivetype[5].idx 10008 +doctype[0].primitivetype[5].name "float" +doctype[0].primitivetype[6].idx 10009 +doctype[0].primitivetype[6].name "double" +doctype[0].primitivetype[7].idx 10011 +doctype[0].primitivetype[7].name "uri" +doctype[0].primitivetype[8].idx 10012 +doctype[0].primitivetype[8].name "predicate" +doctype[0].primitivetype[9].idx 10013 +doctype[0].primitivetype[9].name "bool" +doctype[0].primitivetype[10].idx 10014 +doctype[0].primitivetype[10].name "float16" +doctype[0].wsettype[0].idx 10007 +doctype[0].wsettype[0].elementtype 10005 +doctype[0].wsettype[0].createifnonexistent true +doctype[0].wsettype[0].removeifzero true +doctype[0].structtype[0].idx 10001 +doctype[0].structtype[0].name "document.header" +doctype[0].structtype[1].idx 10010 +doctype[0].structtype[1].name "position" +doctype[0].structtype[1].field[0].name "x" +doctype[0].structtype[1].field[0].internalid 914677694 +doctype[0].structtype[1].field[0].type 10003 +doctype[0].structtype[1].field[1].name "y" +doctype[0].structtype[1].field[1].internalid 900009410 +doctype[0].structtype[1].field[1].type 10003 +doctype[1].name "parent" +doctype[1].idx 10015 +doctype[1].inherits[0].idx 10000 +doctype[1].contentstruct 10016 +doctype[1].fieldsets{[document]}.fields[0] "weight_src" +doctype[1].structtype[0].idx 10016 +doctype[1].structtype[0].name "parent.header" +doctype[1].structtype[0].field[0].name "weight_src" +doctype[1].structtype[0].field[0].internalid 1225660233 +doctype[1].structtype[0].field[0].type 10008 +doctype[1].structtype[0].field[1].name "weight" +doctype[1].structtype[0].field[1].internalid 1001392207 +doctype[1].structtype[0].field[1].type 10008 +doctype[1].structtype[1].idx 10017 +doctype[1].structtype[1].name "parent_struct" +doctype[1].structtype[1].field[0].name "parent_field" +doctype[1].structtype[1].field[0].internalid 933533022 +doctype[1].structtype[1].field[0].type 10005 +doctype[2].name "child" +doctype[2].idx 10018 +doctype[2].inherits[0].idx 10000 +doctype[2].inherits[1].idx 10015 +doctype[2].contentstruct 10019 +doctype[2].fieldsets{[document]}.fields[0] "child_field" +doctype[2].fieldsets{[document]}.fields[1] "weight_src" +doctype[2].structtype[0].idx 10019 +doctype[2].structtype[0].name "child.header" +doctype[2].structtype[0].field[0].name "child_field" +doctype[2].structtype[0].field[0].internalid 1814271363 +doctype[2].structtype[0].field[0].type 10017 diff --git a/config-model/src/test/derived/inheritstruct/child.sd b/config-model/src/test/derived/inheritstruct/child.sd index 0ac4048e5fa..fcc3cececc3 100644 --- a/config-model/src/test/derived/inheritstruct/child.sd +++ b/config-model/src/test/derived/inheritstruct/child.sd @@ -1,9 +1,22 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. search child { document child inherits parent { + struct other_struct inherits my_struct { + field my_int type int {} + } + struct wrapper { + field wrapped type my_struct {} + } + field child_struct_field type my_struct { indexing: summary | index match: prefix } + field other_field type other_struct { + indexing: summary + } + field wrapped_field type wrapper { + indexing: summary + } } } diff --git a/config-model/src/test/derived/inheritstruct/index-info.cfg b/config-model/src/test/derived/inheritstruct/index-info.cfg index 21e68f0c127..5afa91ea1bb 100644 --- a/config-model/src/test/derived/inheritstruct/index-info.cfg +++ b/config-model/src/test/derived/inheritstruct/index-info.cfg @@ -1,25 +1,51 @@ -indexinfo[].name "child" -indexinfo[].command[].indexname "sddocname" -indexinfo[].command[].command "index" -indexinfo[].command[].indexname "sddocname" -indexinfo[].command[].command "word" -indexinfo[].command[].indexname "child_struct_field.my_str" -indexinfo[].command[].command "index" -indexinfo[].command[].indexname "child_struct_field.my_str" -indexinfo[].command[].command "lowercase" -indexinfo[].command[].indexname "child_struct_field.my_str" -indexinfo[].command[].command "stem:BEST" -indexinfo[].command[].indexname "child_struct_field.my_str" -indexinfo[].command[].command "normalize" -indexinfo[].command[].indexname "child_struct_field.my_str" -indexinfo[].command[].command "plain-tokens" -indexinfo[].command[].indexname "child_struct_field.my_str" -indexinfo[].command[].command "type string" -indexinfo[].command[].indexname "child_struct_field" -indexinfo[].command[].command "index" -indexinfo[].command[].indexname "child_struct_field" -indexinfo[].command[].command "lowercase" -indexinfo[].command[].indexname "child_struct_field" -indexinfo[].command[].command "plain-tokens" -indexinfo[].command[].indexname "child_struct_field" -indexinfo[].command[].command "type my_struct" +indexinfo[0].name "child" +indexinfo[0].command[0].indexname "sddocname" +indexinfo[0].command[0].command "index" +indexinfo[0].command[1].indexname "sddocname" +indexinfo[0].command[1].command "word" +indexinfo[0].command[2].indexname "child_struct_field.my_str" +indexinfo[0].command[2].command "index" +indexinfo[0].command[3].indexname "child_struct_field.my_str" +indexinfo[0].command[3].command "lowercase" +indexinfo[0].command[4].indexname "child_struct_field.my_str" +indexinfo[0].command[4].command "stem:BEST" +indexinfo[0].command[5].indexname "child_struct_field.my_str" +indexinfo[0].command[5].command "normalize" +indexinfo[0].command[6].indexname "child_struct_field.my_str" +indexinfo[0].command[6].command "plain-tokens" +indexinfo[0].command[7].indexname "child_struct_field.my_str" +indexinfo[0].command[7].command "type string" +indexinfo[0].command[8].indexname "child_struct_field" +indexinfo[0].command[8].command "index" +indexinfo[0].command[9].indexname "child_struct_field" +indexinfo[0].command[9].command "lowercase" +indexinfo[0].command[10].indexname "child_struct_field" +indexinfo[0].command[10].command "plain-tokens" +indexinfo[0].command[11].indexname "child_struct_field" +indexinfo[0].command[11].command "type my_struct" +indexinfo[0].command[12].indexname "other_field.my_str" +indexinfo[0].command[12].command "index" +indexinfo[0].command[13].indexname "other_field.my_str" +indexinfo[0].command[13].command "type string" +indexinfo[0].command[14].indexname "other_field.my_int" +indexinfo[0].command[14].command "index" +indexinfo[0].command[15].indexname "other_field.my_int" +indexinfo[0].command[15].command "numerical" +indexinfo[0].command[16].indexname "other_field.my_int" +indexinfo[0].command[16].command "type int" +indexinfo[0].command[17].indexname "other_field" +indexinfo[0].command[17].command "index" +indexinfo[0].command[18].indexname "other_field" +indexinfo[0].command[18].command "type other_struct" +indexinfo[0].command[19].indexname "wrapped_field.wrapped.my_str" +indexinfo[0].command[19].command "index" +indexinfo[0].command[20].indexname "wrapped_field.wrapped.my_str" +indexinfo[0].command[20].command "type string" +indexinfo[0].command[21].indexname "wrapped_field.wrapped" +indexinfo[0].command[21].command "index" +indexinfo[0].command[22].indexname "wrapped_field.wrapped" +indexinfo[0].command[22].command "type my_struct" +indexinfo[0].command[23].indexname "wrapped_field" +indexinfo[0].command[23].command "index" +indexinfo[0].command[24].indexname "wrapped_field" +indexinfo[0].command[24].command "type wrapper"
\ No newline at end of file diff --git a/config-model/src/test/derived/structinheritance/bad.sd b/config-model/src/test/derived/structinheritance/bad.sd new file mode 100644 index 00000000000..ef5137842ec --- /dev/null +++ b/config-model/src/test/derived/structinheritance/bad.sd @@ -0,0 +1,18 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +search bad { + document bad { + struct base { + field name type string {} + field year type int {} + } + struct onechild inherits base { + field between type string {} + } + struct childtwo inherits onechild { + field mine type string {} + field name type string {} + } + field f1 type onechild {} + } +} diff --git a/config-model/src/test/derived/structinheritance/documentmanager.cfg b/config-model/src/test/derived/structinheritance/documentmanager.cfg new file mode 100644 index 00000000000..20994bacca2 --- /dev/null +++ b/config-model/src/test/derived/structinheritance/documentmanager.cfg @@ -0,0 +1,71 @@ +enablecompression false +usev8geopositions false +datatype[0].id 1381038251 +datatype[0].structtype[0].name "position" +datatype[0].structtype[0].version 0 +datatype[0].structtype[0].compresstype NONE +datatype[0].structtype[0].compresslevel 0 +datatype[0].structtype[0].compressthreshold 95 +datatype[0].structtype[0].compressminsize 800 +datatype[0].structtype[0].field[0].name "x" +datatype[0].structtype[0].field[0].datatype 0 +datatype[0].structtype[0].field[0].detailedtype "" +datatype[0].structtype[0].field[1].name "y" +datatype[0].structtype[0].field[1].datatype 0 +datatype[0].structtype[0].field[1].detailedtype "" +datatype[1].id -1396204461 +datatype[1].structtype[0].name "base" +datatype[1].structtype[0].version 0 +datatype[1].structtype[0].compresstype NONE +datatype[1].structtype[0].compresslevel 0 +datatype[1].structtype[0].compressthreshold 95 +datatype[1].structtype[0].compressminsize 800 +datatype[1].structtype[0].field[0].name "name" +datatype[1].structtype[0].field[0].datatype 2 +datatype[1].structtype[0].field[0].detailedtype "" +datatype[2].id 746267614 +datatype[2].structtype[0].name "child" +datatype[2].structtype[0].version 0 +datatype[2].structtype[0].compresstype NONE +datatype[2].structtype[0].compresslevel 0 +datatype[2].structtype[0].compressthreshold 95 +datatype[2].structtype[0].compressminsize 800 +datatype[2].structtype[0].field[0].name "age" +datatype[2].structtype[0].field[0].datatype 0 +datatype[2].structtype[0].field[0].detailedtype "" +datatype[2].structtype[0].inherits[0].name "base" +datatype[2].structtype[0].inherits[0].version 0 +datatype[3].id 1811766610 +datatype[3].structtype[0].name "grandchild" +datatype[3].structtype[0].version 0 +datatype[3].structtype[0].compresstype NONE +datatype[3].structtype[0].compresslevel 0 +datatype[3].structtype[0].compressthreshold 95 +datatype[3].structtype[0].compressminsize 800 +datatype[3].structtype[0].field[0].name "toy" +datatype[3].structtype[0].field[0].datatype 2 +datatype[3].structtype[0].field[0].detailedtype "" +datatype[3].structtype[0].inherits[0].name "child" +datatype[3].structtype[0].inherits[0].version 0 +datatype[4].id -2142109237 +datatype[4].structtype[0].name "simple.header" +datatype[4].structtype[0].version 0 +datatype[4].structtype[0].compresstype NONE +datatype[4].structtype[0].compresslevel 0 +datatype[4].structtype[0].compressthreshold 95 +datatype[4].structtype[0].compressminsize 800 +datatype[4].structtype[0].field[0].name "f1" +datatype[4].structtype[0].field[0].datatype 746267614 +datatype[4].structtype[0].field[0].detailedtype "" +datatype[4].structtype[0].field[1].name "f2" +datatype[4].structtype[0].field[1].datatype 1811766610 +datatype[4].structtype[0].field[1].detailedtype "" +datatype[5].id 485659380 +datatype[5].documenttype[0].name "simple" +datatype[5].documenttype[0].version 0 +datatype[5].documenttype[0].inherits[0].name "document" +datatype[5].documenttype[0].inherits[0].version 0 +datatype[5].documenttype[0].headerstruct -2142109237 +datatype[5].documenttype[0].bodystruct 0 +datatype[5].documenttype[0].fieldsets{[document]}.fields[0] "f1" +datatype[5].documenttype[0].fieldsets{[document]}.fields[1] "f2" diff --git a/config-model/src/test/derived/structinheritance/documenttypes.cfg b/config-model/src/test/derived/structinheritance/documenttypes.cfg new file mode 100644 index 00000000000..52a154905c2 --- /dev/null +++ b/config-model/src/test/derived/structinheritance/documenttypes.cfg @@ -0,0 +1,102 @@ +enablecompression false +usev8geopositions false +documenttype[0].id 485659380 +documenttype[0].name "simple" +documenttype[0].version 0 +documenttype[0].headerstruct -2142109237 +documenttype[0].bodystruct 0 +documenttype[0].inherits[0].id 8 +documenttype[0].datatype[0].id 1811766610 +documenttype[0].datatype[0].type STRUCT +documenttype[0].datatype[0].array.element.id 0 +documenttype[0].datatype[0].map.key.id 0 +documenttype[0].datatype[0].map.value.id 0 +documenttype[0].datatype[0].wset.key.id 0 +documenttype[0].datatype[0].wset.createifnonexistent false +documenttype[0].datatype[0].wset.removeifzero false +documenttype[0].datatype[0].annotationref.annotation.id 0 +documenttype[0].datatype[0].sstruct.name "grandchild" +documenttype[0].datatype[0].sstruct.version 0 +documenttype[0].datatype[0].sstruct.compression.type NONE +documenttype[0].datatype[0].sstruct.compression.level 0 +documenttype[0].datatype[0].sstruct.compression.threshold 95 +documenttype[0].datatype[0].sstruct.compression.minsize 200 +documenttype[0].datatype[0].sstruct.field[0].name "toy" +documenttype[0].datatype[0].sstruct.field[0].id 536645790 +documenttype[0].datatype[0].sstruct.field[0].datatype 2 +documenttype[0].datatype[0].sstruct.field[0].detailedtype "" +documenttype[0].datatype[0].sstruct.field[1].name "age" +documenttype[0].datatype[0].sstruct.field[1].id 1862473705 +documenttype[0].datatype[0].sstruct.field[1].datatype 0 +documenttype[0].datatype[0].sstruct.field[1].detailedtype "" +documenttype[0].datatype[0].sstruct.field[2].name "name" +documenttype[0].datatype[0].sstruct.field[2].id 1160796772 +documenttype[0].datatype[0].sstruct.field[2].datatype 2 +documenttype[0].datatype[0].sstruct.field[2].detailedtype "" +documenttype[0].datatype[1].id -1396204461 +documenttype[0].datatype[1].type STRUCT +documenttype[0].datatype[1].array.element.id 0 +documenttype[0].datatype[1].map.key.id 0 +documenttype[0].datatype[1].map.value.id 0 +documenttype[0].datatype[1].wset.key.id 0 +documenttype[0].datatype[1].wset.createifnonexistent false +documenttype[0].datatype[1].wset.removeifzero false +documenttype[0].datatype[1].annotationref.annotation.id 0 +documenttype[0].datatype[1].sstruct.name "base" +documenttype[0].datatype[1].sstruct.version 0 +documenttype[0].datatype[1].sstruct.compression.type NONE +documenttype[0].datatype[1].sstruct.compression.level 0 +documenttype[0].datatype[1].sstruct.compression.threshold 95 +documenttype[0].datatype[1].sstruct.compression.minsize 200 +documenttype[0].datatype[1].sstruct.field[0].name "name" +documenttype[0].datatype[1].sstruct.field[0].id 1160796772 +documenttype[0].datatype[1].sstruct.field[0].datatype 2 +documenttype[0].datatype[1].sstruct.field[0].detailedtype "" +documenttype[0].datatype[2].id 746267614 +documenttype[0].datatype[2].type STRUCT +documenttype[0].datatype[2].array.element.id 0 +documenttype[0].datatype[2].map.key.id 0 +documenttype[0].datatype[2].map.value.id 0 +documenttype[0].datatype[2].wset.key.id 0 +documenttype[0].datatype[2].wset.createifnonexistent false +documenttype[0].datatype[2].wset.removeifzero false +documenttype[0].datatype[2].annotationref.annotation.id 0 +documenttype[0].datatype[2].sstruct.name "child" +documenttype[0].datatype[2].sstruct.version 0 +documenttype[0].datatype[2].sstruct.compression.type NONE +documenttype[0].datatype[2].sstruct.compression.level 0 +documenttype[0].datatype[2].sstruct.compression.threshold 95 +documenttype[0].datatype[2].sstruct.compression.minsize 200 +documenttype[0].datatype[2].sstruct.field[0].name "age" +documenttype[0].datatype[2].sstruct.field[0].id 1862473705 +documenttype[0].datatype[2].sstruct.field[0].datatype 0 +documenttype[0].datatype[2].sstruct.field[0].detailedtype "" +documenttype[0].datatype[2].sstruct.field[1].name "name" +documenttype[0].datatype[2].sstruct.field[1].id 1160796772 +documenttype[0].datatype[2].sstruct.field[1].datatype 2 +documenttype[0].datatype[2].sstruct.field[1].detailedtype "" +documenttype[0].datatype[3].id -2142109237 +documenttype[0].datatype[3].type STRUCT +documenttype[0].datatype[3].array.element.id 0 +documenttype[0].datatype[3].map.key.id 0 +documenttype[0].datatype[3].map.value.id 0 +documenttype[0].datatype[3].wset.key.id 0 +documenttype[0].datatype[3].wset.createifnonexistent false +documenttype[0].datatype[3].wset.removeifzero false +documenttype[0].datatype[3].annotationref.annotation.id 0 +documenttype[0].datatype[3].sstruct.name "simple.header" +documenttype[0].datatype[3].sstruct.version 0 +documenttype[0].datatype[3].sstruct.compression.type NONE +documenttype[0].datatype[3].sstruct.compression.level 0 +documenttype[0].datatype[3].sstruct.compression.threshold 95 +documenttype[0].datatype[3].sstruct.compression.minsize 200 +documenttype[0].datatype[3].sstruct.field[0].name "f1" +documenttype[0].datatype[3].sstruct.field[0].id 750623154 +documenttype[0].datatype[3].sstruct.field[0].datatype 746267614 +documenttype[0].datatype[3].sstruct.field[0].detailedtype "" +documenttype[0].datatype[3].sstruct.field[1].name "f2" +documenttype[0].datatype[3].sstruct.field[1].id 1523850983 +documenttype[0].datatype[3].sstruct.field[1].datatype 1811766610 +documenttype[0].datatype[3].sstruct.field[1].detailedtype "" +documenttype[0].fieldsets{[document]}.fields[0] "f1" +documenttype[0].fieldsets{[document]}.fields[1] "f2" diff --git a/config-model/src/test/derived/structinheritance/simple.sd b/config-model/src/test/derived/structinheritance/simple.sd new file mode 100644 index 00000000000..8b4bb6150c1 --- /dev/null +++ b/config-model/src/test/derived/structinheritance/simple.sd @@ -0,0 +1,17 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +search simple { + document simple { + struct base { + field name type string {} + } + struct child inherits base { + field age type int {} + } + struct grandchild inherits child { + field toy type string {} + } + field f1 type child {} + field f2 type grandchild {} + } +} diff --git a/config-model/src/test/examples/fieldoftypedocument-doctypes.cfg b/config-model/src/test/examples/fieldoftypedocument-doctypes.cfg new file mode 100644 index 00000000000..a7a4c675311 --- /dev/null +++ b/config-model/src/test/examples/fieldoftypedocument-doctypes.cfg @@ -0,0 +1,69 @@ +enablecompression false +usev8geopositions false +doctype[0].name "document" +doctype[0].idx 10000 +doctype[0].contentstruct 10001 +doctype[0].primitivetype[0].idx 10002 +doctype[0].primitivetype[0].name "byte" +doctype[0].primitivetype[1].idx 10003 +doctype[0].primitivetype[1].name "int" +doctype[0].primitivetype[2].idx 10004 +doctype[0].primitivetype[2].name "long" +doctype[0].primitivetype[3].idx 10005 +doctype[0].primitivetype[3].name "string" +doctype[0].primitivetype[4].idx 10006 +doctype[0].primitivetype[4].name "raw" +doctype[0].primitivetype[5].idx 10008 +doctype[0].primitivetype[5].name "float" +doctype[0].primitivetype[6].idx 10009 +doctype[0].primitivetype[6].name "double" +doctype[0].primitivetype[7].idx 10011 +doctype[0].primitivetype[7].name "uri" +doctype[0].primitivetype[8].idx 10012 +doctype[0].primitivetype[8].name "predicate" +doctype[0].primitivetype[9].idx 10013 +doctype[0].primitivetype[9].name "bool" +doctype[0].primitivetype[10].idx 10014 +doctype[0].primitivetype[10].name "float16" +doctype[0].wsettype[0].idx 10007 +doctype[0].wsettype[0].elementtype 10005 +doctype[0].wsettype[0].createifnonexistent true +doctype[0].wsettype[0].removeifzero true +doctype[0].structtype[0].idx 10001 +doctype[0].structtype[0].name "document.header" +doctype[0].structtype[1].idx 10010 +doctype[0].structtype[1].name "position" +doctype[0].structtype[1].field[0].name "x" +doctype[0].structtype[1].field[0].internalid 914677694 +doctype[0].structtype[1].field[0].type 10003 +doctype[0].structtype[1].field[1].name "y" +doctype[0].structtype[1].field[1].internalid 900009410 +doctype[0].structtype[1].field[1].type 10003 +doctype[1].name "book" +doctype[1].idx 10015 +doctype[1].inherits[0].idx 10000 +doctype[1].contentstruct 10016 +doctype[1].fieldsets{[document]}.fields[0] "soundtrack" +doctype[1].structtype[0].idx 10016 +doctype[1].structtype[0].name "book.header" +doctype[1].structtype[0].field[0].name "soundtrack" +doctype[1].structtype[0].field[0].internalid 1258961213 +doctype[1].structtype[0].field[0].type 10017 +doctype[2].name "music" +doctype[2].idx 10017 +doctype[2].inherits[0].idx 10000 +doctype[2].contentstruct 10018 +doctype[2].fieldsets{[document]}.fields[0] "intfield" +doctype[2].fieldsets{[document]}.fields[1] "longfield" +doctype[2].fieldsets{[document]}.fields[2] "stringfield" +doctype[2].structtype[0].idx 10018 +doctype[2].structtype[0].name "music.header" +doctype[2].structtype[0].field[0].name "intfield" +doctype[2].structtype[0].field[0].internalid 435380425 +doctype[2].structtype[0].field[0].type 10003 +doctype[2].structtype[0].field[1].name "stringfield" +doctype[2].structtype[0].field[1].internalid 1182460484 +doctype[2].structtype[0].field[1].type 10005 +doctype[2].structtype[0].field[2].name "longfield" +doctype[2].structtype[0].field[2].internalid 1589309697 +doctype[2].structtype[0].field[2].type 10004 diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/FieldOfTypeDocumentTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/FieldOfTypeDocumentTestCase.java index fdd7fe95c45..bab56c9db2c 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/FieldOfTypeDocumentTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/FieldOfTypeDocumentTestCase.java @@ -31,6 +31,10 @@ public class FieldOfTypeDocumentTestCase extends AbstractSchemaTestCase { assertConfigFile("src/test/examples/fieldoftypedocument.cfg", new DocumentmanagerConfig(value).toString() + "\n"); + value = Deriver.getDocumentManagerConfig(sds, true); + assertConfigFile("src/test/examples/fieldoftypedocument-doctypes.cfg", + new DocumentmanagerConfig(value).toString() + "\n"); + DocumentTypeManager manager = new DocumentTypeManager(); DocumentTypeManagerConfigurer.configure(manager, "raw:" + new DocumentmanagerConfig(value).toString()); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java index 1891a951d01..8b54455d176 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java @@ -28,6 +28,8 @@ public abstract class AbstractExportingTestCase extends AbstractSchemaTestCase { private static final String tempDir = "temp/"; private static final String searchDefRoot = "src/test/derived/"; + boolean useV8DocManagerCfg() { return false; } + private DerivedConfiguration derive(String dirName, String searchDefinitionName, TestProperties properties, @@ -64,7 +66,8 @@ public abstract class AbstractExportingTestCase extends AbstractSchemaTestCase { private DerivedConfiguration export(String name, SchemaBuilder builder, DerivedConfiguration config) throws IOException { String path = exportConfig(name, config); - DerivedConfiguration.exportDocuments(new DocumentManager().produce(builder.getModel(), new DocumentmanagerConfig.Builder()), path); + DerivedConfiguration.exportDocuments(new DocumentManager().useV8DocManagerCfg(useV8DocManagerCfg()) + .produce(builder.getModel(), new DocumentmanagerConfig.Builder()), path); DerivedConfiguration.exportDocuments(new DocumentTypes().produce(builder.getModel(), new DocumenttypesConfig.Builder()), path); DerivedConfiguration.exportQueryProfiles(builder.getQueryProfileRegistry(), path); return config; diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/InheritanceTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/InheritanceTestCase.java index 79df1fc9501..f00072a5a19 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/InheritanceTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/InheritanceTestCase.java @@ -31,6 +31,9 @@ import static org.junit.Assert.assertNull; */ public class InheritanceTestCase extends AbstractExportingTestCase { + @Override + boolean useV8DocManagerCfg() { return true; } + @Rule public TemporaryFolder tmpDir = new TemporaryFolder(); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/StructInheritanceTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/StructInheritanceTestCase.java new file mode 100644 index 00000000000..19bd8305fa5 --- /dev/null +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/StructInheritanceTestCase.java @@ -0,0 +1,62 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition.derived; + +import com.yahoo.document.DataType; +import com.yahoo.document.config.DocumentmanagerConfig; +import com.yahoo.searchdefinition.Index; +import com.yahoo.searchdefinition.Schema; +import com.yahoo.searchdefinition.SchemaBuilder; +import com.yahoo.searchdefinition.document.SDDocumentType; +import com.yahoo.searchdefinition.document.SDField; +import com.yahoo.searchdefinition.parser.ParseException; +import com.yahoo.vespa.configmodel.producers.DocumentManager; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import java.io.File; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; + +import org.junit.rules.TemporaryFolder; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +/** + * Tests struct inheritance + * + * @author arnej + */ +public class StructInheritanceTestCase extends AbstractExportingTestCase { + + @Rule + public TemporaryFolder tmpDir = new TemporaryFolder(); + + @Rule + public final ExpectedException exceptionRule = ExpectedException.none(); + + @Test + public void requireThatStructCanInherit() throws IOException, ParseException { + String dir = "src/test/derived/structinheritance/"; + SchemaBuilder builder = new SchemaBuilder(); + builder.importFile(dir + "simple.sd"); + builder.build(false); + derive("structinheritance", builder, builder.getSchema("simple")); + assertCorrectConfigFiles("structinheritance"); + } + + @Test + public void requireThatRedeclareIsNotAllowed() throws IOException, ParseException { + exceptionRule.expect(IllegalArgumentException.class); + exceptionRule.expectMessage("cannot inherit from base and redeclare field name"); + String dir = "src/test/derived/structinheritance/"; + SchemaBuilder builder = new SchemaBuilder(); + builder.importFile(dir + "bad.sd"); + builder.build(); + derive("structinheritance", builder, builder.getSchema("bad")); + } + +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java index 6d3e83af927..68c623ec9a3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java @@ -802,7 +802,7 @@ public class ContentBuilderTest extends DomBuilderTest { public void ensureFeedSequencerIsControlledByFlag() { verifyFeedSequencer("LATENCY", "LATENCY"); verifyFeedSequencer("ADAPTIVE", "ADAPTIVE"); - verifyFeedSequencer("THROUGHPUT", "LATENCY", 0); + verifyFeedSequencer("THROUGHPUT", "THROUGHPUT", 0); verifyFeedSequencer("THROUGHPUT", "THROUGHPUT", 0.1); verifyFeedSequencer("THOUGHPUT", "LATENCY"); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index eb1a3e32471..56fdae477b2 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.config.proxy; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.config.ConfigurationRuntimeException; import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.impl.JRTConfigRequester; +import com.yahoo.config.subscription.impl.JrtConfigRequesters; import com.yahoo.jrt.Request; import com.yahoo.jrt.Spec; import com.yahoo.jrt.Supervisor; @@ -53,7 +53,7 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { private final ScheduledExecutorService nextConfigScheduler = Executors.newScheduledThreadPool(1, new DaemonThreadFactory("next config")); private final ScheduledFuture<?> nextConfigFuture; - private final JRTConfigRequester requester; + private final JrtConfigRequesters requesters; // Scheduled executor that periodically checks for requests that have timed out and response should be returned to clients private final ScheduledExecutorService delayedResponsesScheduler = Executors.newScheduledThreadPool(1, new DaemonThreadFactory("delayed responses")); @@ -66,7 +66,7 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { this.delayedResponses = new DelayedResponses(); checkConfigSources(); nextConfigFuture = nextConfigScheduler.scheduleAtFixedRate(this, 0, 10, MILLISECONDS); - this.requester = JRTConfigRequester.create(configSourceSet, timingValues); + this.requesters = new JrtConfigRequesters(); DelayedResponseHandler command = new DelayedResponseHandler(delayedResponses, memoryCache, responseHandler); this.delayedResponsesFuture = delayedResponsesScheduler.scheduleAtFixedRate(command, 5, 1, SECONDS); } @@ -145,7 +145,8 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { if (subscribers.containsKey(configCacheKey)) return; log.log(Level.FINE, () -> "Could not find good config in cache, creating subscriber for: " + configCacheKey); - var subscriber = new Subscriber(input, configSourceSet, timingValues, requester); + var subscriber = new Subscriber(input, timingValues, requesters + .getRequester(configSourceSet, timingValues)); try { subscriber.subscribe(); subscribers.put(configCacheKey, subscriber); @@ -197,12 +198,12 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { log.log(Level.FINE, "nextConfigScheduler.shutdownNow"); nextConfigScheduler.shutdownNow(); log.log(Level.FINE, "requester.close"); - requester.close(); + requesters.close(); } @Override public String getActiveSourceConnection() { - return requester.getConnectionPool().getCurrent().getAddress(); + return requesters.getRequester(configSourceSet, timingValues).getConnectionPool().getCurrent().getAddress(); } @Override diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Subscriber.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Subscriber.java index 70ff4456f6c..b407c0e7e76 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Subscriber.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Subscriber.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy; -import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.config.subscription.impl.GenericConfigHandle; import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; @@ -10,7 +9,6 @@ import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; import com.yahoo.yolean.Exceptions; -import java.util.Map; import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; @@ -23,22 +21,20 @@ public class Subscriber { private final static Logger log = Logger.getLogger(Subscriber.class.getName()); private final RawConfig config; - private final ConfigSourceSet configSourceSet; private final TimingValues timingValues; private final GenericConfigSubscriber subscriber; private GenericConfigHandle handle; - Subscriber(RawConfig config, ConfigSourceSet configSourceSet, TimingValues timingValues, JRTConfigRequester requester) { + Subscriber(RawConfig config, TimingValues timingValues, JRTConfigRequester requester) { this.config = config; - this.configSourceSet = configSourceSet; this.timingValues = timingValues; - this.subscriber = new GenericConfigSubscriber(Map.of(configSourceSet, requester)); + this.subscriber = new GenericConfigSubscriber(requester); } void subscribe() { ConfigKey<?> key = config.getKey(); handle = subscriber.subscribe(new ConfigKey<>(key.getName(), key.getConfigId(), key.getNamespace()), - config.getDefContent(), configSourceSet, timingValues); + config.getDefContent(), timingValues); } public Optional<RawConfig> nextGeneration() { @@ -58,14 +54,8 @@ public class Subscriber { return Optional.empty(); } - public void cancel() { - if (subscriber != null) { - subscriber.close(); - } - } + public void cancel() { subscriber.close(); } - boolean isClosed() { - return subscriber.isClosed(); - } + boolean isClosed() { return subscriber.isClosed(); } } diff --git a/config/abi-spec.json b/config/abi-spec.json index fa016fd91da..844835ae1c5 100644 --- a/config/abi-spec.json +++ b/config/abi-spec.json @@ -212,21 +212,18 @@ "public boolean nextGeneration(long)", "protected void throwIfExceptionSet(com.yahoo.config.subscription.impl.ConfigSubscription)", "public void close()", - "protected void closeRequesters()", "public java.lang.String toString()", "public java.lang.Thread startConfigThread(java.lang.Runnable)", "protected com.yahoo.config.subscription.ConfigSubscriber$State state()", "public void reload(long)", "public com.yahoo.config.subscription.ConfigSource getSource()", - "public java.util.Map requesters()", "public boolean isClosed()", "public com.yahoo.config.subscription.ConfigHandle subscribe(com.yahoo.config.subscription.ConfigSubscriber$SingleSubscriber, java.lang.Class, java.lang.String)", "public long getGeneration()", "protected void finalize()" ], "fields": [ - "protected final java.util.List subscriptionHandles", - "protected java.util.Map requesters" + "protected final java.util.List subscriptionHandles" ] }, "com.yahoo.config.subscription.ConfigURI": { diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java index 07132c460f9..01008f0a8a2 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java @@ -5,15 +5,13 @@ import com.yahoo.config.ConfigInstance; import com.yahoo.config.ConfigurationRuntimeException; import com.yahoo.config.subscription.impl.ConfigSubscription; import com.yahoo.config.subscription.impl.JRTConfigRequester; +import com.yahoo.config.subscription.impl.JrtConfigRequesters; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.TimingValues; import com.yahoo.yolean.Exceptions; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; - import java.util.logging.Level; import java.util.logging.Logger; @@ -40,6 +38,7 @@ public class ConfigSubscriber implements AutoCloseable { private final ConfigSource source; private final Object monitor = new Object(); private final Throwable stackTraceAtConstruction; // TODO Remove once finalizer is gone + private final JrtConfigRequesters requesters = new JrtConfigRequesters(); /** The last complete config generation received by this */ private long generation = -1; @@ -52,11 +51,6 @@ public class ConfigSubscriber implements AutoCloseable { private boolean applyOnRestart = false; /** - * Reuse requesters for equal source sets, limit number if many subscriptions. - */ - protected Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>(); - - /** * The states of the subscriber. Affects the validity of calling certain methods. * */ @@ -114,8 +108,8 @@ public class ConfigSubscriber implements AutoCloseable { // for testing <T extends ConfigInstance> ConfigHandle<T> subscribe(Class<T> configClass, String configId, ConfigSource source, TimingValues timingValues) { checkStateBeforeSubscribe(); - final ConfigKey<T> configKey = new ConfigKey<>(configClass, configId); - ConfigSubscription<T> sub = ConfigSubscription.get(configKey, this, source, timingValues); + ConfigKey<T> configKey = new ConfigKey<>(configClass, configId); + ConfigSubscription<T> sub = ConfigSubscription.get(configKey, requesters, source, timingValues); ConfigHandle<T> handle = new ConfigHandle<>(sub); subscribeAndHandleErrors(sub, configKey, handle, timingValues); return handle; @@ -375,19 +369,10 @@ public class ConfigSubscriber implements AutoCloseable { for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) { h.subscription().close(); } - closeRequesters(); + requesters.close(); log.log(FINE, () -> "Config subscriber has been closed."); } - /** - * Closes all open requesters - */ - protected void closeRequesters() { - for (JRTConfigRequester requester : requesters.values()) { - requester.close(); - } - } - @Override public String toString() { StringBuilder sb = new StringBuilder(); @@ -442,14 +427,6 @@ public class ConfigSubscriber implements AutoCloseable { return source; } - /** - * Implementation detail, do not use. - * @return requesters - */ - public Map<ConfigSourceSet, JRTConfigRequester> requesters() { - return requesters; - } - public boolean isClosed() { synchronized (monitor) { return state == State.CLOSED; diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java index 780556e93fa..f8a45a11b70 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java @@ -111,10 +111,9 @@ public abstract class ConfigSubscription<T extends ConfigInstance> { * Correct type of ConfigSubscription instance based on type of source or form of config id * * @param key a {@link ConfigKey} - * @param subscriber the subscriber for this subscription * @return a subclass of a ConfigsSubscription */ - public static <T extends ConfigInstance> ConfigSubscription<T> get(ConfigKey<T> key, ConfigSubscriber subscriber, + public static <T extends ConfigInstance> ConfigSubscription<T> get(ConfigKey<T> key, JrtConfigRequesters requesters, ConfigSource source, TimingValues timingValues) { String configId = key.getConfigId(); if (source instanceof RawSource || configId.startsWith("raw:")) return getRawSub(key, source); @@ -122,7 +121,10 @@ public abstract class ConfigSubscription<T extends ConfigInstance> { if (source instanceof DirSource || configId.startsWith("dir:")) return getDirFileSub(key, source); if (source instanceof JarSource || configId.startsWith("jar:")) return getJarSub(key, source); if (source instanceof ConfigSet) return new ConfigSetSubscription<>(key, source); - if (source instanceof ConfigSourceSet) return new JRTConfigSubscription<>(key, subscriber, (ConfigSourceSet) source, timingValues); + if (source instanceof ConfigSourceSet) { + JRTConfigRequester requester = requesters.getRequester((ConfigSourceSet) source, timingValues); + return new JRTConfigSubscription<>(key, requester, timingValues); + } throw new IllegalArgumentException("Unknown source type: " + source); } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java index 6dc18137639..e382bab576e 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java @@ -3,14 +3,12 @@ package com.yahoo.config.subscription.impl; import com.yahoo.config.ConfigInstance; import com.yahoo.config.subscription.ConfigHandle; -import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; import java.util.List; -import java.util.Map; /** * A subscriber that can subscribe without the class. Used by config proxy. @@ -19,16 +17,18 @@ import java.util.Map; */ public class GenericConfigSubscriber extends ConfigSubscriber { + private final JRTConfigRequester requester; + /** * Constructs a new subscriber using the given pool of requesters (JRTConfigRequester holds 1 connection which in * turn is subject to failover across the elements in the source set.) * The behaviour is undefined if the map key is different from the source set the requester was built with. * See also {@link JRTConfigRequester#JRTConfigRequester(com.yahoo.vespa.config.ConnectionPool, com.yahoo.vespa.config.TimingValues)} * - * @param requesters a map from config source set to config requester + * @param requester a config requester */ - public GenericConfigSubscriber(Map<ConfigSourceSet, JRTConfigRequester> requesters) { - this.requesters = requesters; + public GenericConfigSubscriber(JRTConfigRequester requester) { + this.requester = requester; } /** @@ -36,13 +36,12 @@ public class GenericConfigSubscriber extends ConfigSubscriber { * * @param key the {@link ConfigKey to subscribe to} * @param defContent the config definition content for the config to subscribe to - * @param source the config source to use * @param timingValues {@link TimingValues} * @return generic handle */ - public GenericConfigHandle subscribe(ConfigKey<RawConfig> key, List<String> defContent, ConfigSourceSet source, TimingValues timingValues) { + public GenericConfigHandle subscribe(ConfigKey<RawConfig> key, List<String> defContent, TimingValues timingValues) { checkStateBeforeSubscribe(); - GenericJRTConfigSubscription sub = new GenericJRTConfigSubscription(key, defContent, this, source, timingValues); + GenericJRTConfigSubscription sub = new GenericJRTConfigSubscription(key, defContent, requester, timingValues); GenericConfigHandle handle = new GenericConfigHandle(sub); subscribeAndHandleErrors(sub, key, handle, timingValues); return handle; diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java index 737ca64b075..43f7a1fc168 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java @@ -1,8 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription.impl; -import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; @@ -25,10 +23,9 @@ public class GenericJRTConfigSubscription extends JRTConfigSubscription<RawConfi public GenericJRTConfigSubscription(ConfigKey<RawConfig> key, List<String> defContent, - ConfigSubscriber subscriber, - ConfigSourceSet source, + JRTConfigRequester requester, TimingValues timingValues) { - super(key, subscriber, source, timingValues); + super(key, requester, timingValues); this.defContent = defContent; } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java index b27c75fb61d..0b98e9cd1b2 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java @@ -4,8 +4,6 @@ package com.yahoo.config.subscription.impl; import com.yahoo.config.ConfigInstance; import com.yahoo.config.ConfigurationRuntimeException; import com.yahoo.config.subscription.ConfigInterruptedException; -import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.TimingValues; @@ -31,9 +29,8 @@ import static java.util.logging.Level.INFO; */ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubscription<T> { - private JRTConfigRequester requester; + private final JRTConfigRequester requester; private final TimingValues timingValues; - private final ConfigSubscriber subscriber; // Last time we got an OK JRT callback private Instant lastOK = Instant.MIN; @@ -43,13 +40,11 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc * but has not yet been handled. */ private BlockingQueue<JRTClientConfigRequest> reqQueue = new LinkedBlockingQueue<>(); - private final ConfigSourceSet sources; - public JRTConfigSubscription(ConfigKey<T> key, ConfigSubscriber subscriber, ConfigSourceSet source, TimingValues timingValues) { + public JRTConfigSubscription(ConfigKey<T> key, JRTConfigRequester requester, TimingValues timingValues) { super(key); this.timingValues = timingValues; - this.subscriber = subscriber; - this.sources = source; + this.requester = requester; } @Override @@ -148,7 +143,6 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc @Override public boolean subscribe(long timeout) { lastOK = Instant.now(); - requester = getRequester(); requester.request(this); JRTClientConfigRequest req = reqQueue.peek(); while (req == null && (Instant.now().isBefore(lastOK.plus(Duration.ofMillis(timeout))))) { @@ -162,15 +156,6 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc return req != null; } - private JRTConfigRequester getRequester() { - JRTConfigRequester requester = subscriber.requesters().get(sources); - if (requester == null) { - requester = JRTConfigRequester.create(sources, timingValues); - subscriber.requesters().put(sources, requester); - } - return requester; - } - @Override @SuppressWarnings("serial") public void close() { diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JrtConfigRequesters.java b/config/src/main/java/com/yahoo/config/subscription/impl/JrtConfigRequesters.java new file mode 100644 index 00000000000..1e9612272d5 --- /dev/null +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JrtConfigRequesters.java @@ -0,0 +1,38 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.config.subscription.impl; + +import com.yahoo.config.subscription.ConfigSourceSet; +import com.yahoo.vespa.config.TimingValues; + +import java.util.HashMap; +import java.util.Map; + +/** + * Keeps track of requesters per config subscriber + * + * @author hmusum + */ +public class JrtConfigRequesters { + + /** + * Reuse requesters for equal source sets, limit number if many subscriptions. + */ + protected Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>(); + + public JRTConfigRequester getRequester(ConfigSourceSet source, TimingValues timingValues) { + JRTConfigRequester requester = requesters.get(source); + if (requester == null) { + requester = JRTConfigRequester.create(source, timingValues); + requesters.put(source, requester); + } + return requester; + } + + /** + * Closes all open requesters + */ + public void close() { + requesters.values().forEach(JRTConfigRequester::close); + } + +} diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java index 0d9b8745888..346368ee7d9 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java @@ -2,6 +2,7 @@ package com.yahoo.config.subscription; import com.yahoo.config.subscription.impl.ConfigSubscription; +import com.yahoo.config.subscription.impl.JrtConfigRequesters; import com.yahoo.foo.AppConfig; import com.yahoo.foo.SimpletypesConfig; import com.yahoo.foo.StringConfig; @@ -18,21 +19,21 @@ public class ConfigSetSubscriptionTest { @Test public void testConfigSubscription() { - ConfigSubscriber subscriber = new ConfigSubscriber(); ConfigSet configSet = new ConfigSet(); AppConfig.Builder a0builder = new AppConfig.Builder().message("A message, 0").times(88); configSet.addBuilder("app/0", a0builder); AppConfig.Builder a1builder = new AppConfig.Builder().message("A message, 1").times(89); configSet.addBuilder("app/1", a1builder); + JrtConfigRequesters requesters = new JrtConfigRequesters(); ConfigSubscription<AppConfig> c1 = ConfigSubscription.get( new ConfigKey<>(AppConfig.class, "app/0"), - subscriber, + requesters, configSet, new TimingValues()); ConfigSubscription<AppConfig> c2 = ConfigSubscription.get( new ConfigKey<>(AppConfig.class, "app/1"), - subscriber, + requesters, configSet, new TimingValues()); @@ -42,14 +43,13 @@ public class ConfigSetSubscriptionTest { @Test(expected = IllegalArgumentException.class) public void testUnknownKey() { - ConfigSubscriber subscriber = new ConfigSubscriber(); ConfigSet configSet = new ConfigSet(); AppConfig.Builder a0builder = new AppConfig.Builder().message("A message, 0").times(88); configSet.addBuilder("app/0", a0builder); ConfigSubscription.get( new ConfigKey<>(SimpletypesConfig.class, "simpletypes/1"), - subscriber, + new JrtConfigRequesters(), configSet, new TimingValues()); } diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java index 270c618ee1b..1b0bc858361 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java @@ -4,6 +4,7 @@ package com.yahoo.config.subscription; import com.yahoo.config.ConfigInstance; import com.yahoo.config.ConfigurationRuntimeException; import com.yahoo.config.subscription.impl.ConfigSubscription; +import com.yahoo.config.subscription.impl.JrtConfigRequesters; import com.yahoo.foo.AppConfig; import com.yahoo.foo.SimpletypesConfig; import com.yahoo.vespa.config.ConfigKey; @@ -29,9 +30,10 @@ public class ConfigSubscriptionTest { public void testEquals() { ConfigSubscriber sub = new ConfigSubscriber(); - ConfigSubscription<SimpletypesConfig> a = createSubscription(sub, "test"); - ConfigSubscription<SimpletypesConfig> b = createSubscription(sub, "test"); - ConfigSubscription<SimpletypesConfig> c = createSubscription(sub, "test2"); + JrtConfigRequesters requesters = new JrtConfigRequesters(); + ConfigSubscription<SimpletypesConfig> a = createSubscription(requesters, "test"); + ConfigSubscription<SimpletypesConfig> b = createSubscription(requesters, "test"); + ConfigSubscription<SimpletypesConfig> c = createSubscription(requesters, "test2"); assertEquals(b, a); assertEquals(a, a); assertEquals(b, b); @@ -39,21 +41,21 @@ public class ConfigSubscriptionTest { assertNotEquals(c, a); assertNotEquals(c, b); - ConfigSubscriber subscriber = new ConfigSubscriber(); ConfigSet configSet = new ConfigSet(); AppConfig.Builder a0builder = new AppConfig.Builder().message("A message, 0").times(88); configSet.addBuilder("app/0", a0builder); AppConfig.Builder a1builder = new AppConfig.Builder().message("A message, 1").times(89); configSet.addBuilder("app/1", a1builder); + ConfigSubscription<AppConfig> c1 = ConfigSubscription.get( new ConfigKey<>(AppConfig.class, "app/0"), - subscriber, + requesters, configSet, new TimingValues()); ConfigSubscription<AppConfig> c2 = ConfigSubscription.get( new ConfigKey<>(AppConfig.class, "app/1"), - subscriber, + requesters, configSet, new TimingValues()); @@ -86,9 +88,9 @@ public class ConfigSubscriptionTest { } } - private ConfigSubscription<SimpletypesConfig> createSubscription(ConfigSubscriber sub, String configId) { + private ConfigSubscription<SimpletypesConfig> createSubscription(JrtConfigRequesters requesters, String configId) { return ConfigSubscription.get(new ConfigKey<>(SimpletypesConfig.class, configId), - sub, new RawSource("boolval true"), new TimingValues()); + requesters, new RawSource("boolval true"), new TimingValues()); } private static class TestConfigSubscriber extends ConfigSubscriber { diff --git a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java index 4616630557e..fc922cc3b07 100644 --- a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java @@ -6,15 +6,15 @@ import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.config.subscription.impl.JRTConfigRequesterTest; import com.yahoo.config.subscription.impl.MockConnection; +import com.yahoo.jrt.Supervisor; +import com.yahoo.jrt.Transport; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.JRTConnectionPool; import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.CompressionType; import org.junit.Test; -import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -31,14 +31,11 @@ public class GenericConfigSubscriberTest { @Test public void testSubscribeGeneric() throws InterruptedException { - Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>(); - ConfigSourceSet sourceSet = new ConfigSourceSet("blabla"); - requesters.put(sourceSet, new JRTConfigRequester(new MockConnection(), tv)); - GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters); + JRTConfigRequester requester = new JRTConfigRequester(new MockConnection(), tv); + GenericConfigSubscriber sub = new GenericConfigSubscriber(requester); final List<String> defContent = List.of("myVal int"); GenericConfigHandle handle = sub.subscribe(new ConfigKey<>("simpletypes", "id", "config"), defContent, - sourceSet, tv); assertTrue(sub.nextConfig(false)); assertTrue(handle.isChanged()); @@ -60,23 +57,6 @@ public class GenericConfigSubscriberTest { return handle.getRawConfig().getPayload().withCompression(CompressionType.UNCOMPRESSED).toString(); } - @Test - public void testGenericRequesterPooling() { - ConfigSourceSet source1 = new ConfigSourceSet("tcp/foo:78"); - ConfigSourceSet source2 = new ConfigSourceSet("tcp/bar:79"); - JRTConfigRequester req1 = JRTConfigRequester.create(source1, tv); - JRTConfigRequester req2 = JRTConfigRequester.create(source2, tv); - Map<ConfigSourceSet, JRTConfigRequester> requesters = new LinkedHashMap<>(); - requesters.put(source1, req1); - requesters.put(source2, req2); - GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters); - assertEquals(sub.requesters().get(source1).getConnectionPool().getCurrent().getAddress(), "tcp/foo:78"); - assertEquals(sub.requesters().get(source2).getConnectionPool().getCurrent().getAddress(), "tcp/bar:79"); - for (JRTConfigRequester requester : requesters.values()) { - requester.close(); - } - } - @Test(expected=UnsupportedOperationException.class) public void testOverriddenSubscribeInvalid1() { createSubscriber().subscribe(null, null); @@ -93,9 +73,7 @@ public class GenericConfigSubscriberTest { } private GenericConfigSubscriber createSubscriber() { - return new GenericConfigSubscriber(Map.of( - new ConfigSourceSet("blabla"), - new JRTConfigRequester(new MockConnection(), JRTConfigRequesterTest.getTestTimingValues()))); + return new GenericConfigSubscriber(new JRTConfigRequester(new JRTConnectionPool(new ConfigSourceSet("foo"), new Supervisor(new Transport())), tv)); } } diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java index 15f39f590aa..74af35e39dc 100644 --- a/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/impl/FileConfigSubscriptionTest.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription.impl; -import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.config.subscription.DirSource; import com.yahoo.foo.SimpletypesConfig; import com.yahoo.foo.TestReferenceConfig; @@ -98,8 +97,10 @@ public class FileConfigSubscriptionTest { final String cfgDir = "src/test/resources/configs/foo"; final String cfgId = "dir:" + cfgDir; final ConfigKey<TestReferenceConfig> key = new ConfigKey<>(TestReferenceConfig.class, cfgId); - ConfigSubscriber subscriber = new ConfigSubscriber(); - ConfigSubscription<TestReferenceConfig> sub = ConfigSubscription.get(key, subscriber, new DirSource(new File(cfgDir)), new TimingValues()); + ConfigSubscription<TestReferenceConfig> sub = ConfigSubscription.get(key, + new JrtConfigRequesters(), + new DirSource(new File(cfgDir)), + new TimingValues()); assertTrue(sub.nextConfig(1000)); assertThat(sub.getConfigState().getConfig().configId(), is(cfgId)); } diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java index 62a25fadf25..dca0c2d0018 100644 --- a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java @@ -2,7 +2,6 @@ package com.yahoo.config.subscription.impl; import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.foo.SimpletypesConfig; import com.yahoo.jrt.Request; import com.yahoo.vespa.config.ConfigKey; @@ -51,12 +50,11 @@ public class JRTConfigRequesterTest { @Test public void testFirstRequestAfterSubscribing() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); - JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues); - - final MockConnection connection = new MockConnection(); + TimingValues timingValues = getTestTimingValues(); + MockConnection connection = new MockConnection(); JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); + JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(requester, timingValues); + assertEquals(requester.getConnectionPool(), connection); requester.request(sub); final Request request = connection.getRequest(); @@ -70,25 +68,24 @@ public class JRTConfigRequesterTest { @Test public void testFatalError() { - ConfigSubscriber subscriber = new ConfigSubscriber(); final TimingValues timingValues = getTestTimingValues(); final MockConnection connection = new MockConnection(new ErrorResponseHandler()); JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); - requester.request(createSubscription(subscriber, timingValues)); + requester.request(createSubscription(requester, timingValues)); waitUntilResponse(connection); assertEquals(1, requester.getFailures()); } @Test public void testFatalErrorSubscribed() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); - JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues); + TimingValues timingValues = getTestTimingValues(); + MockConnection connection = new MockConnection(new ErrorResponseHandler()); + JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); + + JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(requester, timingValues); sub.setConfig(1L, false, config(), PayloadChecksums.empty()); - final MockConnection connection = new MockConnection(new ErrorResponseHandler()); - JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); waitUntilResponse(connection); assertEquals(1, requester.getFailures()); @@ -96,25 +93,23 @@ public class JRTConfigRequesterTest { @Test public void testTransientError() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); + TimingValues timingValues = getTestTimingValues(); - final MockConnection connection = new MockConnection(new ErrorResponseHandler(com.yahoo.jrt.ErrorCode.TIMEOUT)); + MockConnection connection = new MockConnection(new ErrorResponseHandler(com.yahoo.jrt.ErrorCode.TIMEOUT)); JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); - requester.request(createSubscription(subscriber, timingValues)); + requester.request(createSubscription(requester, timingValues)); waitUntilResponse(connection); assertEquals(1, requester.getFailures()); } @Test public void testTransientErrorSubscribed() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); - JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues); + TimingValues timingValues = getTestTimingValues(); + MockConnection connection = new MockConnection(new ErrorResponseHandler(com.yahoo.jrt.ErrorCode.TIMEOUT)); + JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); + JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(requester, timingValues); sub.setConfig(1L, false, config(), PayloadChecksums.empty()); - final MockConnection connection = new MockConnection(new ErrorResponseHandler(com.yahoo.jrt.ErrorCode.TIMEOUT)); - JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); waitUntilResponse(connection); assertEquals(1, requester.getFailures()); @@ -122,13 +117,12 @@ public class JRTConfigRequesterTest { @Test public void testUnknownConfigDefinitionError() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); - JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues); + TimingValues timingValues = getTestTimingValues(); + MockConnection connection = new MockConnection(new ErrorResponseHandler(ErrorCode.UNKNOWN_DEFINITION)); + JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); + JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(requester, timingValues); sub.setConfig(1L, false, config(), PayloadChecksums.empty()); - final MockConnection connection = new MockConnection(new ErrorResponseHandler(ErrorCode.UNKNOWN_DEFINITION)); - JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); assertEquals(requester.getConnectionPool(), connection); requester.request(sub); waitUntilResponse(connection); @@ -137,13 +131,12 @@ public class JRTConfigRequesterTest { @Test public void testClosedSubscription() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); - JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues); + TimingValues timingValues = getTestTimingValues(); + MockConnection connection = new MockConnection(new MockConnection.OKResponseHandler()); + JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); + JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(requester, timingValues); sub.close(); - final MockConnection connection = new MockConnection(new MockConnection.OKResponseHandler()); - JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); assertEquals(1, connection.getNumberOfRequests()); // Check that no further request was sent? @@ -157,16 +150,14 @@ public class JRTConfigRequesterTest { @Test public void testTimeout() { - ConfigSubscriber subscriber = new ConfigSubscriber(); - final TimingValues timingValues = getTestTimingValues(); - JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(subscriber, timingValues); + TimingValues timingValues = getTestTimingValues(); + MockConnection connection = new MockConnection(new DelayedResponseHandler(timingValues.getSubscribeTimeout()), + 2); // fake that we have more than one source + JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); + JRTConfigSubscription<SimpletypesConfig> sub = createSubscription(requester, timingValues); sub.close(); - final MockConnection connection = new MockConnection( - new DelayedResponseHandler(timingValues.getSubscribeTimeout()), - 2); // fake that we have more than one source - JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); - requester.request(createSubscription(subscriber, timingValues)); + requester.request(createSubscription(requester, timingValues)); // Check that no further request was sent? try { Thread.sleep(timingValues.getFixedDelay()*2); @@ -175,9 +166,10 @@ public class JRTConfigRequesterTest { } } - private JRTConfigSubscription<SimpletypesConfig> createSubscription(ConfigSubscriber subscriber, TimingValues timingValues) { - return new JRTConfigSubscription<>( - new ConfigKey<>(SimpletypesConfig.class, "testid"), subscriber, null, timingValues); + private JRTConfigSubscription<SimpletypesConfig> createSubscription(JRTConfigRequester requester, TimingValues timingValues) { + return new JRTConfigSubscription<>(new ConfigKey<>(SimpletypesConfig.class, "testid"), + requester, + timingValues); } private SimpletypesConfig config() { diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestFactoryTest.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestFactoryTest.java index 4f7b1df5a43..14183aa087a 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestFactoryTest.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestFactoryTest.java @@ -2,10 +2,11 @@ package com.yahoo.vespa.config.protocol; import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.ConfigSubscriber; +import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.config.subscription.impl.JRTConfigSubscription; import com.yahoo.foo.FunctionTestConfig; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.JRTConnectionPool; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; import org.junit.Test; @@ -42,11 +43,13 @@ public class JRTConfigRequestFactoryTest { @Test public void testCreateFromSub() { - ConfigSubscriber subscriber = new ConfigSubscriber(); Class<FunctionTestConfig> clazz = FunctionTestConfig.class; final String configId = "foo"; - JRTConfigSubscription<FunctionTestConfig> sub = new JRTConfigSubscription<>( - new ConfigKey<>(clazz, configId), subscriber, new ConfigSourceSet(), new TimingValues()); + TimingValues timingValues = new TimingValues(); + JRTConfigSubscription<FunctionTestConfig> sub = + new JRTConfigSubscription<>(new ConfigKey<>(clazz, configId), + new JRTConfigRequester(new JRTConnectionPool(new ConfigSourceSet("tcp/localhost:12345")), timingValues), + timingValues); JRTClientConfigRequest request = JRTConfigRequestFactory.createFromSub(sub); assertThat(request.getVespaVersion().get(), is(defaultVespaVersion)); diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java index 5f2a5c73fa5..dabd87e1eec 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java @@ -2,8 +2,6 @@ package com.yahoo.vespa.config.protocol; import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.ConfigSubscriber; -import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.config.subscription.impl.JRTConfigSubscription; import com.yahoo.config.subscription.impl.MockConnection; @@ -16,6 +14,7 @@ import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.ErrorCode; +import com.yahoo.vespa.config.JRTConnectionPool; import com.yahoo.vespa.config.PayloadChecksums; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; @@ -23,7 +22,6 @@ import com.yahoo.vespa.config.util.ConfigUtils; import org.junit.Before; import org.junit.Test; -import java.util.Collections; import java.util.List; import java.util.Optional; @@ -190,12 +188,11 @@ public class JRTConfigRequestV3Test { @Test public void created_from_subscription() { - ConfigSubscriber subscriber = new ConfigSubscriber(); + TimingValues timingValues = new TimingValues(); JRTConfigSubscription<SimpletypesConfig> sub = new JRTConfigSubscription<>(new ConfigKey<>(SimpletypesConfig.class, configId), - subscriber, - new ConfigSourceSet(), - new TimingValues()); + new JRTConfigRequester(new JRTConnectionPool(new ConfigSourceSet("tcp/localhost:985")), timingValues), + timingValues); JRTClientConfigRequest request = createReq(sub, Trace.createNew(9)); assertThat(request.getConfigKey().getName(), is(SimpletypesConfig.CONFIG_DEF_NAME)); JRTServerConfigRequest serverRequest = createReq(request.getRequest()); @@ -212,9 +209,10 @@ public class JRTConfigRequestV3Test { } }); - ConfigSourceSet src = new ConfigSourceSet(); - ConfigSubscriber subscriber = new GenericConfigSubscriber(Collections.singletonMap(src, new JRTConfigRequester(connection, new TimingValues()))); - JRTConfigSubscription<SimpletypesConfig> sub = new JRTConfigSubscription<>(new ConfigKey<>(SimpletypesConfig.class, configId), subscriber, src, new TimingValues()); + TimingValues timingValues = new TimingValues(); + JRTConfigSubscription<SimpletypesConfig> sub = new JRTConfigSubscription<>(new ConfigKey<>(SimpletypesConfig.class, configId), + new JRTConfigRequester(connection, timingValues), + timingValues); sub.subscribe(120_0000); assertTrue(sub.nextConfig(120_0000)); sub.close(); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index aedbb3afb69..9292e2024df 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -5,7 +5,6 @@ import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.jdisc.Metric.Context; import com.yahoo.jdisc.References; import com.yahoo.jdisc.ResourceReference; -import com.yahoo.jdisc.Response; import com.yahoo.jdisc.handler.BindingNotFoundException; import com.yahoo.jdisc.handler.ContentChannel; import com.yahoo.jdisc.handler.OverloadException; @@ -38,7 +37,6 @@ import java.util.logging.Logger; import static com.yahoo.jdisc.http.HttpHeaders.Values.APPLICATION_X_WWW_FORM_URLENCODED; import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; -import static com.yahoo.yolean.Exceptions.throwUnchecked; /** * @author Simon Thoresen Hult @@ -104,22 +102,6 @@ class HttpRequestDispatch { servletRequestReader.start(); } - ContentChannel dispatchFilterRequest(Response response) { - try { - CompletableFuture<Void> requestCompletion = startServletAsyncExecution(); - jettyRequest.getInputStream().close(); - ContentChannel responseContentChannel = servletResponseController.responseHandler().handleResponse(response); - servletResponseController.finishedFuture() - .whenComplete((r, t) -> { - if (t != null) requestCompletion.completeExceptionally(t); - else requestCompletion.complete(null); - }); - return responseContentChannel; - } catch (IOException e) { - throw throwUnchecked(e); - } - } - private CompletableFuture<Void> startServletAsyncExecution() { CompletableFuture<Void> requestCompletion = new CompletableFuture<>(); AsyncContext asyncCtx = jettyRequest.startAsync(); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java index ffa31a9e8de..e90dde0e4eb 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java @@ -32,27 +32,32 @@ import static com.yahoo.jdisc.http.server.jetty.CompletionHandlerUtils.NOOP_COMP */ class ServletResponseController { + private enum State { + WAITING_FOR_RESPONSE, + ACCEPTED_RESPONSE_FROM_HANDLER, + COMMITTED_RESPONSE_FROM_HANDLER, + COMPLETED_WITH_RESPONSE_FROM_HANDLER, + COMPLETED_WITH_ERROR_RESPONSE + } + private static final Logger log = Logger.getLogger(ServletResponseController.class.getName()); /** - * The servlet spec does not require (Http)ServletResponse nor ServletOutputStream to be thread-safe. Therefore, - * we must provide our own synchronization, since we may attempt to access these objects simultaneously from - * different threads. (The typical cause of this is when one thread is writing a response while another thread - * throws an exception, causing the request to fail with an error response). + * Only a single thread must modify {@link HttpServletRequest}/{@link HttpServletResponse} at a time, + * and it must only be performed when the response is committed. + * The response cannot be modified once response content is being written. */ private final Object monitor = new Object(); - //servletResponse must not be modified after the response has been committed. private final HttpServletRequest servletRequest; private final HttpServletResponse servletResponse; private final boolean developerMode; private final ErrorResponseContentCreator errorResponseContentCreator = new ErrorResponseContentCreator(); - - //all calls to the servletOutputStreamWriter must hold the monitor first to ensure visibility of servletResponse changes. private final ServletOutputStreamWriter out; // GuardedBy("monitor") - private boolean responseCommitted = false; + private State state = State.WAITING_FOR_RESPONSE; + private Response handlerResponse; ServletResponseController( HttpServletRequest servletRequest, @@ -71,7 +76,24 @@ class ServletResponseController { void trySendErrorResponse(Throwable t) { synchronized (monitor) { try { - sendErrorResponseIfUncommitted(t); + switch (state) { + case WAITING_FOR_RESPONSE: + case ACCEPTED_RESPONSE_FROM_HANDLER: + state = State.COMPLETED_WITH_ERROR_RESPONSE; + break; + case COMMITTED_RESPONSE_FROM_HANDLER: + case COMPLETED_WITH_RESPONSE_FROM_HANDLER: + if (log.isLoggable(Level.FINE)) { + RuntimeException exceptionWithStackTrace = new RuntimeException(t); + log.log(Level.FINE, "Response already committed, can't change response code", exceptionWithStackTrace); + } + return; + case COMPLETED_WITH_ERROR_RESPONSE: + return; + default: + throw new IllegalStateException(); + } + writeErrorResponse(t); } catch (Throwable suppressed) { t.addSuppressed(suppressed); } finally { @@ -93,34 +115,28 @@ class ServletResponseController { ResponseHandler responseHandler() { return responseHandler; } - private void sendErrorResponseIfUncommitted(Throwable t) { - if (!responseCommitted) { - responseCommitted = true; - servletResponse.setHeader(HttpHeaders.Names.EXPIRES, null); - servletResponse.setHeader(HttpHeaders.Names.LAST_MODIFIED, null); - servletResponse.setHeader(HttpHeaders.Names.CACHE_CONTROL, null); - servletResponse.setHeader(HttpHeaders.Names.CONTENT_TYPE, null); - servletResponse.setHeader(HttpHeaders.Names.CONTENT_LENGTH, null); - String reasonPhrase = getReasonPhrase(t, developerMode); - int statusCode = getStatusCode(t); - setStatus(servletResponse, statusCode, reasonPhrase); - // If we are allowed to have a body - if (statusCode != HttpServletResponse.SC_NO_CONTENT && - statusCode != HttpServletResponse.SC_NOT_MODIFIED && - statusCode != HttpServletResponse.SC_PARTIAL_CONTENT && - statusCode >= HttpServletResponse.SC_OK) { - servletResponse.setHeader(HttpHeaders.Names.CACHE_CONTROL, "must-revalidate,no-cache,no-store"); - servletResponse.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.toString()); - byte[] errorContent = errorResponseContentCreator - .createErrorContent(servletRequest.getRequestURI(), statusCode, reasonPhrase); - servletResponse.setContentLength(errorContent.length); - out.writeBuffer(ByteBuffer.wrap(errorContent), NOOP_COMPLETION_HANDLER); - } else { - servletResponse.setContentLength(0); - } + private void writeErrorResponse(Throwable t) { + servletResponse.setHeader(HttpHeaders.Names.EXPIRES, null); + servletResponse.setHeader(HttpHeaders.Names.LAST_MODIFIED, null); + servletResponse.setHeader(HttpHeaders.Names.CACHE_CONTROL, null); + servletResponse.setHeader(HttpHeaders.Names.CONTENT_TYPE, null); + servletResponse.setHeader(HttpHeaders.Names.CONTENT_LENGTH, null); + String reasonPhrase = getReasonPhrase(t, developerMode); + int statusCode = getStatusCode(t); + setStatus(servletResponse, statusCode, reasonPhrase); + // If we are allowed to have a body + if (statusCode != HttpServletResponse.SC_NO_CONTENT && + statusCode != HttpServletResponse.SC_NOT_MODIFIED && + statusCode != HttpServletResponse.SC_PARTIAL_CONTENT && + statusCode >= HttpServletResponse.SC_OK) { + servletResponse.setHeader(HttpHeaders.Names.CACHE_CONTROL, "must-revalidate,no-cache,no-store"); + servletResponse.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.toString()); + byte[] errorContent = errorResponseContentCreator + .createErrorContent(servletRequest.getRequestURI(), statusCode, reasonPhrase); + servletResponse.setContentLength(errorContent.length); + out.writeBuffer(ByteBuffer.wrap(errorContent), NOOP_COMPLETION_HANDLER); } else { - RuntimeException exceptionWithStackTrace = new RuntimeException(t); - log.log(Level.FINE, "Response already committed, can't change response code", exceptionWithStackTrace); + servletResponse.setContentLength(0); } } @@ -151,60 +167,79 @@ class ServletResponseController { } } - private void setResponse(Response jdiscResponse) { + private void acceptResponseFromHandler(Response response) { synchronized (monitor) { - servletRequest.setAttribute(HttpResponseStatisticsCollector.requestTypeAttribute, jdiscResponse.getRequestType()); - if (responseCommitted) { - log.log(Level.FINE, - jdiscResponse.getError(), - () -> "Response already committed, can't change response code. " + - "From: " + servletResponse.getStatus() + ", To: " + jdiscResponse.getStatus()); - - //TODO: should throw an exception here, but this breaks unit tests. - //The failures will now instead happen when writing buffers. - out.close(); - return; - } - - if (jdiscResponse instanceof HttpResponse) { - setStatus(servletResponse, jdiscResponse.getStatus(), ((HttpResponse) jdiscResponse).getMessage()); - } else { - String message = Optional.ofNullable(jdiscResponse.getError()) - .flatMap(error -> Optional.ofNullable(error.getMessage())) - .orElse(null); - setStatus(servletResponse, jdiscResponse.getStatus(), message); - } - for (final Map.Entry<String, String> entry : jdiscResponse.headers().entries()) { - servletResponse.addHeader(entry.getKey(), entry.getValue()); - } - if (servletResponse.getContentType() == null) { - servletResponse.setContentType("text/plain;charset=utf-8"); + switch (state) { + case WAITING_FOR_RESPONSE: + case ACCEPTED_RESPONSE_FROM_HANDLER: // Allow multiple invocations to ResponseHandler.handleResponse() + handlerResponse = response; + state = State.ACCEPTED_RESPONSE_FROM_HANDLER; + servletRequest.setAttribute( + HttpResponseStatisticsCollector.requestTypeAttribute, handlerResponse.getRequestType()); + return; + case COMMITTED_RESPONSE_FROM_HANDLER: + case COMPLETED_WITH_RESPONSE_FROM_HANDLER: + String message = "Response already committed, can't change response code. " + + "From: " + servletResponse.getStatus() + ", To: " + response.getStatus(); + log.log(Level.FINE, message, response.getError()); + throw new IllegalStateException(message); + case COMPLETED_WITH_ERROR_RESPONSE: + log.log(Level.FINE, "Error response already written"); + return; // Silently ignore response from handler when request was failed out + default: + throw new IllegalStateException(); } } } - @SuppressWarnings("deprecation") private static void setStatus(HttpServletResponse response, int statusCode, String reasonPhrase) { + org.eclipse.jetty.server.Response jettyResponse = (org.eclipse.jetty.server.Response) response; if (reasonPhrase != null) { - // Sets the status line: a status code along with a custom message. - // Using a custom status message is deprecated in the Servlet API. No alternative exist. - response.setStatus(statusCode, reasonPhrase); // DEPRECATED + jettyResponse.setStatusWithReason(statusCode, reasonPhrase); } else { - response.setStatus(statusCode); + jettyResponse.setStatus(statusCode); } } - private void ensureCommitted() { + private void commitResponseFromHandlerIfUncommitted(boolean close) { synchronized (monitor) { - responseCommitted = true; + switch (state) { + case ACCEPTED_RESPONSE_FROM_HANDLER: + state = close ? State.COMPLETED_WITH_RESPONSE_FROM_HANDLER : State.COMMITTED_RESPONSE_FROM_HANDLER; + break; + case WAITING_FOR_RESPONSE: + throw new IllegalStateException("No response provided"); + case COMMITTED_RESPONSE_FROM_HANDLER: + case COMPLETED_WITH_RESPONSE_FROM_HANDLER: + return; + case COMPLETED_WITH_ERROR_RESPONSE: + log.fine("An error response is already committed - failure will be handled by ServletOutputStreamWriter"); + return; + default: + throw new IllegalStateException(); + } + if (handlerResponse instanceof HttpResponse) { + setStatus(servletResponse, handlerResponse.getStatus(), ((HttpResponse) handlerResponse).getMessage()); + } else { + String message = Optional.ofNullable(handlerResponse.getError()) + .flatMap(error -> Optional.ofNullable(error.getMessage())) + .orElse(null); + setStatus(servletResponse, handlerResponse.getStatus(), message); + } + for (final Map.Entry<String, String> entry : handlerResponse.headers().entries()) { + servletResponse.addHeader(entry.getKey(), entry.getValue()); + } + if (servletResponse.getContentType() == null) { + servletResponse.setContentType("text/plain;charset=utf-8"); + } } } private final ResponseHandler responseHandler = new ResponseHandler() { @Override public ContentChannel handleResponse(Response response) { - setResponse(response); + acceptResponseFromHandler(response); return responseContentChannel; } }; @@ -212,13 +247,13 @@ class ServletResponseController { private final ContentChannel responseContentChannel = new ContentChannel() { @Override public void write(ByteBuffer buf, CompletionHandler handler) { - ensureCommitted(); + commitResponseFromHandlerIfUncommitted(false); out.writeBuffer(buf, handlerOrNoopHandler(handler)); } @Override public void close(CompletionHandler handler) { - ensureCommitted(); + commitResponseFromHandlerIfUncommitted(true); out.close(handlerOrNoopHandler(handler)); } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeMetricsTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeMetricsTest.java index ef3e52304c6..677fb2dbf6d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeMetricsTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeMetricsTest.java @@ -11,6 +11,7 @@ import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; import java.io.IOException; +import java.net.SocketException; import java.nio.file.Path; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -169,7 +170,7 @@ class SslHandshakeMetricsTest { fail("SSLHandshakeException expected"); } catch (SSLHandshakeException e) { assertThat(e.getMessage()).contains(expectedExceptionSubstring); - } catch (SSLException e) { + } catch (SocketException | SSLException e) { // This exception is thrown if Apache httpclient's write thread detects the handshake failure before the read thread. log.log(Level.WARNING, "Client failed to get a proper TLS handshake response: " + e.getMessage(), e); // Only ignore a subset of exceptions diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index f1e5f4ebd9d..a739a8e2b01 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -25,7 +25,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -73,7 +72,7 @@ public class SystemFlagsDataArchive { if (!entry.isDirectory() && name.startsWith("flags/")) { Path filePath = Paths.get(name); String rawData = new String(zipIn.readAllBytes(), StandardCharsets.UTF_8); - addFile(builder, rawData, filePath, Set.of()); + addFile(builder, rawData, filePath, Set.of(), null); } } return builder.build(); @@ -102,7 +101,7 @@ public class SystemFlagsDataArchive { if (!Files.isDirectory(absolutePath) && relativePath.startsWith("flags")) { String rawData = uncheck(() -> Files.readString(absolutePath, StandardCharsets.UTF_8)); - addFile(builder, rawData, relativePath, filenamesForSystem); + addFile(builder, rawData, relativePath, filenamesForSystem, systemDefinition); } }); return builder.build(); @@ -169,12 +168,17 @@ public class SystemFlagsDataArchive { .collect(Collectors.toSet()); } - private static void addFile(Builder builder, String rawData, Path filePath, Set<String> filenamesForSystem) { + private static void addFile(Builder builder, String rawData, Path filePath, Set<String> filenamesForSystem, + ZoneRegistry systemDefinition) { String filename = filePath.getFileName().toString(); if (filename.startsWith(".")) { return; // Ignore files starting with '.' } if (!filenamesForSystem.isEmpty() && !filenamesForSystem.contains(filename)) { + if (systemDefinition != null && filename.startsWith(systemDefinition.system().value() + '.')) { + throw new IllegalArgumentException(String.format( + "Environment or zone in filename '%s' is does not exist", filename)); + } return; // Ignore files irrelevant for system } if (!filename.endsWith(".json")) { diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index 6564ddef81f..d1df9b095d5 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -130,6 +130,15 @@ public class SystemFlagsDataArchiveTest { } @Test + public void throws_exception_on_unknown_region() { + Path directory = Paths.get("src/test/resources/system-flags-with-unknown-file-name/"); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage( + "Environment or zone in filename 'main.prod.unknown-region.json' is does not exist"); + SystemFlagsDataArchive.fromDirectoryAndSystem(directory, createZoneRegistryMock()); + } + + @Test public void throws_on_unknown_field() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage( 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 9acffe58cc1..0a9c810a8f8 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -282,7 +282,7 @@ public class Flags { public static final UnboundBooleanFlag DELETE_UNMAINTAINED_CERTIFICATES = defineFeatureFlag( "delete-unmaintained-certificates", false, - List.of("andreer"), "2021-09-23", "2021-12-11", + List.of("andreer"), "2021-09-23", "2021-12-21", "Whether to delete certificates that are known by provider but not by controller", "Takes effect on next run of EndpointCertificateMaintainer" ); diff --git a/linguistics/src/test/java/com/yahoo/language/opennlp/OptimaizeDetectorTestCase.java b/linguistics/src/test/java/com/yahoo/language/opennlp/OptimaizeDetectorTestCase.java index 3fc173dd82e..20b5de3b165 100644 --- a/linguistics/src/test/java/com/yahoo/language/opennlp/OptimaizeDetectorTestCase.java +++ b/linguistics/src/test/java/com/yahoo/language/opennlp/OptimaizeDetectorTestCase.java @@ -3,7 +3,6 @@ package com.yahoo.language.opennlp; import com.yahoo.language.Language; import com.yahoo.language.detect.Detector; -import com.yahoo.language.simple.SimpleDetector; import org.junit.Test; import static org.junit.Assert.assertEquals; diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java index 7c6cae660a7..457e27a5896 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java @@ -4,9 +4,11 @@ package ai.vespa.metricsproxy.core; import ai.vespa.metricsproxy.core.ConsumersConfig.Consumer; import ai.vespa.metricsproxy.metric.model.ConsumerId; +import ai.vespa.metricsproxy.metric.model.MetricId; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -33,11 +35,20 @@ public class MetricsConsumers { // All consumers for each metric (more useful than the opposite map). private final Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric; + // All consumers for each metric, by metric id + private final Map<MetricId, Map<ConfiguredMetric, Set<ConsumerId>>> consumersByMetricByMetricId; + public MetricsConsumers(ConsumersConfig config) { consumerMetrics = config.consumer().stream().collect( toUnmodifiableLinkedMap(consumer -> ConsumerId.toConsumerId(consumer.name()), consumer -> convert(consumer.metric()))); consumersByMetric = createConsumersByMetric(consumerMetrics); + consumersByMetricByMetricId = new HashMap<>(); + consumersByMetric.forEach((configuredMetric, consumers) -> { + var consumersByMetric = consumersByMetricByMetricId.computeIfAbsent(configuredMetric.id(), id -> new HashMap<>()); + var consumerSet = consumersByMetric.computeIfAbsent(configuredMetric, id -> new HashSet<>()); + consumerSet.addAll(consumers); + }); } /** @@ -52,6 +63,10 @@ public class MetricsConsumers { return consumersByMetric; } + public Map<ConfiguredMetric, Set<ConsumerId>> getConsumersByMetric(MetricId id) { + return consumersByMetricByMetricId.get(id); + } + public Set<ConsumerId> getAllConsumers() { return unmodifiableSet(consumerMetrics.keySet()); } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java index 44eca2f57b4..3629e81582a 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_SERVICE_ID; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; @@ -69,15 +68,13 @@ public class VespaMetrics { public List<MetricsPacket.Builder> getMetrics(List<VespaService> services) { List<MetricsPacket.Builder> metricsPackets = new ArrayList<>(); - Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric = metricsConsumers.getConsumersByMetric(); - for (VespaService service : services) { // One metrics packet for system metrics Optional<MetricsPacket.Builder> systemCheck = getSystemMetrics(service); systemCheck.ifPresent(metricsPackets::add); MetricAggregator aggregator = new MetricAggregator(service.getDimensions()); - GetServiceMetricsConsumer metricsConsumer = new GetServiceMetricsConsumer(consumersByMetric, aggregator); + GetServiceMetricsConsumer metricsConsumer = new GetServiceMetricsConsumer(metricsConsumers, aggregator); service.consumeMetrics(metricsConsumer); if (! aggregator.getAggregated().isEmpty()) { @@ -118,58 +115,50 @@ public class VespaMetrics { * In order to include a metric, it must exist in the given map of metric to consumers. * Each returned metric will contain a collection of consumers that it should be routed to. */ - private class GetServiceMetricsConsumer implements MetricsParser.Consumer { + private static class GetServiceMetricsConsumer implements MetricsParser.Consumer { private final MetricAggregator aggregator; - private final Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric; - GetServiceMetricsConsumer(Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric, MetricAggregator aggregator) { - this.consumersByMetric = consumersByMetric; + private final MetricsConsumers metricsConsumers; + GetServiceMetricsConsumer(MetricsConsumers metricsConsumers, MetricAggregator aggregator) { + this.metricsConsumers = metricsConsumers; this.aggregator = aggregator; } @Override public void consume(Metric candidate) { - getConfiguredMetrics(candidate.getName(), consumersByMetric.keySet()).forEach( - configuredMetric -> aggregator.aggregate( - metricWithConfigProperties(candidate, configuredMetric, consumersByMetric))); + Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric = metricsConsumers.getConsumersByMetric(candidate.getName()); + if (consumersByMetric != null) { + consumersByMetric.keySet().forEach( + configuredMetric -> aggregator.aggregate( + metricWithConfigProperties(candidate, configuredMetric, consumersByMetric.get(configuredMetric)))); + } } - } - - private Map<DimensionId, String> extractDimensions(Map<DimensionId, String> dimensions, List<Dimension> configuredDimensions) { - if ( ! configuredDimensions.isEmpty()) { - Map<DimensionId, String> dims = new HashMap<>(dimensions); - configuredDimensions.forEach(d -> dims.put(d.key(), d.value())); - dimensions = Collections.unmodifiableMap(dims); + private static Metric metricWithConfigProperties(Metric candidate, + ConfiguredMetric configuredMetric, + Set<ConsumerId> consumers) { + Metric metric = candidate.clone(); + metric.setDimensions(extractDimensions(candidate.getDimensions(), configuredMetric.dimension())); + metric.setConsumers(extractConsumers(consumers)); + + if (configuredMetric.outputname() != null && !configuredMetric.outputname().id.isEmpty()) + metric.setName(configuredMetric.outputname()); + return metric; } - return dimensions; - } - - private Set<ConsumerId> extractConsumers(Set<ConsumerId> configuredConsumers) { - Set<ConsumerId> consumers = Collections.emptySet(); - if (configuredConsumers != null) { - consumers = configuredConsumers; + private static Map<DimensionId, String> extractDimensions(Map<DimensionId, String> dimensions, List<Dimension> configuredDimensions) { + if ( ! configuredDimensions.isEmpty()) { + Map<DimensionId, String> dims = new HashMap<>(dimensions); + configuredDimensions.forEach(d -> dims.put(d.key(), d.value())); + dimensions = Collections.unmodifiableMap(dims); + } + return dimensions; } - return consumers; - } - private Metric metricWithConfigProperties(Metric candidate, - ConfiguredMetric configuredMetric, - Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric) { - Metric metric = candidate.clone(); - metric.setDimensions(extractDimensions(candidate.getDimensions(), configuredMetric.dimension())); - metric.setConsumers(extractConsumers(consumersByMetric.get(configuredMetric))); - - if (configuredMetric.outputname() != null && !configuredMetric.outputname().id.isEmpty()) - metric.setName(configuredMetric.outputname()); - return metric; - } - - /** - * Returns all configured metrics (for any consumer) that have the given id as 'name'. - */ - private static Set<ConfiguredMetric> getConfiguredMetrics(MetricId id, Set<ConfiguredMetric> configuredMetrics) { - return configuredMetrics.stream() - .filter(m -> m.id().equals(id)) - .collect(Collectors.toSet()); + private static Set<ConsumerId> extractConsumers(Set<ConsumerId> configuredConsumers) { + Set<ConsumerId> consumers = Collections.emptySet(); + if (configuredConsumers != null) { + consumers = configuredConsumers; + } + return consumers; + } } private Optional<MetricsPacket.Builder> getSystemMetrics(VespaService service) { diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java index 079633b28a1..8157ecb72fd 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java @@ -145,6 +145,7 @@ public class MetricsParser { } JsonNode aggregates = metric.get("values"); + String prefix = name + "."; for (Iterator<?> it = aggregates.fieldNames(); it.hasNext(); ) { String aggregator = (String) it.next(); JsonNode aggregatorValue = aggregates.get(aggregator); @@ -155,7 +156,7 @@ public class MetricsParser { if (value == null) { throw new IllegalArgumentException("Value for aggregator '" + aggregator + "' is not a number"); } - String metricName = new StringBuilder().append(name).append(".").append(aggregator).toString(); + String metricName = prefix + aggregator; consumer.consume(new Metric(MetricId.toMetricId(metricName), value, timestamp, dim, description)); } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java index 49668a59d63..c548d187569 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java @@ -163,9 +163,9 @@ public class SystemPoller { List<VespaService> services, Map<VespaService, Long> lastCpuJiffiesMetrics) { JiffiesAndCpus sysJiffies = getJiffies.getTotalSystemJiffies(); JiffiesAndCpus sysJiffiesDiff = sysJiffies.diff(prevTotalJiffies); + log.log(Level.FINE, () -> "Total jiffies: " + sysJiffies.jiffies + " - " + prevTotalJiffies.jiffies + " = " + sysJiffiesDiff.jiffies); for (VespaService s : services) { Metrics metrics = new Metrics(); - log.log(Level.FINE, () -> "Current size of system metrics for service " + s + " is " + metrics.size()); long[] size = getMemoryUsage(s); log.log(Level.FINE, () -> "Updating memory metric for service " + s); @@ -177,12 +177,14 @@ public class SystemPoller { long last = lastCpuJiffiesMetrics.get(s); long diff = procJiffies - last; + log.log(Level.FINE, () -> "Service " + s + " jiffies: " + procJiffies + " - " + last + " = " + diff); if (diff >= 0) { metrics.add(new Metric(CPU, 100 * sysJiffiesDiff.ratioSingleCoreJiffies(diff), timeStamp)); metrics.add(new Metric(CPU_UTIL, 100 * sysJiffiesDiff.ratioJiffies(diff), timeStamp)); } lastCpuJiffiesMetrics.put(s, procJiffies); s.setSystemMetrics(metrics); + log.log(Level.FINE, () -> "Current size of system metrics for service " + s + " is " + metrics.size()); } return sysJiffies; } diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index 87ee83945cb..30f0081a8f0 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -144,7 +144,7 @@ indexing.semiunboundtasklimit int default = 1000 indexing.kind_of_watermark int default = 0 restart ## Controls minimum reaction time in seconds if using THROUGHPUT -indexing.reactiontime double default = 0.002 restart +indexing.reactiontime double default = 0.001 restart ## How long a freshly loaded index shall be warmed up diff --git a/storage/src/tests/distributor/getoperationtest.cpp b/storage/src/tests/distributor/getoperationtest.cpp index 9fecb005659..dfe4f09de3f 100644 --- a/storage/src/tests/distributor/getoperationtest.cpp +++ b/storage/src/tests/distributor/getoperationtest.cpp @@ -267,26 +267,6 @@ TEST_F(GetOperationTest, send_to_all_invalid_nodes_when_inconsistent) { EXPECT_EQ("newauthor", getLastReplyAuthor()); } -// GetOperation document-level consistency checks are used by the multi-phase update -// logic to see if we can fall back to a fast path even though not all replicas are in sync. -// Empty replicas are not considered part of the send-set, so only looking at replies from -// replicas _sent_ to will not detect this case. -// If we haphazardly treat an empty replicas as implicitly being in sync we risk triggering -// undetectable inconsistencies at the document level. This can happen if we send create-if-missing -// updates to an empty replica as well as a non-empty replica, and the document exists in the -// latter replica. The document would then be implicitly created on the empty replica with the -// same timestamp as that of the non-empty one, even though their contents would almost -// certainly differ. -TEST_F(GetOperationTest, get_not_sent_to_empty_replicas_but_bucket_tagged_as_inconsistent) { - setClusterState("distributor:1 storage:4"); - addNodesToBucketDB(bucketId, "2=0/0/0,3=1/2/3"); - sendGet(); - ASSERT_EQ("Get => 3", _sender.getCommands(true)); - ASSERT_NO_FATAL_FAILURE(sendReply(0, api::ReturnCode::OK, "newauthor", 2)); - EXPECT_FALSE(op->any_replicas_failed()); - EXPECT_FALSE(last_reply_had_consistent_replicas()); -} - TEST_F(GetOperationTest, inconsistent_split) { setClusterState("distributor:1 storage:4"); diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp index 868de8d0ae2..06872cadde6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp @@ -267,11 +267,6 @@ GetOperation::assignTargetNodeGroups(const BucketDatabase::ReadGuard& read_guard _responses[GroupId(e.getBucketId(), copy.getChecksum(), copy.getNode())].emplace_back(copy); } else if (!copy.empty()) { _responses[GroupId(e.getBucketId(), copy.getChecksum(), -1)].emplace_back(copy); - } else { // empty replica - // We must treat a bucket with empty replicas as inherently inconsistent. - // See GetOperationTest::get_not_sent_to_empty_replicas_but_bucket_tagged_as_inconsistent for - // rationale as to why this is the case. - _has_replica_inconsistency = true; } } } diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp index 5233e5678fa..a16eef0ab6f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.cpp @@ -90,7 +90,6 @@ UpdateOperation::onStart(DistributorStripeMessageSender& sender) // An UpdateOperation should only be started iff all replicas are consistent // with each other, so sampling a single replica should be equal to sampling them all. - // FIXME this no longer holds when replicas are consistent at the _document_ level but not at the _bucket_ level. assert(_entries[0].getBucketInfo().getNodeCount() > 0); // Empty buckets are not allowed _infoAtSendTime = _entries[0].getBucketInfo().getNodeRef(0).getBucketInfo(); diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java index 6dc9ec4efb1..1874bd42e16 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java @@ -23,6 +23,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static java.nio.charset.StandardCharsets.UTF_8; @@ -40,9 +44,11 @@ class ApacheCluster implements Cluster { private final RequestConfig defaultConfig = RequestConfig.custom() .setConnectTimeout(Timeout.ofSeconds(10)) .setConnectionRequestTimeout(Timeout.DISABLED) - .setResponseTimeout(Timeout.ofMinutes(5)) + .setResponseTimeout(Timeout.ofSeconds(190)) .build(); + private final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(t -> new Thread(t, "request-timeout-thread")); + ApacheCluster(FeedClientBuilderImpl builder) throws IOException { for (URI endpoint : builder.endpoints) for (int i = 0; i < builder.connectionsPerEndpoint; i++) @@ -59,6 +65,7 @@ class ApacheCluster implements Cluster { min = endpoints.get(i).inflight.get(); } Endpoint endpoint = endpoints.get(index); + endpoint.inflight.incrementAndGet(); try { SimpleHttpRequest request = new SimpleHttpRequest(wrapped.method(), wrapped.path()); @@ -70,13 +77,15 @@ class ApacheCluster implements Cluster { if (wrapped.body() != null) request.setBody(wrapped.body(), ContentType.APPLICATION_JSON); - endpoint.inflight.incrementAndGet(); - endpoint.client.execute(request, - new FutureCallback<SimpleHttpResponse>() { - @Override public void completed(SimpleHttpResponse response) { vessel.complete(new ApacheHttpResponse(response)); } - @Override public void failed(Exception ex) { vessel.completeExceptionally(ex); } - @Override public void cancelled() { vessel.cancel(false); } - }); + Future<?> future = endpoint.client.execute(request, + new FutureCallback<SimpleHttpResponse>() { + @Override public void completed(SimpleHttpResponse response) { vessel.complete(new ApacheHttpResponse(response)); } + @Override public void failed(Exception ex) { vessel.completeExceptionally(ex); } + @Override public void cancelled() { vessel.cancel(false); } + }); + long timeoutMillis = wrapped.timeout() == null ? 200_000 : wrapped.timeout().toMillis() * 11 / 10 + 1_000; + Future<?> cancellation = executor.schedule(() -> { future.cancel(true); vessel.cancel(true); }, timeoutMillis, TimeUnit.MILLISECONDS); + vessel.whenComplete((__, ___) -> cancellation.cancel(true)); } catch (Throwable thrown) { vessel.completeExceptionally(thrown); @@ -87,7 +96,7 @@ class ApacheCluster implements Cluster { @Override public void close() { Throwable thrown = null; - for (Endpoint endpoint : endpoints) + for (Endpoint endpoint : endpoints) { try { endpoint.client.close(); } @@ -95,6 +104,8 @@ class ApacheCluster implements Cluster { if (thrown == null) thrown = t; else thrown.addSuppressed(t); } + } + executor.shutdownNow().forEach(Runnable::run); if (thrown != null) throw new RuntimeException(thrown); } diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java index 3fd44596d63..c136d697a0b 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java @@ -92,7 +92,8 @@ class HttpFeedClient implements FeedClient { HttpRequest request = new HttpRequest(method, getPath(documentId) + getQuery(params), requestHeaders, - operationJson == null ? null : operationJson.getBytes(UTF_8)); // TODO: make it bytes all the way? + operationJson == null ? null : operationJson.getBytes(UTF_8), // TODO: make it bytes all the way? + params.timeout().orElse(null)); CompletableFuture<Result> promise = new CompletableFuture<>(); requestStrategy.enqueue(documentId, request) diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequest.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequest.java index 08b8ca08c61..0ad7b82347e 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequest.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequest.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.feed.client.impl; +import java.time.Duration; import java.util.Map; import java.util.function.Supplier; @@ -10,12 +11,14 @@ class HttpRequest { private final String path; private final Map<String, Supplier<String>> headers; private final byte[] body; + private final Duration timeout; - public HttpRequest(String method, String path, Map<String, Supplier<String>> headers, byte[] body) { + public HttpRequest(String method, String path, Map<String, Supplier<String>> headers, byte[] body, Duration timeout) { this.method = method; this.path = path; this.headers = headers; this.body = body; + this.timeout = timeout; } public String method() { @@ -34,6 +37,10 @@ class HttpRequest { return body; } + public Duration timeout() { + return timeout; + } + @Override public String toString() { return method + " " + path; diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java index d293abf4f3e..d7be4ead078 100644 --- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpRequestStrategyTest.java @@ -41,7 +41,7 @@ class HttpRequestStrategyTest { @Test void testConcurrency() { int documents = 1 << 16; - HttpRequest request = new HttpRequest("PUT", "/", null, null); + HttpRequest request = new HttpRequest("PUT", "/", null, null, null); HttpResponse response = HttpResponse.of(200, "{}".getBytes(UTF_8)); ScheduledExecutorService executor = Executors.newScheduledThreadPool(1); Cluster cluster = new BenchmarkingCluster((__, vessel) -> executor.schedule(() -> vessel.complete(response), (int) (Math.random() * 2 * 10), TimeUnit.MILLISECONDS)); @@ -99,7 +99,7 @@ class HttpRequestStrategyTest { DocumentId id1 = DocumentId.of("ns", "type", "1"); DocumentId id2 = DocumentId.of("ns", "type", "2"); - HttpRequest request = new HttpRequest("POST", "/", null, null); + HttpRequest request = new HttpRequest("POST", "/", null, null, null); // Runtime exception is not retried. cluster.expect((__, vessel) -> vessel.completeExceptionally(new RuntimeException("boom"))); @@ -140,8 +140,8 @@ class HttpRequestStrategyTest { else vessel.complete(success); }); CompletableFuture<HttpResponse> delayed = strategy.enqueue(id1, request); - CompletableFuture<HttpResponse> serialised = strategy.enqueue(id1, new HttpRequest("PUT", "/", null, null)); - assertEquals(success, strategy.enqueue(id2, new HttpRequest("DELETE", "/", null, null)).get()); + CompletableFuture<HttpResponse> serialised = strategy.enqueue(id1, new HttpRequest("PUT", "/", null, null, null)); + assertEquals(success, strategy.enqueue(id2, new HttpRequest("DELETE", "/", null, null, null)).get()); latch.await(); assertEquals(8, strategy.stats().requests()); // 3 attempts at throttled and one at id2. now.set(4000); @@ -159,7 +159,7 @@ class HttpRequestStrategyTest { // Error responses are not retried when not of appropriate type. cluster.expect((__, vessel) -> vessel.complete(serverError)); - assertEquals(serverError, strategy.enqueue(id1, new HttpRequest("PUT", "/", null, null)).get()); + assertEquals(serverError, strategy.enqueue(id1, new HttpRequest("PUT", "/", null, null, null)).get()); assertEquals(12, strategy.stats().requests()); // Some error responses are not retried. @@ -205,9 +205,9 @@ class HttpRequestStrategyTest { DocumentId id2 = DocumentId.of("ns", "type", "2"); DocumentId id3 = DocumentId.of("ns", "type", "3"); DocumentId id4 = DocumentId.of("ns", "type", "4"); - HttpRequest failing = new HttpRequest("POST", "/", null, null); - HttpRequest request = new HttpRequest("POST", "/", null, null); - HttpRequest blocking = new HttpRequest("POST", "/", null, null); + HttpRequest failing = new HttpRequest("POST", "/", null, null, null); + HttpRequest request = new HttpRequest("POST", "/", null, null, null); + HttpRequest blocking = new HttpRequest("POST", "/", null, null, null); // Enqueue some operations to the same id, which are serialised, and then shut down while operations are in flight. Phaser phaser = new Phaser(2); diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt index d4c7bc881a5..400c1ec5d1a 100644 --- a/vespalib/CMakeLists.txt +++ b/vespalib/CMakeLists.txt @@ -42,6 +42,7 @@ vespa_define_module( src/tests/datastore/array_store src/tests/datastore/array_store_config src/tests/datastore/buffer_type + src/tests/datastore/compact_buffer_candidates src/tests/datastore/datastore src/tests/datastore/fixed_size_hash_map src/tests/datastore/sharded_hash_map diff --git a/vespalib/src/tests/datastore/compact_buffer_candidates/CMakeLists.txt b/vespalib/src/tests/datastore/compact_buffer_candidates/CMakeLists.txt new file mode 100644 index 00000000000..d6731071927 --- /dev/null +++ b/vespalib/src/tests/datastore/compact_buffer_candidates/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(vespalib_compact_buffer_candidates_test_app TEST + SOURCES + compact_buffer_candidates_test.cpp + DEPENDS + vespalib + GTest::GTest +) +vespa_add_test(NAME vespalib_compact_buffer_candidates_test_app COMMAND vespalib_compact_buffer_candidates_test_app) diff --git a/vespalib/src/tests/datastore/compact_buffer_candidates/compact_buffer_candidates_test.cpp b/vespalib/src/tests/datastore/compact_buffer_candidates/compact_buffer_candidates_test.cpp new file mode 100644 index 00000000000..80c0d571894 --- /dev/null +++ b/vespalib/src/tests/datastore/compact_buffer_candidates/compact_buffer_candidates_test.cpp @@ -0,0 +1,91 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/datastore/compact_buffer_candidates.h> +#include <vespa/vespalib/gtest/gtest.h> + +using vespalib::datastore::CompactBufferCandidates; + +namespace { + +constexpr uint32_t num_buffers = 1024; +constexpr double default_ratio = 0.2 / 2; +constexpr size_t default_slack = 1000; + +}; + + +class CompactBufferCandidatesTest : public ::testing::Test +{ +public: + CompactBufferCandidates candidates; + CompactBufferCandidatesTest(); + ~CompactBufferCandidatesTest() override; + void reset_candidates(uint32_t max_buffers); + CompactBufferCandidatesTest& add(uint32_t buffer_id, size_t used, size_t dead); + void assert_select(const std::vector<uint32_t>& exp); +}; + +CompactBufferCandidatesTest::CompactBufferCandidatesTest() + : ::testing::Test(), + candidates(num_buffers, 1, default_ratio, default_slack) +{ +} + +CompactBufferCandidatesTest::~CompactBufferCandidatesTest() = default; + +void +CompactBufferCandidatesTest::reset_candidates(uint32_t max_buffers) +{ + candidates = CompactBufferCandidates(num_buffers, max_buffers, default_ratio, default_slack); +} + +CompactBufferCandidatesTest& +CompactBufferCandidatesTest::add(uint32_t buffer_id, size_t used, size_t dead) +{ + candidates.add(buffer_id, used, dead); + return *this; +} + +void +CompactBufferCandidatesTest::assert_select(const std::vector<uint32_t>& exp) +{ + std::vector<uint32_t> act; + candidates.select(act); + EXPECT_EQ(exp, act); +} + +TEST_F(CompactBufferCandidatesTest, select_single) +{ + add(0, 10000, 2000).add(1, 10000, 3000); + assert_select({1}); +} + +TEST_F(CompactBufferCandidatesTest, select_two) +{ + reset_candidates(2); + add(0, 10000, 2000).add(3, 10000, 3000).add(7, 10000, 4000); + assert_select({7, 3}); +} + +TEST_F(CompactBufferCandidatesTest, select_all) +{ + reset_candidates(4); + add(1, 10000, 2000).add(3, 10000, 4000).add(8, 10000, 3000); + assert_select({3, 8, 1}); +} + +TEST_F(CompactBufferCandidatesTest, select_cutoff_by_ratio) +{ + reset_candidates(4); + add(1, 100000, 9999).add(3, 100000, 40000).add(8, 100000, 30000); + assert_select({3, 8}); +} + +TEST_F(CompactBufferCandidatesTest, select_cutoff_by_slack) +{ + reset_candidates(4); + add(1, 2000, 999).add(3, 2000, 1200).add(9, 2000, 1300); + assert_select({9, 3}); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt b/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt index c36077e4dd0..d628843279d 100644 --- a/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt +++ b/vespalib/src/vespa/vespalib/datastore/CMakeLists.txt @@ -6,6 +6,7 @@ vespa_add_library(vespalib_vespalib_datastore OBJECT buffer_type.cpp bufferstate.cpp compaction_strategy.cpp + compact_buffer_candidates.cpp datastore.cpp datastorebase.cpp entryref.cpp diff --git a/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidate.h b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidate.h new file mode 100644 index 00000000000..85ea1e42eac --- /dev/null +++ b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidate.h @@ -0,0 +1,36 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <cstddef> +#include <cstdint> + +namespace vespalib::datastore { + +/* + * Class representing candidate buffer for compaction. + */ +class CompactBufferCandidate { + uint32_t _buffer_id; + size_t _used; + size_t _dead; +public: + CompactBufferCandidate(uint32_t buffer_id, size_t used, size_t dead) noexcept + : _buffer_id(buffer_id), + _used(used), + _dead(dead) + { + } + + CompactBufferCandidate() noexcept + : CompactBufferCandidate(0, 0, 0) + { + } + + bool operator<(const CompactBufferCandidate& rhs) const noexcept { return _dead > rhs._dead; } + uint32_t get_buffer_id() const noexcept { return _buffer_id; } + size_t get_used() const noexcept { return _used; } + size_t get_dead() const noexcept { return _dead; } +}; + +} diff --git a/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp new file mode 100644 index 00000000000..3003ef315e8 --- /dev/null +++ b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp @@ -0,0 +1,52 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "compact_buffer_candidates.h" +#include <algorithm> + +namespace vespalib::datastore { + +CompactBufferCandidates::CompactBufferCandidates(uint32_t num_buffers, uint32_t max_buffers, double ratio, size_t slack) + : _candidates(), + _used(0), + _dead(0), + _max_buffers(std::max(max_buffers, 1u)), + _ratio(ratio), + _slack(slack) +{ + _candidates.reserve(num_buffers); +} + +CompactBufferCandidates::~CompactBufferCandidates() = default; + +void +CompactBufferCandidates::add(uint32_t buffer_id, size_t used, size_t dead) +{ + _candidates.emplace_back(buffer_id, used, dead); + _used += used; + _dead += dead; +} + +void +CompactBufferCandidates::select(std::vector<uint32_t>& buffers) +{ + if (_candidates.empty()) { + return; + } + if (_candidates.size() > _max_buffers) { + std::nth_element(_candidates.begin(), _candidates.begin() + (_max_buffers - 1), _candidates.end()); + _candidates.resize(_max_buffers); + } + std::sort(_candidates.begin(), _candidates.end()); + size_t remaining_used = _used; + size_t remaining_dead = _dead; + for (auto& candidate : _candidates) { + buffers.emplace_back(candidate.get_buffer_id()); + remaining_used -= candidate.get_used(); + remaining_dead -= candidate.get_dead(); + if ((remaining_dead < _slack) || (remaining_dead <= remaining_used * _ratio)) { + break; + } + } +} + +} diff --git a/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.h b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.h new file mode 100644 index 00000000000..59d35422328 --- /dev/null +++ b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.h @@ -0,0 +1,27 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "compact_buffer_candidate.h" +#include <vector> + +namespace vespalib::datastore { + +/* + * Class representing candidate buffers for compaction. + */ +class CompactBufferCandidates { + std::vector<CompactBufferCandidate> _candidates; + size_t _used; + size_t _dead; + uint32_t _max_buffers; + double _ratio; + size_t _slack; +public: + CompactBufferCandidates(uint32_t num_buffers, uint32_t max_buffers, double ratio, size_t slack); + ~CompactBufferCandidates(); + void add(uint32_t buffer_id, size_t used, size_t dead); + void select(std::vector<uint32_t>& buffers); +}; + +} diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp index 059171e1f02..f137d5379fb 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp @@ -1,9 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "datastore.h" +#include "compact_buffer_candidates.h" #include "compaction_spec.h" +#include "compaction_strategy.h" #include <vespa/vespalib/util/array.hpp> #include <vespa/vespalib/util/stringfmt.h> +#include <algorithm> #include <limits> #include <cassert> @@ -529,41 +532,35 @@ DataStoreBase::markCompacting(uint32_t bufferId) std::vector<uint32_t> DataStoreBase::startCompactWorstBuffers(CompactionSpec compaction_spec, const CompactionStrategy& compaction_strategy) { - (void) compaction_strategy; - constexpr uint32_t noBufferId = std::numeric_limits<uint32_t>::max(); - uint32_t worstMemoryBufferId = noBufferId; - uint32_t worstAddressSpaceBufferId = noBufferId; - size_t worstDeadElems = 0; - size_t worstDeadArrays = 0; + // compact memory usage + CompactBufferCandidates elem_buffers(_numBuffers, compaction_strategy.get_max_buffers(), compaction_strategy.getMaxDeadBytesRatio() / 2, CompactionStrategy::DEAD_BYTES_SLACK); + // compact address space + CompactBufferCandidates array_buffers(_numBuffers, compaction_strategy.get_max_buffers(), compaction_strategy.getMaxDeadAddressSpaceRatio() / 2, CompactionStrategy::DEAD_ADDRESS_SPACE_SLACK); for (uint32_t bufferId = 0; bufferId < _numBuffers; ++bufferId) { const auto &state = getBufferState(bufferId); if (state.isActive()) { auto typeHandler = state.getTypeHandler(); uint32_t arraySize = typeHandler->getArraySize(); uint32_t reservedElements = typeHandler->getReservedElements(bufferId); + size_t used_elems = state.size(); size_t deadElems = state.getDeadElems() - reservedElements; - if (compaction_spec.compact_memory() && deadElems > worstDeadElems) { - worstMemoryBufferId = bufferId; - worstDeadElems = deadElems; + if (compaction_spec.compact_memory()) { + elem_buffers.add(bufferId, used_elems, deadElems); } if (compaction_spec.compact_address_space()) { - size_t deadArrays = deadElems / arraySize; - if (deadArrays > worstDeadArrays) { - worstAddressSpaceBufferId = bufferId; - worstDeadArrays = deadArrays; - } + array_buffers.add(bufferId, used_elems / arraySize, deadElems / arraySize); } } } std::vector<uint32_t> result; - if (worstMemoryBufferId != noBufferId) { - markCompacting(worstMemoryBufferId); - result.emplace_back(worstMemoryBufferId); - } - if (worstAddressSpaceBufferId != noBufferId && - worstAddressSpaceBufferId != worstMemoryBufferId) { - markCompacting(worstAddressSpaceBufferId); - result.emplace_back(worstAddressSpaceBufferId); + result.reserve(std::min(_numBuffers, 2 * compaction_strategy.get_max_buffers())); + elem_buffers.select(result); + array_buffers.select(result); + std::sort(result.begin(), result.end()); + auto last = std::unique(result.begin(), result.end()); + result.erase(last, result.end()); + for (auto buffer_id : result) { + markCompacting(buffer_id); } return result; } |