Merge pull request #12310 from vespa-engine/hakonhall/remove-large-orchestrator-locks-flag

Remove large-orchestrator-locks flag

5 files changed, 6 insertions, 35 deletions

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 32a235f2989..bb4e281a85f 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -126,14 +126,6 @@ public class Flags {
             "scheduled evenly distributed in the 1x-2x range (and naturally guaranteed at the 2x boundary).",
             "Takes effect on next run of NodeRebooter");
 
-    public static final UnboundBooleanFlag ENABLE_LARGE_ORCHESTRATOR_LOCKS = defineFeatureFlag(
-            "enable-large-orchestrator-locks", true,
-            "If enabled, the orchestrator will accumulate application locks during probe in batch suspension, " +
-            "and release them in reverse order only after the non-probe is complete. Can be set depending on " +
-            "parent hostname.",
-            "Takes immediate effect for new batch suspensions.",
-            HOSTNAME);
-
     public static final UnboundBooleanFlag RETIRE_WITH_PERMANENTLY_DOWN = defineFeatureFlag(
             "retire-with-permanently-down", false,
             "If enabled, retirement will end with setting the host status to PERMANENTLY_DOWN, " +
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
index eb6a4119f8a..af59eb11e9b 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
@@ -38,9 +38,11 @@ public class OrchestratorContext implements AutoCloseable {
     private final HashMap<ApplicationInstanceReference, Runnable> locks = new HashMap<>();
 
     /** Create an OrchestratorContext for operations on multiple applications. */
-    public static OrchestratorContext createContextForMultiAppOp(Clock clock, boolean largeLocks) {
+    public static OrchestratorContext createContextForMultiAppOp(Clock clock) {
         return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_BATCH_OP),
-                false, largeLocks, false);
+                false, // probe
+                true, // large locks
+                false); // use permanently down status
     }
 
     /** Create an OrchestratorContext for an operation on a single application. */
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
index fbe6864274c..f860370faf9 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -64,7 +64,6 @@ public class OrchestratorImpl implements Orchestrator {
     private final ClusterControllerClientFactory clusterControllerClientFactory;
     private final Clock clock;
     private final ApplicationApiFactory applicationApiFactory;
-    private final BooleanFlag enableLargeOrchestratorLocks;
     private final BooleanFlag retireWithPermanentlyDownFlag;
 
     @Inject
@@ -101,7 +100,6 @@ public class OrchestratorImpl implements Orchestrator {
         this.instanceLookupService = instanceLookupService;
         this.clock = clock;
         this.applicationApiFactory = applicationApiFactory;
-        this.enableLargeOrchestratorLocks = Flags.ENABLE_LARGE_ORCHESTRATOR_LOCKS.bindTo(flagSource);
         this.retireWithPermanentlyDownFlag = Flags.RETIRE_WITH_PERMANENTLY_DOWN.bindTo(flagSource);
     }
 
@@ -258,10 +256,7 @@ public class OrchestratorImpl implements Orchestrator {
     @Override
     public void suspendAll(HostName parentHostname, List<HostName> hostNames)
             throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException {
-        boolean largeLocks = enableLargeOrchestratorLocks
-                .with(FetchVector.Dimension.HOSTNAME, parentHostname.s())
-                .value();
-        try (OrchestratorContext context = OrchestratorContext.createContextForMultiAppOp(clock, largeLocks)) {
+        try (OrchestratorContext context = OrchestratorContext.createContextForMultiAppOp(clock)) {
             List<NodeGroup> nodeGroupsOrderedByApplication;
             try {
                 nodeGroupsOrderedByApplication = nodeGroupsOrderedForSuspend(hostNames);
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorContextTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorContextTest.java
index 607894ee104..16b66b2804e 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorContextTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorContextTest.java
@@ -23,7 +23,7 @@ public class OrchestratorContextTest {
         var mutable = new Object() { boolean locked = true; };
         Runnable unlock = () -> mutable.locked = false;
 
-        try (OrchestratorContext rootContext = OrchestratorContext.createContextForMultiAppOp(new ManualClock(), true)) {
+        try (OrchestratorContext rootContext = OrchestratorContext.createContextForMultiAppOp(new ManualClock())) {
             try (OrchestratorContext probeContext = rootContext.createSubcontextForSingleAppOp(true)) {
                 assertFalse(probeContext.hasLock(application));
                 assertTrue(probeContext.registerLockAcquisition(application, unlock));
@@ -41,19 +41,4 @@ public class OrchestratorContextTest {
         }
         assertFalse(mutable.locked);
     }
-
-    @Test
-    public void testLargeLocksDisabled() {
-        var mutable = new Object() { boolean locked = true; };
-        Runnable unlock = () -> mutable.locked = false;
-
-        try (OrchestratorContext rootContext = OrchestratorContext.createContextForMultiAppOp(new ManualClock(), false)) {
-            try (OrchestratorContext probeContext = rootContext.createSubcontextForSingleAppOp(true)) {
-                assertFalse(probeContext.hasLock(application));
-                assertFalse(probeContext.registerLockAcquisition(application, unlock));
-            }
-        }
-
-        assertTrue(mutable.locked);
-    }
 }
\ No newline at end of file
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
index 77ec824da54..bfe4b523e4a 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -17,7 +17,6 @@ import com.yahoo.vespa.applicationmodel.ServiceStatus;
 import com.yahoo.vespa.applicationmodel.ServiceType;
 import com.yahoo.vespa.applicationmodel.TenantId;
 import com.yahoo.vespa.curator.mock.MockCurator;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock;
@@ -325,8 +324,6 @@ public class OrchestratorImplTest {
 
     @Test
     public void testLargeLocks() throws Exception {
-        flagSource.withBooleanFlag(Flags.ENABLE_LARGE_ORCHESTRATOR_LOCKS.id(), true);
-
         var tenantId = new TenantId("tenant");
         var applicationInstanceId = new ApplicationInstanceId("app:dev:us-east-1:default");
         var applicationInstanceReference = new ApplicationInstanceReference(tenantId, applicationInstanceId);