From f2d2048de4028a5e78e29e7c7949141c1a1e8aff Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 8 Feb 2021 12:44:41 +0100 Subject: Remove obsolete feature falg --- .../main/java/ai/vespa/reindexing/Reindexer.java | 13 +++++-------- .../ai/vespa/reindexing/ReindexingMaintainer.java | 3 +-- .../java/ai/vespa/reindexing/ReindexerTest.java | 21 ++++++++++----------- .../com/yahoo/config/model/api/ModelContext.java | 2 +- .../ClusterControllerContainer.java | 1 - .../ClusterControllerContainerCluster.java | 3 +-- .../admin/clustercontroller/ReindexingContext.java | 6 +----- configdefinitions/src/vespa/reindexing.def | 9 +-------- .../config/server/deploy/ModelContextImpl.java | 3 --- .../src/main/java/com/yahoo/vespa/flags/Flags.java | 7 ------- 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 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 ready, ReindexingCurator database, - Function visitorSessions, Metric metric, Clock clock, - double windowSizeIncrement) { + Function 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> 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", -- cgit v1.2.3