Merge pull request #16434 from vespa-engine/jonmv/remove-reindexer-window-size-flag

Remove obsolete feature falg

10 files changed, 20 insertions, 48 deletions

diff --git a/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/Reindexer.java b/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/Reindexer.java
index 306e06b7c7e..af43211011a 100644
--- a/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/Reindexer.java
+++ b/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/Reindexer.java
@@ -49,10 +49,9 @@ public class Reindexer {
     private final ReindexingMetrics metrics;
     private final Clock clock;
     private final Phaser phaser = new Phaser(2); // Reindexer and visitor.
-    private final double windowSizeIncrement;
 
     public Reindexer(Cluster cluster, Map<DocumentType, Instant> ready, ReindexingCurator database,
-                     DocumentAccess access, Metric metric, Clock clock, double windowSizeIncrement) {
+                     DocumentAccess access, Metric metric, Clock clock) {
         this(cluster,
              ready,
              database,
@@ -65,13 +64,12 @@ public class Reindexer {
                  }
              },
              metric,
-             clock,
-             windowSizeIncrement);
+             clock
+        );
     }
 
     Reindexer(Cluster cluster, Map<DocumentType, Instant> ready, ReindexingCurator database,
-              Function<VisitorParameters, Runnable> visitorSessions, Metric metric, Clock clock,
-              double windowSizeIncrement) {
+              Function<VisitorParameters, Runnable> visitorSessions, Metric metric, Clock clock) {
         for (DocumentType type : ready.keySet())
             cluster.bucketSpaceOf(type); // Verifies this is known.
 
@@ -81,7 +79,6 @@ public class Reindexer {
         this.visitorSessions = visitorSessions;
         this.metrics = new ReindexingMetrics(metric, cluster.name);
         this.clock = clock;
-        this.windowSizeIncrement = windowSizeIncrement;
     }
 
     /** Lets the reindexer abort any ongoing visit session, wait for it to complete normally, then exit. */
@@ -198,7 +195,7 @@ public class Reindexer {
 
     VisitorParameters createParameters(DocumentType type, ProgressToken progress) {
         VisitorParameters parameters = new VisitorParameters(type.getName());
-        parameters.setThrottlePolicy(new DynamicThrottlePolicy().setWindowSizeIncrement(windowSizeIncrement)
+        parameters.setThrottlePolicy(new DynamicThrottlePolicy().setWindowSizeIncrement(0.2)
                                                                 .setWindowSizeDecrementFactor(5)
                                                                 .setResizeRate(10)
                                                                 .setMinWindowSize(1));
diff --git a/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMaintainer.java b/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMaintainer.java
index 9cb1981c230..a37a24d2b01 100644
--- a/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMaintainer.java
+++ b/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMaintainer.java
@@ -72,8 +72,7 @@ public class ReindexingMaintainer extends AbstractComponent {
                                                                                               access.getDocumentTypeManager()),
                                                                         access,
                                                                         metric,
-                                                                        clock,
-                                                                        reindexingConfig.windowSizeIncrement()))
+                                                                        clock))
                                           .collect(toUnmodifiableList());
         this.executor = new ScheduledThreadPoolExecutor(reindexingConfig.clusters().size(), new DaemonThreadFactory("reindexer-"));
         if (reindexingConfig.enabled())
diff --git a/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java b/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java
index 0f290250e2d..8cf96fa8c45 100644
--- a/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java
+++ b/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java
@@ -32,7 +32,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author jonmv
@@ -58,12 +57,12 @@ class ReindexerTest {
     @Test
     void throwsWhenUnknownBuckets() {
         assertThrows(NullPointerException.class,
-                     () -> new Reindexer(new Cluster("cluster", Map.of()), Map.of(music, Instant.EPOCH), database, ReindexerTest::failIfCalled, metric, clock, 0.2));
+                     () -> new Reindexer(new Cluster("cluster", Map.of()), Map.of(music, Instant.EPOCH), database, ReindexerTest::failIfCalled, metric, clock));
     }
 
     @Test
     void throwsWhenLockHeldElsewhere() throws InterruptedException, ExecutionException {
-        Reindexer reindexer = new Reindexer(cluster, Map.of(music, Instant.EPOCH), database, ReindexerTest::failIfCalled, metric, clock, 0.2);
+        Reindexer reindexer = new Reindexer(cluster, Map.of(music, Instant.EPOCH), database, ReindexerTest::failIfCalled, metric, clock);
         Executors.newSingleThreadExecutor().submit(() -> database.lockReindexing("cluster")).get();
         assertThrows(ReindexingLockException.class, reindexer::reindex);
     }
@@ -71,13 +70,13 @@ class ReindexerTest {
     @Test
     @Timeout(10)
     void nothingToDoWithEmptyConfig() throws ReindexingLockException {
-        new Reindexer(cluster, Map.of(), database, ReindexerTest::failIfCalled, metric, clock, 0.2).reindex();
+        new Reindexer(cluster, Map.of(), database, ReindexerTest::failIfCalled, metric, clock).reindex();
         assertEquals(Map.of(), metric.metrics());
     }
 
     @Test
     void testParameters() {
-        Reindexer reindexer = new Reindexer(cluster, Map.of(), database, ReindexerTest::failIfCalled, metric, clock, 0.2);
+        Reindexer reindexer = new Reindexer(cluster, Map.of(), database, ReindexerTest::failIfCalled, metric, clock);
         ProgressToken token = new ProgressToken();
         VisitorParameters parameters = reindexer.createParameters(music, token);
         assertEquals("music:[document]", parameters.getFieldSet());
@@ -93,14 +92,14 @@ class ReindexerTest {
     @Timeout(10)
     void testReindexing() throws ReindexingLockException {
         // Reindexer is created without any ready document types, which means nothing should run.
-        new Reindexer(cluster, Map.of(), database, ReindexerTest::failIfCalled, metric, clock, 0.2).reindex();
+        new Reindexer(cluster, Map.of(), database, ReindexerTest::failIfCalled, metric, clock).reindex();
         Reindexing reindexing = Reindexing.empty();
         assertEquals(reindexing, database.readReindexing("cluster"));
 
         // New config tells reindexer to reindex "music" documents no earlier than at 10 millis after EPOCH, which isn't yet.
         // Nothing happens, since it's not yet time. This isn't supposed to happen unless high clock skew.
         clock.advance(Duration.ofMillis(5));
-        new Reindexer(cluster, Map.of(music, Instant.ofEpochMilli(10)), database, ReindexerTest::failIfCalled, metric, clock, 0.2).reindex();
+        new Reindexer(cluster, Map.of(music, Instant.ofEpochMilli(10)), database, ReindexerTest::failIfCalled, metric, clock).reindex();
         assertEquals(reindexing, database.readReindexing("cluster"));
 
         // It's time to reindex the "music" documents — let this complete successfully.
@@ -111,7 +110,7 @@ class ReindexerTest {
                     database.writeReindexing(Reindexing.empty(), "cluster"); // Wipe database to verify we write data from reindexer.
                     executor.execute(() -> parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.SUCCESS, "OK"));
                     return () -> shutDown.set(true);
-                }, metric, clock, 0.2).reindex();
+                }, metric, clock).reindex();
         reindexing = reindexing.with(music, Status.ready(clock.instant()).running().successful(clock.instant()));
         assertEquals(reindexing, database.readReindexing("cluster"));
         assertTrue(shutDown.get(), "Session was shut down");
@@ -146,7 +145,7 @@ class ReindexerTest {
                         shutDown.set(true);
                         parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.ABORTED, "Shut down");
                     };
-                }, metric, clock, 0.2));
+                }, metric, clock));
         aborted.get().reindex();
         reindexing = reindexing.with(music, Status.ready(clock.instant()).running().progressed(new ProgressToken()).halted());
         assertEquals(reindexing, database.readReindexing("cluster"));
@@ -164,13 +163,13 @@ class ReindexerTest {
                     database.writeReindexing(Reindexing.empty(), "cluster"); // Wipe database to verify we write data from reindexer.
                     executor.execute(() -> parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.FAILURE, "Error"));
                     return () -> shutDown.set(true);
-                }, metric, clock, 0.2).reindex();
+                }, metric, clock).reindex();
         reindexing = reindexing.with(music, Status.ready(clock.instant()).running().failed(clock.instant(), "Error"));
         assertEquals(reindexing, database.readReindexing("cluster"));
         assertTrue(shutDown.get(), "Session was shut down");
 
         // Document type is ignored in next run, as it has failed fatally.
-        new Reindexer(cluster, Map.of(music, Instant.ofEpochMilli(30)), database, ReindexerTest::failIfCalled, metric, clock, 0.2).reindex();
+        new Reindexer(cluster, Map.of(music, Instant.ofEpochMilli(30)), database, ReindexerTest::failIfCalled, metric, clock).reindex();
         assertEquals(reindexing, database.readReindexing("cluster"));
     }
 
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 a5dc3f21a1d..1a916e4f3d1 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
@@ -65,7 +65,7 @@ public interface ModelContext {
      */
     interface FeatureFlags {
         @ModelFeatureFlag(owners = {"bjorncs", "jonmv"}, removeAfter = "7.352") default boolean enableAutomaticReindexing() { return true; }
-        @ModelFeatureFlag(owners = {"jonmv"}) default double reindexerWindowSizeIncrement() { return 0.2; }
+        @ModelFeatureFlag(owners = {"jonmv"}, removeAfter = "7.355") default double reindexerWindowSizeIncrement() { return 0.2; }
         @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"); }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
index 4cb3dde0833..10d3d52561f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
@@ -155,7 +155,6 @@ public class ClusterControllerContainer extends Container implements
         }
 
         builder.enabled(ctx.reindexing().enabled());
-        builder.windowSizeIncrement(ctx.windowSizeIncrement());
         for (String clusterId : ctx.clusterIds()) {
             ReindexingConfig.Clusters.Builder clusterBuilder = new ReindexingConfig.Clusters.Builder();
             for (NewDocumentType type : ctx.documentTypesForCluster(clusterId)) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
index 8423f7d723a..f222a1f3de1 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
@@ -32,8 +32,7 @@ public class ClusterControllerContainerCluster extends ContainerCluster<ClusterC
     public ReindexingContext reindexingContext() { return reindexingContext; }
 
     private static ReindexingContext createReindexingContext(DeployState deployState) {
-        Reindexing reindexing = deployState.reindexing().orElse(Reindexing.DISABLED_INSTANCE);
-        return new ReindexingContext(reindexing, deployState.featureFlags().reindexerWindowSizeIncrement());
+        return new ReindexingContext(deployState.reindexing().orElse(Reindexing.DISABLED_INSTANCE));
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java
index 7380b950fb2..026c0b93712 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ReindexingContext.java
@@ -21,11 +21,9 @@ public class ReindexingContext {
     private final Object monitor = new Object();
     private final Map<String, Set<NewDocumentType>> documentTypesPerCluster = new HashMap<>();
     private final Reindexing reindexing;
-    private final double windowSizeIncrement;
 
-    public ReindexingContext(Reindexing reindexing, double windowSizeIncrement) {
+    public ReindexingContext(Reindexing reindexing) {
         this.reindexing = Objects.requireNonNull(reindexing);
-        this.windowSizeIncrement = windowSizeIncrement;
     }
 
     public void addDocumentType(String clusterId, NewDocumentType type) {
@@ -49,6 +47,4 @@ public class ReindexingContext {
 
     public Reindexing reindexing() { return reindexing; }
 
-    public double windowSizeIncrement() { return windowSizeIncrement; }
-
 }
diff --git a/configdefinitions/src/vespa/reindexing.def b/configdefinitions/src/vespa/reindexing.def
index 93dc767fed0..6eb084f6417 100644
--- a/configdefinitions/src/vespa/reindexing.def
+++ b/configdefinitions/src/vespa/reindexing.def
@@ -6,16 +6,9 @@ namespace=vespa.config.content.reindexing
 # Whether reindexing should run at all
 enabled                bool default=false
 
-# TODO jonmv: remove after 7.324 is gone
-# The name of the content cluster to reindex documents from
-clusterName            string default=""
-
-# TODO jonmv: remove after 7.324 is gone
-# Epoch millis after which latest reprocessing may begin, per document type
-status{}.readyAtMillis long
-
 # Epoch millis after which latest reprocessing may begin, per document type, per cluster
 clusters{}.documentTypes{}.readyAtMillis long
 
 # Window size increment used for the dynamic throttling policy of the reindexing visitor session
+# TODO jonmv: remove after 7.355 is gone
 windowSizeIncrement    double default=0.2
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 7cbdacca838..4eade1e76fe 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
@@ -147,7 +147,6 @@ public class ModelContextImpl implements ModelContext {
 
     public static class FeatureFlags implements ModelContext.FeatureFlags {
 
-        private final double reindexerWindowSizeIncrement;
         private final double defaultTermwiseLimit;
         private final boolean useThreePhaseUpdates;
         private final String feedSequencer;
@@ -167,7 +166,6 @@ public class ModelContextImpl implements ModelContext {
         private final double maxDeadBytesRatio;
 
         public FeatureFlags(FlagSource source, ApplicationId appId) {
-            this.reindexerWindowSizeIncrement = flagValue(source, appId, Flags.REINDEXER_WINDOW_SIZE_INCREMENT);
             this.defaultTermwiseLimit = flagValue(source, appId, Flags.DEFAULT_TERM_WISE_LIMIT);
             this.useThreePhaseUpdates = flagValue(source, appId, Flags.USE_THREE_PHASE_UPDATES);
             this.feedSequencer = flagValue(source, appId, Flags.FEED_SEQUENCER_TYPE);
@@ -187,7 +185,6 @@ public class ModelContextImpl implements ModelContext {
             this.maxDeadBytesRatio = flagValue(source, appId, Flags.MAX_DEAD_BYTES_RATIO);
         }
 
-        @Override public double reindexerWindowSizeIncrement() { return reindexerWindowSizeIncrement; }
         @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; }
         @Override public boolean useThreePhaseUpdates() { return useThreePhaseUpdates; }
         @Override public String feedSequencerType() { return feedSequencer; }
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 27864541a90..2421669b192 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -179,13 +179,6 @@ public class Flags {
             "Takes effect at redeployment",
             ZONE_ID, APPLICATION_ID);
 
-    public static final UnboundDoubleFlag REINDEXER_WINDOW_SIZE_INCREMENT = defineDoubleFlag(
-            "reindexer-window-size-increment", 0.2,
-            List.of("jonmv"), "2020-12-09", "2021-02-07",
-            "Window size increment for dynamic throttle policy used by reindexer visitor session — more means more aggressive reindexing",
-            "Takes effect on (re)deployment",
-            APPLICATION_ID);
-
     public static final UnboundBooleanFlag USE_BUCKET_EXECUTOR_FOR_LID_SPACE_COMPACT = defineFeatureFlag(
             "use-bucket-executor-for-lid-space-compact", false,
             List.of("baldersheim"), "2021-01-24", "2021-03-01",