diff options
16 files changed, 88 insertions, 55 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/ContentChannelOutputStream.java b/container-core/src/main/java/com/yahoo/container/jdisc/ContentChannelOutputStream.java index d5dc18c1e5e..037506bed7f 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/ContentChannelOutputStream.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/ContentChannelOutputStream.java @@ -84,7 +84,7 @@ public class ContentChannelOutputStream extends OutputStream implements Writable */ @Override public void write(byte[] b) throws IOException { - nonCopyingWrite(Arrays.copyOf(b, b.length)); + nonCopyingWrite(Arrays.copyOf(b, b.length)); } /** @@ -142,10 +142,13 @@ public class ContentChannelOutputStream extends OutputStream implements Writable } private class LoggingCompletionHandler implements CompletionHandler { + private final CompletionHandler nested; + LoggingCompletionHandler(CompletionHandler nested) { this.nested = nested; } + @Override public void completed() { if (nested != null) { diff --git a/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java b/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java index 2749b73272d..bac514266cb 100644 --- a/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java +++ b/container-core/src/main/java/com/yahoo/processing/rendering/AsynchronousSectionedRenderer.java @@ -16,7 +16,6 @@ import com.yahoo.processing.response.Streamed; import java.io.IOException; import java.io.OutputStream; import java.util.ArrayDeque; -import java.util.Collections; import java.util.Deque; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -143,7 +142,7 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e } /** - * Create an renderer using the specified executor instead of the default one which should be used for production. + * Create a renderer using the specified executor instead of the default one which should be used for production. * Using a custom executor is useful for tests to avoid creating new threads for each renderer registry. * * @param executor the executor to use or null to use the default executor suitable for production @@ -226,7 +225,7 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e this.response = response; this.stream = stream; this.execution = execution; - DataListListener parentOfTopLevelListener = new DataListListener(new ParentOfTopLevel(request,response.data()), null); + DataListListener parentOfTopLevelListener = new DataListListener(new ParentOfTopLevel(request, response.data()), null); dataListListenerStack.addFirst(parentOfTopLevelListener); success = new CompletableFuture<>(); try { @@ -264,13 +263,13 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e /** * How deep into the tree of nested data lists the callback currently is. - * beginList() is invoked after this this is increased, and endList() is + * beginList() is invoked after this is increased, and endList() is * invoked before it is decreased. * * @return an integer of 1 or above */ public int getRecursionLevel() { - return dataListListenerStack.size()-1; + return dataListListenerStack.size() - 1; } /** @@ -406,7 +405,7 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e /** Renders a list. */ private void renderDataList(DataList list) throws IOException { - final boolean ordered = isOrdered(list); + boolean ordered = isOrdered(list); if (list.asList() == null) { logger.log(Level.WARNING, "DataList.asList() returned null, indicating it is closed. " + "This is likely caused by adding the same list multiple " + @@ -565,9 +564,9 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e */ private static class ParentOfTopLevel extends AbstractDataList { - private DataList trueTopLevel; + private final DataList trueTopLevel; - public ParentOfTopLevel(Request request,DataList trueTopLevel) { + public ParentOfTopLevel(Request request, DataList trueTopLevel) { super(request); this.trueTopLevel = trueTopLevel; freeze(); @@ -585,13 +584,13 @@ public abstract class AsynchronousSectionedRenderer<RESPONSE extends Response> e @Override public Data get(int index) { - if (index>0) throw new IndexOutOfBoundsException(); + if (index > 0) throw new IndexOutOfBoundsException(); return trueTopLevel; } @Override public List<Data> asList() { - return Collections.<Data>singletonList(trueTopLevel); + return List.of(trueTopLevel); } @Override diff --git a/container-core/src/main/java/com/yahoo/processing/rendering/Renderer.java b/container-core/src/main/java/com/yahoo/processing/rendering/Renderer.java index df53ac846f2..97ca0bfb6a4 100644 --- a/container-core/src/main/java/com/yahoo/processing/rendering/Renderer.java +++ b/container-core/src/main/java/com/yahoo/processing/rendering/Renderer.java @@ -53,7 +53,7 @@ public abstract class Renderer<RESPONSE extends Response> extends AbstractCompon * @return a {@link CompletableFuture} containing a boolean where true indicates a successful rendering */ public abstract CompletableFuture<Boolean> renderResponse(OutputStream stream, RESPONSE response, - Execution execution, Request request); + Execution execution, Request request); /** * Name of the output encoding, if applicable. diff --git a/container-core/src/main/java/com/yahoo/processing/response/IncomingData.java b/container-core/src/main/java/com/yahoo/processing/response/IncomingData.java index fc0e6d21e13..358f8ad9693 100644 --- a/container-core/src/main/java/com/yahoo/processing/response/IncomingData.java +++ b/container-core/src/main/java/com/yahoo/processing/response/IncomingData.java @@ -100,7 +100,7 @@ public interface IncomingData<DATATYPE extends Data> { */ final class NullIncomingData<DATATYPE extends Data> implements IncomingData<DATATYPE> { - private DataList<DATATYPE> owner; + private final DataList<DATATYPE> owner; private final ImmediateFuture<DATATYPE> completionFuture; public NullIncomingData(DataList<DATATYPE> owner) { diff --git a/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java b/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java index c6906ea7566..bc730f18986 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java +++ b/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java @@ -91,8 +91,7 @@ public class HttpSearchResponse extends ExtendedResponse { @Override public void render(OutputStream output, ContentChannel networkChannel, CompletionHandler handler) throws IOException { - if (rendererCopy instanceof AsynchronousSectionedRenderer) { - AsynchronousSectionedRenderer<Result> renderer = (AsynchronousSectionedRenderer<Result>) rendererCopy; + if (rendererCopy instanceof AsynchronousSectionedRenderer<Result> renderer) { renderer.setNetworkWiring(networkChannel, handler); } try { diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java b/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java index 87880ce2445..dc044c77c94 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java @@ -86,7 +86,7 @@ public class ExecutionFactory extends AbstractComponent { this.schemaInfo = schemaInfo; this.specialTokens = new SpecialTokenRegistry(specialTokens); this.linguistics = linguistics; - this.renderingExecutor = createRenderingExecutor(); + this.renderingExecutor = new RenderingExecutorFactory().createExecutor(); this.rendererRegistry = new RendererRegistry(renderers.allComponents(), renderingExecutor); this.executor = executor != null ? executor : Executors.newSingleThreadExecutor(); } @@ -151,13 +151,4 @@ public class ExecutionFactory extends AbstractComponent { null); } - private static ThreadPoolExecutor createRenderingExecutor() { - int threadCount = Runtime.getRuntime().availableProcessors(); - ThreadPoolExecutor executor = new ThreadPoolExecutor(threadCount, threadCount, 1L, TimeUnit.SECONDS, - new LinkedBlockingQueue<>(), - ThreadFactoryFactory.getThreadFactory("common-rendering")); - executor.prestartAllCoreThreads(); - return executor; - } - } diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/RenderingExecutorFactory.java b/container-search/src/main/java/com/yahoo/search/searchchain/RenderingExecutorFactory.java new file mode 100644 index 00000000000..f67db059470 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/searchchain/RenderingExecutorFactory.java @@ -0,0 +1,39 @@ +package com.yahoo.search.searchchain; + +import com.yahoo.concurrent.ThreadFactoryFactory; + +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +/** + * Factory of the executor passed to renderers by default. + * + * @author bratseth + */ +class RenderingExecutorFactory { + + private final int maxQueuedRenderingTasksPerProcessor; + private final int availableProcessors; + + public RenderingExecutorFactory() { + this.maxQueuedRenderingTasksPerProcessor = 100; + this.availableProcessors = Runtime.getRuntime().availableProcessors(); + } + + ThreadPoolExecutor createExecutor() { + int maxOutstandingTasks = maxQueuedRenderingTasksPerProcessor * availableProcessors; + ThreadPoolExecutor executor = new ThreadPoolExecutor(availableProcessors, availableProcessors, 1L, TimeUnit.SECONDS, + new LinkedBlockingQueue<>(maxOutstandingTasks), + ThreadFactoryFactory.getThreadFactory("common-rendering"), + (task, exec) -> renderingRejected(maxOutstandingTasks)); + executor.prestartAllCoreThreads(); + return executor; + } + + private void renderingRejected(int maxOutstandingTasks) { + throw new RejectedExecutionException("More than " + maxOutstandingTasks + " rendering tasks queued, rejecting this"); + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java index dac9a0f3518..c75a5ca0b26 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java @@ -1,9 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.autoscale; -import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -14,10 +12,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.CapacityPolicies; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.Optional; import java.util.OptionalDouble; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A cluster with its associated metrics which allows prediction about its future behavior. @@ -27,8 +22,6 @@ import java.util.logging.Logger; */ public class ClusterModel { - private static final Logger log = Logger.getLogger(ClusterModel.class.getName()); - /** Containers typically use more cpu right after generation change, so discard those metrics */ public static final Duration warmupDuration = Duration.ofMinutes(7); @@ -175,7 +168,7 @@ public class ClusterModel { } public static Duration minScalingDuration(ClusterSpec clusterSpec) { - if (clusterSpec.isStateful()) return Duration.ofHours(8); + if (clusterSpec.isStateful()) return Duration.ofHours(6); return Duration.ofMinutes(5); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index 3d51c50f681..64c5dff0718 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -636,12 +636,11 @@ public class AutoscalingTest { Duration timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 200.0 : 100.0, t -> 0.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.25, 200); - fixture.tester().assertResources("Scale up since we assume we need 2x cpu for growth when no scaling time data", 8, 1, 1.6, 7.4, 29.0, fixture.autoscale()); - fixture.setScalingDuration(Duration.ofMinutes(5)); + fixture.setScalingDuration(Duration.ofHours(8)); fixture.tester().clock().advance(Duration.ofDays(2)); timeAdded = fixture.loader().addLoadMeasurements(100, t -> 100.0 + (t < 50 ? t : 100 - t), t -> 0.0); fixture.tester.clock().advance(timeAdded.negated()); @@ -650,7 +649,7 @@ public class AutoscalingTest { 8, 1, 1.2, 7.4, 29.0, fixture.autoscale()); - fixture.setScalingDuration(Duration.ofMinutes(60)); + fixture.setScalingDuration(Duration.ofHours(8)); fixture.tester().clock().advance(Duration.ofDays(2)); timeAdded = fixture.loader().addLoadMeasurements(100, t -> 100.0 + (t < 50 ? t * t * t : 125000 - (t - 49) * (t - 49) * (t - 49)), @@ -717,8 +716,8 @@ public class AutoscalingTest { timeAdded = fixture.loader().addLoadMeasurements(100, t -> t == 0 ? 200.0 : 100.0, t-> 0.0); fixture.tester.clock().advance(timeAdded.negated()); fixture.loader().addCpuMeasurements(0.4, 200); - fixture.tester().assertResources("Query only -> largest possible", - 8, 1, 2.5, 7.4, 29.0, + fixture.tester().assertResources("Query only -> larger", + 8, 1, 2.1, 7.4, 29.0, fixture.autoscale()); fixture.tester().clock().advance(Duration.ofDays(2)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingUsingBcpGroupInfoTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingUsingBcpGroupInfoTest.java index ae40795d783..29a7aff3e6a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingUsingBcpGroupInfoTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingUsingBcpGroupInfoTest.java @@ -246,7 +246,7 @@ public class AutoscalingUsingBcpGroupInfoTest { fixture.tester().clock().advance(duration6.negated()); fixture.loader().addQueryRateMeasurements(10, __ -> 40.0); fixture.tester().assertResources("Scaling up cpu using bcp group cpu info", - 8, 1, 1.6, 7.4, 29.0, + 8, 1, 1.5, 7.4, 29.0, fixture.autoscale()); // Local query rate is too low but global is even lower so disregard it, giving the same as above @@ -256,7 +256,7 @@ public class AutoscalingUsingBcpGroupInfoTest { fixture.tester().clock().advance(duration7.negated()); fixture.loader().addQueryRateMeasurements(10, __ -> 40.0); fixture.tester().assertResources("Scaling up cpu using bcp group cpu info", - 8, 1, 1.6, 7.4, 29.0, + 8, 1, 1.5, 7.4, 29.0, fixture.autoscale()); // Local query rate is too low to be fully confident, and so is global but as it is slightly larger, incorporate it slightly diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java index 0ab7040bdb4..b043a1cfb0f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java @@ -158,7 +158,7 @@ public class DynamicProvisioningTester { List.of()); // Remove scaling events cluster = cluster.with(ScalingEvent.create(cluster.minResources(), cluster.minResources(), 0, - clock().instant().minus(Duration.ofDays(1).minus(duration))).withCompletion(clock().instant().minus(Duration.ofDays(1)))); + clock().instant().minus(Duration.ofDays(1).plus(duration))).withCompletion(clock().instant().minus(Duration.ofDays(1)))); application = application.with(cluster); nodeRepository().applications().put(application, nodeRepository().applications().lock(applicationId)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json index 2a8f436b30c..42925b797d7 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json @@ -94,7 +94,7 @@ "at" : 123 } ], - "scalingDuration": 28800000 + "scalingDuration": 21600000 } } } diff --git a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java index d753f3d9e73..7de2627209c 100644 --- a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java +++ b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java @@ -199,7 +199,8 @@ public class DocumentGenMojo extends AbstractMojo { " * Generated by vespa-documentgen-plugin, do not edit.\n" + " * Date: "+new Date()+"\n" + " */\n"); - out.write("@com.yahoo.document.Generated\npublic class ConcreteDocumentFactory extends com.yahoo.docproc.AbstractConcreteDocumentFactory {\n"); + out.write("@com.yahoo.document.Generated\n"); + out.write("public class ConcreteDocumentFactory extends com.yahoo.docproc.AbstractConcreteDocumentFactory {\n"); out.write(ind()+"private static java.util.Map<java.lang.String, java.lang.Class<? extends com.yahoo.document.Document>> dTypes = new java.util.HashMap<java.lang.String, java.lang.Class<? extends com.yahoo.document.Document>>();\n"); out.write(ind()+"private static java.util.Map<java.lang.String, com.yahoo.document.DocumentType> docTypes = new java.util.HashMap<>();\n"); out.write(ind()+"private static java.util.Map<java.lang.String, java.lang.Class<? extends com.yahoo.document.datatypes.Struct>> sTypes = new java.util.HashMap<java.lang.String, java.lang.Class<? extends com.yahoo.document.datatypes.Struct>>();\n"); @@ -312,7 +313,8 @@ public class DocumentGenMojo extends AbstractMojo { " * Input annotation type: "+annType.getName()+"\n" + " * Date: "+new Date()+"\n" + " */\n" + - "@com.yahoo.document.Generated\npublic "+annTypeModifier(annType)+"class "+className+" extends "+getParentAnnotationType(annType)+" {\n\n"); + "@com.yahoo.document.Generated\n" + + "public "+annTypeModifier(annType)+"class "+className+" extends "+getParentAnnotationType(annType)+" {\n\n"); if (annType.getDataType() instanceof StructDataType) { out.write(ind() + "public "+className+"() {\n" + ind(2) + "setType(new com.yahoo.document.annotation.AnnotationType(\""+annType.getName()+"\", Fields.type));\n" + @@ -431,7 +433,9 @@ public class DocumentGenMojo extends AbstractMojo { " * Input document type: "+docType.getName()+"\n" + " * Date: "+new Date()+"\n" + " */\n" + - "@com.yahoo.document.Generated\npublic class "+className+" extends "+superType+" {\n\n"+ + "@com.yahoo.document.Generated\n" + + "@SuppressWarnings(\"unchecked\")\n" + + "public class "+className+" extends "+superType+" {\n\n"+ ind(1)+"/** The doc type of this.*/\n" + ind(1)+"public static final com.yahoo.document.DocumentType type = getDocumentType();\n\n"); diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/ThreadFactoryFactory.java b/vespajlib/src/main/java/com/yahoo/concurrent/ThreadFactoryFactory.java index a2478046a9e..4db74cc37a0 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/ThreadFactoryFactory.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/ThreadFactoryFactory.java @@ -11,6 +11,8 @@ import java.util.concurrent.atomic.AtomicInteger; */ public class ThreadFactoryFactory { + static private final Map<String, PooledFactory> factory = new HashMap<>(); + static public synchronized ThreadFactory getThreadFactory(String name) { PooledFactory p = factory.get(name); if (p == null) { @@ -30,16 +32,21 @@ public class ThreadFactoryFactory { } private static class PooledFactory { + + private final String name; + private final AtomicInteger poolId = new AtomicInteger(1); + private static class Factory implements ThreadFactory { + final ThreadGroup group; final AtomicInteger threadNumber = new AtomicInteger(1); final String namePrefix; final boolean isDaemon; @SuppressWarnings("removal") - Factory(final String name, boolean isDaemon) { + Factory(String name, boolean isDaemon) { this.isDaemon = isDaemon; - final SecurityManager s = System.getSecurityManager(); + SecurityManager s = System.getSecurityManager(); group = (s != null) ? s.getThreadGroup() : Thread.currentThread().getThreadGroup(); @@ -47,8 +54,8 @@ public class ThreadFactoryFactory { } @Override - public Thread newThread(final Runnable r) { - final Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement(), 0); + public Thread newThread(Runnable r) { + Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement(), 0); if (t.isDaemon() != isDaemon) { t.setDaemon(isDaemon); } @@ -58,16 +65,15 @@ public class ThreadFactoryFactory { return t; } } + PooledFactory(String name) { this.name = name; } + ThreadFactory getFactory(boolean isDaemon) { return new Factory(name + "-" + poolId.getAndIncrement() + "-thread-", isDaemon); - } - private final String name; - private final AtomicInteger poolId = new AtomicInteger(1); + } - static private final Map<String, PooledFactory> factory = new HashMap<>(); } diff --git a/vespajlib/src/main/java/com/yahoo/io/BufferChain.java b/vespajlib/src/main/java/com/yahoo/io/BufferChain.java index cee7a3c25dd..8fbf13e32ba 100644 --- a/vespajlib/src/main/java/com/yahoo/io/BufferChain.java +++ b/vespajlib/src/main/java/com/yahoo/io/BufferChain.java @@ -127,7 +127,7 @@ public final class BufferChain { } public void flush() throws IOException { - for (final ByteBuffer b : buffers) { + for (ByteBuffer b : buffers) { endpoint.send(b); } buffers.clear(); diff --git a/vespajlib/src/main/java/com/yahoo/io/WritableByteTransmitter.java b/vespajlib/src/main/java/com/yahoo/io/WritableByteTransmitter.java index 78551b5a578..3b0c1b55af7 100644 --- a/vespajlib/src/main/java/com/yahoo/io/WritableByteTransmitter.java +++ b/vespajlib/src/main/java/com/yahoo/io/WritableByteTransmitter.java @@ -5,7 +5,7 @@ import java.io.IOException; import java.nio.ByteBuffer; /** - * Marker interface for use with the BufferChain data store. + * For use with the BufferChain data store. * * @author Steinar Knutsen */ |