Merge pull request #30024 from vespa-engine/balder/gc-use-per-document-delete-and-max-merge-memory-from-java

GC control of use-per-document-delete and max-merge-memory from confi…

9 files changed, 11 insertions, 88 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 eb5942bd49c..a6910c059fc 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
@@ -112,8 +112,8 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"bjorncs"}, removeAfter = "8.289") default boolean dynamicHeapSize() { return true; }
         @ModelFeatureFlag(owners = {"hmusum"}) default String unknownConfigDefinition() { return "warn"; }
         @ModelFeatureFlag(owners = {"hmusum"}) default int searchHandlerThreadpool() { return 2; }
-        @ModelFeatureFlag(owners = {"vekterli"}) default long mergingMaxMemoryUsagePerNode() { return -1; }
-        @ModelFeatureFlag(owners = {"vekterli"}) default boolean usePerDocumentThrottledDeleteBucket() { return false; }
+        @ModelFeatureFlag(owners = {"vekterli"}, removeAfter = "8.292.x") default long mergingMaxMemoryUsagePerNode() { return 0; }
+        @ModelFeatureFlag(owners = {"vekterli"}, removeAfter = "8.292.x") default boolean usePerDocumentThrottledDeleteBucket() { return true; }
         @ModelFeatureFlag(owners = {"baldersheim"}) default boolean alwaysMarkPhraseExpensive() { return false; }
         @ModelFeatureFlag(owners = {"hmusum"}) default boolean restartOnDeployWhenOnnxModelChanges() { return false; }
         @ModelFeatureFlag(owners = {"baldersheim"}) default boolean sortBlueprintsByCost() { return false; }
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 6f0254145d6..b088231e84a 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
@@ -84,8 +84,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private List<DataplaneToken> dataplaneTokens;
     private int contentLayerMetadataFeatureLevel = 0;
     private boolean dynamicHeapSize = false;
-    private long mergingMaxMemoryUsagePerNode = -1;
-    private boolean usePerDocumentThrottledDeleteBucket = false;
     private boolean restartOnDeployWhenOnnxModelChanges = false;
 
     @Override public ModelContext.FeatureFlags featureFlags() { return this; }
@@ -144,8 +142,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public List<DataplaneToken> dataplaneTokens() { return dataplaneTokens; }
     @Override public int contentLayerMetadataFeatureLevel() { return contentLayerMetadataFeatureLevel; }
     @Override public boolean dynamicHeapSize() { return dynamicHeapSize; }
-    @Override public long mergingMaxMemoryUsagePerNode() { return mergingMaxMemoryUsagePerNode; }
-    @Override public boolean usePerDocumentThrottledDeleteBucket() { return usePerDocumentThrottledDeleteBucket; }
     @Override public boolean restartOnDeployWhenOnnxModelChanges() { return restartOnDeployWhenOnnxModelChanges; }
 
     public TestProperties sharedStringRepoNoReclaim(boolean sharedStringRepoNoReclaim) {
@@ -379,16 +375,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
 
     public TestProperties setDynamicHeapSize(boolean b) { this.dynamicHeapSize = b; return this; }
 
-    public TestProperties setMergingMaxMemoryUsagePerNode(long maxUsage) {
-        this.mergingMaxMemoryUsagePerNode = maxUsage;
-        return this;
-    }
-
-    public TestProperties setUsePerDocumentThrottledDeleteBucket(boolean enableThrottling) {
-        this.usePerDocumentThrottledDeleteBucket = enableThrottling;
-        return this;
-    }
-
     public TestProperties setRestartOnDeployForOnnxModelChanges(boolean enable) {
         this.restartOnDeployWhenOnnxModelChanges = enable;
         return this;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java
index 18b9129cead..56ca23523b6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java
@@ -47,7 +47,6 @@ public class FileStorProducer implements StorFilestorConfig.Producer {
     private final int responseNumThreads;
     private final StorFilestorConfig.Response_sequencer_type.Enum responseSequencerType;
     private final boolean useAsyncMessageHandlingOnSchedule;
-    private final boolean usePerDocumentThrottledDeleteBucket;
 
     private static StorFilestorConfig.Response_sequencer_type.Enum convertResponseSequencerType(String sequencerType) {
         try {
@@ -63,9 +62,9 @@ public class FileStorProducer implements StorFilestorConfig.Producer {
         this.responseNumThreads = featureFlags.defaultNumResponseThreads();
         this.responseSequencerType = convertResponseSequencerType(featureFlags.responseSequencerType());
         this.useAsyncMessageHandlingOnSchedule = featureFlags.useAsyncMessageHandlingOnSchedule();
-        this.usePerDocumentThrottledDeleteBucket = featureFlags.usePerDocumentThrottledDeleteBucket();
     }
 
+
     @Override
     public void getConfig(StorFilestorConfig.Builder builder) {
         if (numThreads != null) {
@@ -75,7 +74,7 @@ public class FileStorProducer implements StorFilestorConfig.Producer {
         builder.num_response_threads(responseNumThreads);
         builder.response_sequencer_type(responseSequencerType);
         builder.use_async_message_handling_on_schedule(useAsyncMessageHandlingOnSchedule);
-        builder.use_per_document_throttled_delete_bucket(usePerDocumentThrottledDeleteBucket);
+        builder.use_per_document_throttled_delete_bucket(true);
         var throttleBuilder = new StorFilestorConfig.Async_operation_throttler.Builder();
         builder.async_operation_throttler(throttleBuilder);
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java
index 1865db0ec1c..81fc19de929 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java
@@ -29,7 +29,6 @@ public class StorServerProducer implements StorServerConfig.Producer {
     private final String clusterName;
     private Integer maxMergesPerNode;
     private Integer queueSize;
-    private Long mergingMaxMemoryUsagePerNode;
 
     private StorServerProducer setMaxMergesPerNode(Integer value) {
         if (value != null) {
@@ -46,7 +45,6 @@ public class StorServerProducer implements StorServerConfig.Producer {
 
     StorServerProducer(String clusterName, ModelContext.FeatureFlags featureFlags) {
         this.clusterName = clusterName;
-        this.mergingMaxMemoryUsagePerNode = featureFlags.mergingMaxMemoryUsagePerNode();
     }
 
     @Override
@@ -63,10 +61,8 @@ public class StorServerProducer implements StorServerConfig.Producer {
         if (queueSize != null) {
             builder.max_merge_queue_size(queueSize);
         }
-        if (mergingMaxMemoryUsagePerNode != null) {
-            builder.merge_throttling_memory_limit(
-                    new StorServerConfig.Merge_throttling_memory_limit.Builder()
-                            .max_usage_bytes(mergingMaxMemoryUsagePerNode));
-        }
+        builder.merge_throttling_memory_limit(
+                new StorServerConfig.Merge_throttling_memory_limit.Builder()
+                        .max_usage_bytes(0));
     }
 }
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 bdd61d93136..7c3e66aa109 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
@@ -189,7 +189,7 @@ public class StorageClusterTest {
         var config = configFromProperties(new TestProperties());
         var limit = config.merge_throttling_memory_limit();
 
-        assertEquals(-1L, limit.max_usage_bytes()); // TODO change default
+        assertEquals(0L, limit.max_usage_bytes());
         assertMergeAutoScaleConfigHasExpectedValues(limit);
     }
 
@@ -200,21 +200,6 @@ public class StorageClusterTest {
     }
 
     @Test
-    void merge_throttler_memory_limit_is_controlled_by_feature_flag() {
-        var config = configFromProperties(new TestProperties().setMergingMaxMemoryUsagePerNode(-1));
-        assertEquals(-1L, config.merge_throttling_memory_limit().max_usage_bytes());
-
-        config = configFromProperties(new TestProperties().setMergingMaxMemoryUsagePerNode(0));
-        assertEquals(0L, config.merge_throttling_memory_limit().max_usage_bytes());
-
-        config = configFromProperties(new TestProperties().setMergingMaxMemoryUsagePerNode(1_234_456_789));
-        assertEquals(1_234_456_789L, config.merge_throttling_memory_limit().max_usage_bytes());
-
-        // Feature flag should not affect the other config values
-        assertMergeAutoScaleConfigHasExpectedValues(config.merge_throttling_memory_limit());
-    }
-
-    @Test
     void testVisitors() {
         StorVisitorConfig.Builder builder = new StorVisitorConfig.Builder();
         parse(cluster("bees",
@@ -355,24 +340,6 @@ public class StorageClusterTest {
         assertTrue(config.async_operation_throttler().throttle_individual_merge_feed_ops());
     }
 
-    private void verifyUsePerDocumentThrottledDeleteBucket(boolean expected, Boolean enabled) {
-        var props = new TestProperties();
-        if (enabled != null) {
-            props.setUsePerDocumentThrottledDeleteBucket(enabled);
-        }
-        var config = filestorConfigFromProducer(simpleCluster(props));
-        assertEquals(expected, config.use_per_document_throttled_delete_bucket());
-    }
-
-    @Test
-    void delete_bucket_throttling_is_controlled_by_feature_flag() {
-        // TODO update default once rolled out and tested
-        verifyUsePerDocumentThrottledDeleteBucket(false, null);
-
-        verifyUsePerDocumentThrottledDeleteBucket(false, false);
-        verifyUsePerDocumentThrottledDeleteBucket(true, true);
-    }
-
     @Test
     void testCapacity() {
         String xml = joinLines(
diff --git a/configdefinitions/src/vespa/stor-filestor.def b/configdefinitions/src/vespa/stor-filestor.def
index 090f74dec12..950797f8bc2 100644
--- a/configdefinitions/src/vespa/stor-filestor.def
+++ b/configdefinitions/src/vespa/stor-filestor.def
@@ -121,4 +121,4 @@ async_operation_dynamic_throttling_window_increment int default=20 restart
 ## first place.
 ##
 ## This is a live config.
-use_per_document_throttled_delete_bucket bool default=false
+use_per_document_throttled_delete_bucket bool default=true
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index 22b2b581b44..d500e56d079 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -201,13 +201,11 @@ public class ModelContextImpl implements ModelContext {
         private final int rpc_events_before_wakeup;
         private final int heapPercentage;
         private final String summaryDecodePolicy;
-        private boolean sortBlueprintsByCost;
+        private final boolean sortBlueprintsByCost;
         private final boolean alwaysMarkPhraseExpensive;
         private final int contentLayerMetadataFeatureLevel;
         private final String unknownConfigDefinition;
         private final int searchHandlerThreadpool;
-        private final long mergingMaxMemoryUsagePerNode;
-        private final boolean usePerDocumentThrottledDeleteBucket;
         private final boolean restartOnDeployWhenOnnxModelChanges;
 
         public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
@@ -249,8 +247,6 @@ public class ModelContextImpl implements ModelContext {
             this.contentLayerMetadataFeatureLevel = flagValue(source, appId, version, Flags.CONTENT_LAYER_METADATA_FEATURE_LEVEL);
             this.unknownConfigDefinition = flagValue(source, appId, version, Flags.UNKNOWN_CONFIG_DEFINITION);
             this.searchHandlerThreadpool = flagValue(source, appId, version, Flags.SEARCH_HANDLER_THREADPOOL);
-            this.mergingMaxMemoryUsagePerNode = flagValue(source, appId, version, Flags.MERGING_MAX_MEMORY_USAGE_PER_NODE);
-            this.usePerDocumentThrottledDeleteBucket = flagValue(source, appId, version, Flags.USE_PER_DOCUMENT_THROTTLED_DELETE_BUCKET);
             this.alwaysMarkPhraseExpensive =  flagValue(source, appId, version, Flags.ALWAYS_MARK_PHRASE_EXPENSIVE);
             this.restartOnDeployWhenOnnxModelChanges = flagValue(source, appId, version, Flags.RESTART_ON_DEPLOY_WHEN_ONNX_MODEL_CHANGES);
             this.sortBlueprintsByCost = flagValue(source, appId, version, Flags.SORT_BLUEPRINTS_BY_COST);
@@ -303,8 +299,6 @@ public class ModelContextImpl implements ModelContext {
         @Override public int contentLayerMetadataFeatureLevel() { return contentLayerMetadataFeatureLevel; }
         @Override public String unknownConfigDefinition() { return unknownConfigDefinition; }
         @Override public int searchHandlerThreadpool() { return searchHandlerThreadpool; }
-        @Override public long mergingMaxMemoryUsagePerNode() { return mergingMaxMemoryUsagePerNode; }
-        @Override public boolean usePerDocumentThrottledDeleteBucket() { return usePerDocumentThrottledDeleteBucket; }
         @Override public boolean restartOnDeployWhenOnnxModelChanges() { return restartOnDeployWhenOnnxModelChanges; }
         @Override public boolean sortBlueprintsByCost() { return sortBlueprintsByCost; }
 
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 e800792af4f..4aac29c5093 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -421,25 +421,6 @@ public class Flags {
             "Whether to send cloud trial email notifications",
             "Takes effect immediately");
 
-    public static final UnboundLongFlag MERGING_MAX_MEMORY_USAGE_PER_NODE = defineLongFlag(
-            "merging-max-memory-usage-per-node", 0,
-            List.of("vekterli"), "2023-11-03", "2024-03-01",
-            "Soft limit of the maximum amount of memory that can be used across merge operations on a content node. " +
-            "Value semantics: < 0: unlimited (legacy behavior), == 0: auto-deduced from node HW and config," +
-            " > 0: explicit memory usage limit in bytes.",
-            "Takes effect at redeployment",
-            INSTANCE_ID);
-
-    public static final UnboundBooleanFlag USE_PER_DOCUMENT_THROTTLED_DELETE_BUCKET = defineFeatureFlag(
-            "use-per-document-throttled-delete-bucket", true,
-            List.of("vekterli"), "2023-11-13", "2024-03-01",
-            "If set, DeleteBucket operations are internally expanded to an individually persistence-" +
-            "throttled remove per document stored in the bucket. This makes the cost model of " +
-            "executing a DeleteBucket symmetrical with feeding the documents to the bucket in the " +
-            "first place.",
-            "Takes effect at redeployment",
-            INSTANCE_ID);
-
     public static final UnboundBooleanFlag ENABLE_NEW_PAYMENT_METHOD_FLOW = defineFeatureFlag(
             "enable-new-payment-method-flow", false,
             List.of("bjorncs"), "2023-11-29", "2024-03-01",
diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def
index 0d877d33277..3d304dd1727 100644
--- a/storage/src/vespa/storage/config/stor-server.def
+++ b/storage/src/vespa/storage/config/stor-server.def
@@ -56,7 +56,7 @@ merge_throttling_policy.window_size_increment double default=2.0
 ##    > 0  explicit limit in bytes
 ##   == 0  limit automatically deduced by content node
 ##    < 0  unlimited (legacy behavior)
-merge_throttling_memory_limit.max_usage_bytes long default=-1
+merge_throttling_memory_limit.max_usage_bytes long default=0
 
 ## If merge_throttling_memory_limit.max_usage_bytes == 0, this factor is used
 ## as a multiplier to automatically deduce a memory limit for merges on the