perf(kubernetes): Reduce namespace lookups in a loop (#4036)

The call to primaryKinds() on a caching agent can be expensive, as it may (depending on the caching agent) need to look up available CRDs and/or available namespaces. We make this call in a tight loop in a few places; pull the call out of the loop. Also replace the lambda in liveNamespaceSupplier with an actual private function for readability (and to make later refactoring easier). Finally, we make a redundant call to credentials::isValidKind in the CRD caching agent; primaryKinds() already filters on this before returning.
spinnaker · Sep 17, 2019 · 92b3bc9 · 92b3bc9
1 parent 5d4af3f
commit 92b3bc9
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 27 deletions.
diff --git a/...ddriver/kubernetes/v2/caching/agent/KubernetesUnregisteredCustomResourceCachingAgent.java b/...ddriver/kubernetes/v2/caching/agent/KubernetesUnregisteredCustomResourceCachingAgent.java
@@ -47,7 +47,6 @@ public KubernetesUnregisteredCustomResourceCachingAgent(
   public Collection<AgentDataType> getProvidedDataTypes() {
     return Collections.unmodifiableSet(
         primaryKinds().stream()
-            .filter(credentials::isValidKind)
             .map(k -> AUTHORITATIVE.forType(k.toString()))
             .collect(Collectors.toSet()));
   }

diff --git a/...m/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesV2CachingAgent.java b/...m/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesV2CachingAgent.java
@@ -75,18 +75,19 @@ protected Map<String, Object> defaultIntrospectionDetails() {
   protected abstract List<KubernetesKind> primaryKinds();
 
   protected Map<KubernetesKind, List<KubernetesManifest>> loadPrimaryResourceList() {
+    List<KubernetesKind> primaryKinds = primaryKinds();
     Map<KubernetesKind, List<KubernetesManifest>> result =
         getNamespaces()
             .parallelStream()
             .map(
                 n -> {
                   try {
-                    return credentials.list(primaryKinds(), n);
+                    return credentials.list(primaryKinds, n);
                   } catch (KubectlException e) {
                     log.warn(
                         "{}: Failed to read kind {} from namespace {}: {}",
                         getAgentType(),
-                        primaryKinds(),
+                        primaryKinds,
                         n,
                         e.getMessage());
                     throw e;

diff --git a/...x/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesV2OnDemandCachingAgent.java b/...x/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesV2OnDemandCachingAgent.java
@@ -349,6 +349,7 @@ public Collection<Map> pendingOnDemandRequests(ProviderCache providerCache) {
       return Collections.emptyList();
     }
 
+    List<KubernetesKind> primaryKinds = primaryKinds();
     List<String> matchingKeys =
         providerCache.getIdentifiers(ON_DEMAND_TYPE).stream()
             .map(Keys::parseKey)
@@ -359,7 +360,7 @@ public Collection<Map> pendingOnDemandRequests(ProviderCache providerCache) {
             .filter(
                 i ->
                     i.getAccount().equals(getAccountName())
-                        && primaryKinds().contains(i.getKubernetesKind()))
+                        && primaryKinds.contains(i.getKubernetesKind()))
             .map(Keys.InfrastructureCacheKey::toString)
             .collect(Collectors.toList());
 

diff --git a/...ava/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java b/...ava/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java
@@ -26,6 +26,7 @@
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.common.base.Suppliers;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.netflix.spectator.api.Clock;
@@ -128,13 +129,15 @@ public class KubernetesV2Credentials implements KubernetesCredentials {
   @Include @Getter private final boolean debug;
 
   private String cachedDefaultNamespace;
-  private final Supplier<List<String>> liveNamespaceSupplier;
   @Getter private final ResourcePropertyRegistry resourcePropertyRegistry;
   @Getter private final KubernetesKindRegistry kindRegistry;
   private final KubernetesSpinnakerKindMap kubernetesSpinnakerKindMap;
   private final PermissionValidator permissionValidator;
   private final Supplier<ImmutableMap<KubernetesKind, KubernetesKindProperties>> crdSupplier =
       Suppliers.memoizeWithExpiration(this::crdSupplier, CRD_EXPIRY_SECONDS, TimeUnit.SECONDS);
+  private final Supplier<ImmutableList<String>> liveNamespaceSupplier =
+      Memoizer.memoizeWithExpiration(
+          this::namespaceSupplier, NAMESPACE_EXPIRY_SECONDS, TimeUnit.SECONDS);
 
   private KubernetesV2Credentials(
       Registry registry,
@@ -196,28 +199,6 @@ private KubernetesV2Credentials(
     this.metrics = managedAccount.isMetrics();
 
     this.debug = managedAccount.isDebug();
-
-    this.liveNamespaceSupplier =
-        Memoizer.memoizeWithExpiration(
-            () -> {
-              try {
-                return jobExecutor
-                    .list(
-                        this,
-                        Collections.singletonList(KubernetesKind.NAMESPACE),
-                        "",
-                        new KubernetesSelectorList())
-                    .stream()
-                    .map(KubernetesManifest::getName)
-                    .collect(Collectors.toList());
-              } catch (KubectlException e) {
-                log.error(
-                    "Could not list namespaces for account {}: {}", accountName, e.getMessage());
-                return new ArrayList<>();
-              }
-            },
-            NAMESPACE_EXPIRY_SECONDS,
-            TimeUnit.SECONDS);
   }
 
   /**
@@ -369,6 +350,24 @@ private ImmutableMap<KubernetesKind, KubernetesKindProperties> crdSupplier() {
     }
   }
 
+  @Nonnull
+  private ImmutableList<String> namespaceSupplier() {
+    try {
+      return jobExecutor
+          .list(
+              this,
+              Collections.singletonList(KubernetesKind.NAMESPACE),
+              "",
+              new KubernetesSelectorList())
+          .stream()
+          .map(KubernetesManifest::getName)
+          .collect(toImmutableList());
+    } catch (KubectlException e) {
+      log.error("Could not list namespaces for account {}: {}", accountName, e.getMessage());
+      return ImmutableList.of();
+    }
+  }
+
   @Override
   public List<String> getDeclaredNamespaces() {
     List<String> result;