From 84c4e3320084013ba9e78eaef9e68d2422631740 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Thu, 29 Apr 2021 12:36:02 +0200 Subject: Only support cache-acl flag is false code branch --- .../admin/nodeadmin/NodeAdminStateUpdater.java | 55 ++-------------------- .../admin/nodeadmin/NodeAdminStateUpdaterTest.java | 44 +---------------- 2 files changed, 6 insertions(+), 93 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java index 772e17291ef..58ca4ae3f41 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java @@ -3,9 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.provision.HostName; -import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; @@ -17,18 +15,14 @@ import com.yahoo.yolean.Exceptions; import java.time.Clock; import java.time.Duration; -import java.time.Instant; import java.util.ArrayList; import java.util.EnumSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.function.Function; -import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -50,13 +44,11 @@ public class NodeAdminStateUpdater { private final ScheduledExecutorService metricsScheduler = Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("metricsscheduler")); - private final CachedSupplier> cachedAclSupplier; private final NodeAgentContextFactory nodeAgentContextFactory; private final NodeRepository nodeRepository; private final Orchestrator orchestrator; private final NodeAdmin nodeAdmin; private final String hostHostname; - private final BooleanFlag cacheAclFlag; public enum State { TRANSITIONING, RESUMED, SUSPENDED_NODE_ADMIN, SUSPENDED } @@ -75,8 +67,6 @@ public class NodeAdminStateUpdater { this.orchestrator = orchestrator; this.nodeAdmin = nodeAdmin; this.hostHostname = hostHostname.value(); - this.cachedAclSupplier = new CachedSupplier<>(clock, Duration.ofSeconds(115), () -> nodeRepository.getAcls(this.hostHostname)); - this.cacheAclFlag = Flags.CACHE_ACL.bindTo(flagSource); } public void start() { @@ -172,18 +162,10 @@ public class NodeAdminStateUpdater { void adjustNodeAgentsToRunFromNodeRepository() { try { - Map nodeSpecByHostname = nodeRepository.getNodes(hostHostname).stream() - .collect(Collectors.toMap(NodeSpec::hostname, Function.identity())); - Map aclByHostname = cacheAclFlag.value() ? - Optional.of(cachedAclSupplier.get()) - .filter(acls -> acls.keySet().containsAll(nodeSpecByHostname.keySet())) - .orElseGet(cachedAclSupplier::invalidateAndGet) : - cachedAclSupplier.invalidateAndGet(); - - Set nodeAgentContexts = nodeSpecByHostname.keySet().stream() - .map(hostname -> nodeAgentContextFactory.create( - nodeSpecByHostname.get(hostname), - aclByHostname.getOrDefault(hostname, Acl.EMPTY))) + Map aclByHostname = nodeRepository.getAcls(hostHostname); + + Set nodeAgentContexts = nodeRepository.getNodes(hostHostname).stream() + .map(node -> nodeAgentContextFactory.create(node, aclByHostname.getOrDefault(node.hostname(), Acl.EMPTY))) .collect(Collectors.toSet()); nodeAdmin.refreshContainersToRun(nodeAgentContexts); } catch (ConvergenceException e) { @@ -200,33 +182,4 @@ public class NodeAdminStateUpdater { .map(NodeSpec::hostname) .collect(Collectors.toList()); } - - private static class CachedSupplier implements Supplier { - private final Clock clock; - private final Duration expiration; - private final Supplier supplier; - private Instant refreshAt; - private T cachedValue; - - private CachedSupplier(Clock clock, Duration expiration, Supplier supplier) { - this.clock = clock; - this.expiration = expiration; - this.supplier = supplier; - this.refreshAt = Instant.MIN; - } - - @Override - public T get() { - if (! clock.instant().isBefore(refreshAt)) { - cachedValue = supplier.get(); - refreshAt = clock.instant().plus(expiration); - } - return cachedValue; - } - - private T invalidateAndGet() { - refreshAt = Instant.MIN; - return get(); - } - } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java index d348d2b74e9..4a678597e41 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin; import com.yahoo.config.provision.HostName; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; @@ -178,26 +177,6 @@ public class NodeAdminStateUpdaterTest { verify(orchestrator, times(1)).suspend(eq(hostHostname.value()), eq(activeHostnames)); } - @Test - public void uses_cached_acl() { - mockNodeRepo(NodeState.active, 1); - mockAcl(Acl.EMPTY, 1); - - updater.adjustNodeAgentsToRunFromNodeRepository(); - verify(nodeRepository, times(1)).getAcls(any()); - clock.advance(Duration.ofSeconds(30)); - - updater.adjustNodeAgentsToRunFromNodeRepository(); - clock.advance(Duration.ofSeconds(30)); - updater.adjustNodeAgentsToRunFromNodeRepository(); - clock.advance(Duration.ofSeconds(30)); - verify(nodeRepository, times(1)).getAcls(any()); - - clock.advance(Duration.ofSeconds(30)); - updater.adjustNodeAgentsToRunFromNodeRepository(); - verify(nodeRepository, times(2)).getAcls(any()); - } - @Test public void node_spec_and_acl_aligned() { Acl acl = new Acl.Builder().withTrustedPorts(22).build(); @@ -208,25 +187,6 @@ public class NodeAdminStateUpdaterTest { updater.adjustNodeAgentsToRunFromNodeRepository(); updater.adjustNodeAgentsToRunFromNodeRepository(); - verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host1.yahoo.com")), eq(acl)); - verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host2.yahoo.com")), eq(acl)); - verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host3.yahoo.com")), eq(acl)); - verify(nodeRepository, times(3)).getNodes(eq(hostHostname.value())); - verify(nodeRepository, times(1)).getAcls(eq(hostHostname.value())); - } - - @Test - public void node_spec_and_acl_aligned_with_acl_cache_disabled() { - flagSource.withBooleanFlag(Flags.CACHE_ACL.id(), false); - - Acl acl = new Acl.Builder().withTrustedPorts(22).build(); - mockNodeRepo(NodeState.active, 3); - mockAcl(acl, 1, 2, 3); - - updater.adjustNodeAgentsToRunFromNodeRepository(); - updater.adjustNodeAgentsToRunFromNodeRepository(); - updater.adjustNodeAgentsToRunFromNodeRepository(); - verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host1.yahoo.com")), eq(acl)); verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host2.yahoo.com")), eq(acl)); verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host3.yahoo.com")), eq(acl)); @@ -249,7 +209,7 @@ public class NodeAdminStateUpdaterTest { verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host2.yahoo.com")), eq(acl)); verify(nodeAgentContextFactory, times(1)).create(argThat(spec -> spec.hostname().equals("host3.yahoo.com")), eq(Acl.EMPTY)); verify(nodeRepository, times(3)).getNodes(eq(hostHostname.value())); - verify(nodeRepository, times(2)).getAcls(eq(hostHostname.value())); // During the first tick, the cache is invalidated and retried + verify(nodeRepository, times(3)).getAcls(eq(hostHostname.value())); } @Test @@ -265,7 +225,7 @@ public class NodeAdminStateUpdaterTest { verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host1.yahoo.com")), eq(acl)); verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host2.yahoo.com")), eq(acl)); verify(nodeRepository, times(3)).getNodes(eq(hostHostname.value())); - verify(nodeRepository, times(1)).getAcls(eq(hostHostname.value())); + verify(nodeRepository, times(3)).getAcls(eq(hostHostname.value())); } private void assertConvergeError(NodeAdminStateUpdater.State targetState, String reason) { -- cgit v1.2.3