From 3578ae64641d6a3fb412d6ca1516e3cec0ac70e3 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Wed, 12 Jan 2022 14:46:54 +0000 Subject: Set defaults in config defs and ModelContext api to improve bucket merge performance on content nodes. These are the same defaults set for the feature flags in https://github.com/vespa-engine/vespa/pull/20759. --- .../com/yahoo/config/model/api/ModelContext.java | 18 ++++++++-------- .../yahoo/config/model/deploy/TestProperties.java | 16 +++++++-------- .../model/builder/xml/dom/ContentBuilderTest.java | 2 +- .../vespa/model/content/ContentClusterTest.java | 24 +++++++++++----------- .../vespa/model/content/StorageClusterTest.java | 10 ++++----- configdefinitions/src/vespa/stor-filestor.def | 2 +- .../shared_threading_service_test.cpp | 5 +++-- searchcore/src/vespa/searchcore/config/proton.def | 4 ++-- .../storage/config/stor-distributormanager.def | 6 +++--- storage/src/vespa/storage/config/stor-server.def | 4 ++-- 10 files changed, 46 insertions(+), 45 deletions(-) diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 2ea1ef186d2..03813d21ca8 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -71,9 +71,9 @@ public interface ModelContext { interface FeatureFlags { @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Revisit in May or June 2021") default double defaultTermwiseLimit() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"vekterli"}) default boolean useThreePhaseUpdates() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Select sequencer type use while feeding") default String feedSequencerType() { throw new UnsupportedOperationException("TODO specify default value"); } + @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Select sequencer type use while feeding") default String feedSequencerType() { return "THROUGHPUT"; } @ModelFeatureFlag(owners = {"geirst, baldersheim"}) default int feedTaskLimit() { return 1000; } - @ModelFeatureFlag(owners = {"geirst, baldersheim"}) default int feedMasterTaskLimit() { return 0; } + @ModelFeatureFlag(owners = {"geirst, baldersheim"}) default int feedMasterTaskLimit() { return 1000; } @ModelFeatureFlag(owners = {"geirst, baldersheim"}) default String sharedFieldWriterExecutor() { return "NONE"; } @ModelFeatureFlag(owners = {"baldersheim"}) default String responseSequencerType() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default int defaultNumResponseThreads() { return 2; } @@ -85,9 +85,9 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default int metricsproxyNumThreads() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}, removeAfter = "7.526") default int largeRankExpressionLimit() { return 8192; } @ModelFeatureFlag(owners = {"baldersheim"}) default int maxUnCommittedMemory() { return 130000; } - @ModelFeatureFlag(owners = {"baldersheim"}) default int maxConcurrentMergesPerNode() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"baldersheim"}) default int maxMergeQueueSize() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean ignoreMergeQueueLimit() { throw new UnsupportedOperationException("TODO specify default value"); } + @ModelFeatureFlag(owners = {"baldersheim"}) default int maxConcurrentMergesPerNode() { return 16; } + @ModelFeatureFlag(owners = {"baldersheim"}) default int maxMergeQueueSize() { return 100; } + @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean ignoreMergeQueueLimit() { return true; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean containerDumpHeapOnShutdownTimeout() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default double containerShutdownTimeout() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default double diskBloatFactor() { throw new UnsupportedOperationException("TODO specify default value"); } @@ -101,12 +101,12 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitMemory() { return 0.8; } @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default double minNodeRatioPerGroup() { return 0.0; } @ModelFeatureFlag(owners = {"arnej"}) default boolean newLocationBrokerLogic() { return true; } - @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default int distributorMergeBusyWait() { return 10; } - @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean distributorEnhancedMaintenanceScheduling() { return false; } + @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default int distributorMergeBusyWait() { return 1; } + @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean distributorEnhancedMaintenanceScheduling() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean forwardIssuesAsErrors() { return true; } - @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default boolean asyncApplyBucketDiff() { return false; } + @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default boolean asyncApplyBucketDiff() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean ignoreThreadStackSizes() { return false; } - @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean unorderedMergeChaining() { return false; } + @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean unorderedMergeChaining() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean useV8GeoPositions() { return false; } @ModelFeatureFlag(owners = {"arnej", "baldersheim"}) default boolean useV8DocManagerCfg() { return false; } @ModelFeatureFlag(owners = {"baldersheim", "geirst", "toregge"}) default int maxCompactBuffers() { return 1; } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 0007fa0d18a..3a3df4224b1 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -40,9 +40,9 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private boolean useThreePhaseUpdates = false; private double defaultTermwiseLimit = 1.0; private String jvmGCOptions = null; - private String sequencerType = "LATENCY"; + private String sequencerType = "THROUGHPUT"; private int feedTaskLimit = 1000; - private int feedMasterTaskLimit = 0; + private int feedMasterTaskLimit = 1000; private String sharedFieldWriterExecutor = "NONE"; private boolean firstTimeDeployment = false; private String responseSequencerType = "ADAPTIVE"; @@ -57,8 +57,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private List tenantSecretStores = Collections.emptyList(); private String jvmOmitStackTraceInFastThrowOption; private int maxConcurrentMergesPerNode = 16; - private int maxMergeQueueSize = 1024; - private boolean ignoreMergeQueueLimit = false; + private int maxMergeQueueSize = 100; + private boolean ignoreMergeQueueLimit = true; private boolean allowDisableMtls = true; private List operatorCertificates = Collections.emptyList(); private double resourceLimitDisk = 0.8; @@ -66,13 +66,13 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private double minNodeRatioPerGroup = 0.0; private boolean containerDumpHeapOnShutdownTimeout = false; private double containerShutdownTimeout = 50.0; - private int distributorMergeBusyWait = 10; + private int distributorMergeBusyWait = 1; private int docstoreCompressionLevel = 9; private int maxUnCommittedMemory = 123456; private double diskBloatFactor = 0.25; - private boolean distributorEnhancedMaintenanceScheduling = false; - private boolean asyncApplyBucketDiff = false; - private boolean unorderedMergeChaining = false; + private boolean distributorEnhancedMaintenanceScheduling = true; + private boolean asyncApplyBucketDiff = true; + private boolean unorderedMergeChaining = true; private List zoneDnsSuffixes = List.of(); private int maxCompactBuffers = 1; private boolean failDeploymentWithInvalidJvmOptions = false; 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 a27b6786800..c746db168ee 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 @@ -825,7 +825,7 @@ public class ContentBuilderTest extends DomBuilderTest { @Test public void feed_master_task_limit_is_controlled_by_feature_flag() { - assertEquals(0, resolveFeedMasterTaskLimitConfigWithFeatureFlag(null)); + assertEquals(1000, resolveFeedMasterTaskLimitConfigWithFeatureFlag(null)); assertEquals(2000, resolveFeedMasterTaskLimitConfigWithFeatureFlag(2000)); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java index 4295f0aa6ec..4591726d1f9 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java @@ -1127,8 +1127,8 @@ public class ContentClusterTest extends ContentBaseTest { @Test public void distributor_merge_busy_wait_controlled_by_properties() throws Exception { - assertEquals(10, resolveDistributorMergeBusyWaitConfig(Optional.empty())); - assertEquals(1, resolveDistributorMergeBusyWaitConfig(Optional.of(1))); + assertEquals(1, resolveDistributorMergeBusyWaitConfig(Optional.empty())); + assertEquals(5, resolveDistributorMergeBusyWaitConfig(Optional.of(5))); } private int resolveDistributorMergeBusyWaitConfig(Optional mergeBusyWait) throws Exception { @@ -1144,14 +1144,14 @@ public class ContentClusterTest extends ContentBaseTest { @Test public void distributor_enhanced_maintenance_scheduling_controlled_by_properties() throws Exception { - assertFalse(resolveDistributorEnhancedSchedulingConfig(false)); - assertTrue(resolveDistributorEnhancedSchedulingConfig(true)); + assertFalse(resolveDistributorEnhancedSchedulingConfig(Optional.of(false))); + assertTrue(resolveDistributorEnhancedSchedulingConfig(Optional.empty())); } - private boolean resolveDistributorEnhancedSchedulingConfig(boolean enhancedScheduling) throws Exception { + private boolean resolveDistributorEnhancedSchedulingConfig(Optional enhancedScheduling) throws Exception { var props = new TestProperties(); - if (enhancedScheduling) { - props.distributorEnhancedMaintenanceScheduling(enhancedScheduling); + if (enhancedScheduling.isPresent()) { + props.distributorEnhancedMaintenanceScheduling(enhancedScheduling.get()); } var cluster = createOneNodeCluster(props); var builder = new StorDistributormanagerConfig.Builder(); @@ -1161,14 +1161,14 @@ public class ContentClusterTest extends ContentBaseTest { @Test public void unordered_merge_chaining_config_controlled_by_properties() throws Exception { - assertFalse(resolveUnorderedMergeChainingConfig(false)); - assertTrue(resolveUnorderedMergeChainingConfig(true)); + assertFalse(resolveUnorderedMergeChainingConfig(Optional.of(false))); + assertTrue(resolveUnorderedMergeChainingConfig(Optional.empty())); } - private boolean resolveUnorderedMergeChainingConfig(boolean unorderedMergeChaining) throws Exception { + private boolean resolveUnorderedMergeChainingConfig(Optional unorderedMergeChaining) throws Exception { var props = new TestProperties(); - if (unorderedMergeChaining) { - props.setUnorderedMergeChaining(true); + if (unorderedMergeChaining.isPresent()) { + props.setUnorderedMergeChaining(unorderedMergeChaining.get()); } var cluster = createOneNodeCluster(props); var builder = new StorDistributormanagerConfig.Builder(); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java index 739f8b7fff2..c521d24d22f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java @@ -117,8 +117,8 @@ public class StorageClusterTest { StorServerConfig config = new StorServerConfig(builder); assertEquals(16, config.max_merges_per_node()); - assertEquals(1024, config.max_merge_queue_size()); - assertFalse(config.disable_queue_limits_for_chained_merges()); + assertEquals(100, config.max_merge_queue_size()); + assertTrue(config.disable_queue_limits_for_chained_merges()); } @Test @@ -171,10 +171,10 @@ public class StorageClusterTest { @Test public void async_apply_bucket_diff_can_be_controlled_by_feature_flag() { var config = filestorConfigFromProperties(new TestProperties()); - assertFalse(config.async_apply_bucket_diff()); - - config = filestorConfigFromProperties(new TestProperties().setAsyncApplyBucketDiff(true)); assertTrue(config.async_apply_bucket_diff()); + + config = filestorConfigFromProperties(new TestProperties().setAsyncApplyBucketDiff(false)); + assertFalse(config.async_apply_bucket_diff()); } @Test diff --git a/configdefinitions/src/vespa/stor-filestor.def b/configdefinitions/src/vespa/stor-filestor.def index c351e52b557..1aaa7f71f8e 100644 --- a/configdefinitions/src/vespa/stor-filestor.def +++ b/configdefinitions/src/vespa/stor-filestor.def @@ -45,7 +45,7 @@ bucket_merge_chunk_size int default=33554432 restart ## If set, portions of apply bucket diff handling will be performed asynchronously ## with persistence thread not waiting for local writes to complete. -async_apply_bucket_diff bool default=false +async_apply_bucket_diff bool default=true ## When merging, it is possible to send more metadata than needed in order to ## let local nodes in merge decide which entries fits best to add this time diff --git a/searchcore/src/tests/proton/server/shared_threading_service/shared_threading_service_test.cpp b/searchcore/src/tests/proton/server/shared_threading_service/shared_threading_service_test.cpp index e90bfc8ae57..bede9f967a4 100644 --- a/searchcore/src/tests/proton/server/shared_threading_service/shared_threading_service_test.cpp +++ b/searchcore/src/tests/proton/server/shared_threading_service/shared_threading_service_test.cpp @@ -24,7 +24,7 @@ make_proton_config(double concurrency) builder.feeding.concurrency = concurrency; builder.feeding.sharedFieldWriterExecutor = ProtonConfig::Feeding::SharedFieldWriterExecutor::DOCUMENT_DB; - builder.indexing.tasklimit = 300; + builder.indexing.tasklimit = 255; return builder; } @@ -73,7 +73,8 @@ TEST_F(SharedThreadingServiceTest, field_writer_can_be_shared_across_all_documen setup(0.75, 8); EXPECT_TRUE(field_writer()); EXPECT_EQ(6, field_writer()->getNumExecutors()); - EXPECT_EQ(300, field_writer()->first_executor()->getTaskLimit()); + // This is rounded to the nearest power of 2 when using THROUGHPUT feed executor. + EXPECT_EQ(256, field_writer()->first_executor()->getTaskLimit()); } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index 62b9f6416af..34530afe9f9 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -130,7 +130,7 @@ indexing.threads int default=1 restart ## Option to specify what is most important during indexing. ## This is experimental and will most likely be temporary. -indexing.optimize enum {LATENCY, THROUGHPUT, ADAPTIVE} default=LATENCY restart +indexing.optimize enum {LATENCY, THROUGHPUT, ADAPTIVE} default=THROUGHPUT restart ## Maximum number of pending operations for each of the internal ## indexing threads. Only used when visibility delay is zero. @@ -517,7 +517,7 @@ feeding.shared_field_writer_executor enum {NONE, INDEX, INDEX_AND_ATTRIBUTE, DOC ## This limit is only considered when executing tasks for handling external feed operations. ## In that case the calling thread (persistence thread) is blocked until the master thread has capacity to handle more tasks. ## When this limit is set to 0 it is ignored. -feeding.master_task_limit int default = 0 +feeding.master_task_limit int default = 1000 ## Adjustment to resource limit when determining if maintenance jobs can run. ## diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def index 5162e337f24..dba36d1b73d 100644 --- a/storage/src/vespa/storage/config/stor-distributormanager.def +++ b/storage/src/vespa/storage/config/stor-distributormanager.def @@ -189,7 +189,7 @@ sequence_mutating_operations bool default=true ## Number of seconds that scheduling of new merge operations should be inhibited ## towards a node if it has indicated that its merge queues are full or it is ## suffering from resource exhaustion. -inhibit_merge_sending_on_busy_node_duration_sec int default=10 +inhibit_merge_sending_on_busy_node_duration_sec int default=1 ## If set, enables potentially stale reads during cluster state transitions where ## buckets change ownership. This also implicitly enables support for two-phase @@ -285,11 +285,11 @@ num_distributor_stripes int default=0 restart ## bucket maintenance priority database even when no operation can be started for the ## bucket due to being blocked by concurrent operations. This avoids potential head-of-line ## blocking of later buckets in the priority database. -implicitly_clear_bucket_priority_on_schedule bool default=false +implicitly_clear_bucket_priority_on_schedule bool default=true ## Enables sending merges that are forwarded between content nodes in ideal state node key ## order, instead of strictly increasing node key order (which is the default). ## Even if this config is set to true, unordered merges will only be sent if _all_ nodes ## involved in a given merge have previously reported (as part of bucket info fetching) ## that they support the unordered merge feature. -use_unordered_merge_chaining bool default=false +use_unordered_merge_chaining bool default=true diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def index 6611c3cba91..a368c2e5b6f 100644 --- a/storage/src/vespa/storage/config/stor-server.def +++ b/storage/src/vespa/storage/config/stor-server.def @@ -34,7 +34,7 @@ node_reliability int default=1 restart ## merge, only actually starting the operation when every node has ## allowed it to pass through. max_merges_per_node int default=16 -max_merge_queue_size int default=1024 +max_merge_queue_size int default=100 ## If the persistence provider indicates that it has exhausted one or more ## of its internal resources during a mutating operation, new merges will @@ -51,7 +51,7 @@ resource_exhaustion_merge_back_pressure_duration_secs double default=30.0 ## the current queue size. This avoids wasting the time spent being accepted ## into merge windows, which would happen if the merge were to be bounced with ## a busy-reply that would subsequently be unwound through the entire merge chain. -disable_queue_limits_for_chained_merges bool default=false +disable_queue_limits_for_chained_merges bool default=true ## Whether the deadlock detector should be enabled or not. If disabled, it will ## still run, but it will never actually abort the process it is running in. -- cgit v1.2.3