Skip to content

Commit

Permalink
perf(kubernetes): Only create applications for some kinds (#3829)
Browse files Browse the repository at this point in the history
* perf(kubernetes): Use a more efficient data structure for cache data

The current logic to build the Kubernetes cache data first creates
a list of CacheData objects representing each of the objects to be
cached (including its attributes and relationships).

At the end of the caching process, we want only a single CacheData
object for each cache key and also all relationships to be
bidirectional.

Bidirectionality is currently enforced by calling a function
invertRelationships to look up all relationships and create entries
representing their inverses. Uniqueness is enforced by calling
dedupCacheData to detect duplicate entries and merge their attributes
and relationships.

Both invertRelationships and dedupCacheData are fairly expensive as they
involve traversing the entire list of relationships (which are stored on
CacheData as a generic Collection) and building new objects based on
these. These functions also need parse keys as they encounter them,
leading to each key being parsed multiple times.

It would be more efficient to store the entries and relationships in
a data structure that guarantees uniqueness and bidrectionality of
relationships (and can do so efficiently).  Create a class
KubernetesCacheData that stores cache items as we are processing them
and that supports adding items or (bidirectional) relationships between
items.  Update the caching logic to add items to this structure instead
of creating individual CacheData entries, and then ask the structure
to give us a full list of CacheData entries at the end.

* refactor(kubernetes): Remove duplicated code in cache conversion

For each manifest we encounter, we call both convertAsResource
to create an infrastructure cache key as well as convertAsArtifact
to create a logical cache key. There is fair amount of duplication
between the two methods.

Make the following changes:
(1) Move convertAsArtifact to be a called from convertAsResource
and remove the duplicated code
(2) Remove the code to add artifact relationships from
annotatedRelationships and instead add the necessary relationship
right after calling convertAsArtifact (if that call produced an
artifact).  This avoids needing to generate the artifact twice
(once for the actual cache entry and once for the relationship)
and also keeps the artifact code in one place.

* perf(kubernetes): Only create applications for some kinds

After generating all of the cache data keys, we currently call
getClusterRelationships to populate application-cluster
relationships. This call ends up creating an application for
every kubernetes manifest instead of only those that have a
cluster relationship.

Instead of calling getClusterRelationships after generating the
CacheData entries, update the code that creates the manifest-cluster
and manifest-application relationships to also create an
application-cluster relationship.

In addition to increasing performance of caching itself, this change
should also greatly improve the performance of some cache lookups
(such as the search endpoint) as it will greatly reduce the number
of applications in the cache.

* refactor(kubernetes): Remove unused functions

Remove functions that are now unused after the changes here.
  • Loading branch information
ezimanyi committed Jul 1, 2019
1 parent a6f7fd3 commit bc4a064
Show file tree
Hide file tree
Showing 8 changed files with 336 additions and 270 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.stream.Collectors;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Slf4j
Expand Down Expand Up @@ -162,6 +163,7 @@ private static CacheKey parseLogicalKey(String[] parts) {
}
}

@EqualsAndHashCode
public abstract static class CacheKey {
private static final String provider = "kubernetes.v2";

Expand All @@ -180,6 +182,7 @@ public abstract static class LogicalKey extends CacheKey {

@EqualsAndHashCode(callSuper = true)
@Getter
@RequiredArgsConstructor
public static class ArtifactCacheKey extends CacheKey {
@Getter private static final Kind kind = Kind.ARTIFACT;
private final String type;
Expand Down Expand Up @@ -215,6 +218,7 @@ public String getGroup() {

@EqualsAndHashCode(callSuper = true)
@Getter
@RequiredArgsConstructor
public static class ApplicationCacheKey extends LogicalKey {
private static final LogicalKind logicalKind = LogicalKind.APPLICATIONS;
private final String name;
Expand Down Expand Up @@ -249,6 +253,7 @@ public String getGroup() {

@EqualsAndHashCode(callSuper = true)
@Getter
@RequiredArgsConstructor
public static class ClusterCacheKey extends LogicalKey {
private static final LogicalKind logicalKind = LogicalKind.CLUSTERS;
private final String account;
Expand Down Expand Up @@ -287,6 +292,7 @@ public String getGroup() {

@EqualsAndHashCode(callSuper = true)
@Getter
@RequiredArgsConstructor
public static class InfrastructureCacheKey extends CacheKey {
@Getter private static final Kind kind = Kind.INFRASTRUCTURE;
private final KubernetesKind kubernetesKind;
Expand All @@ -306,6 +312,10 @@ public InfrastructureCacheKey(String[] parts) {
name = parts[5];
}

public InfrastructureCacheKey(KubernetesManifest manifest, String account) {
this(manifest.getKind(), account, manifest.getNamespace(), manifest.getName());
}

public static String createKey(
KubernetesKind kubernetesKind, String account, String namespace, String name) {
return createKeyFromParts(kind, kubernetesKind, account, namespace, name);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* Copyright 2019 Google, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License")
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.netflix.spinnaker.clouddriver.kubernetes.v2.caching.agent;

import com.netflix.spinnaker.cats.cache.CacheData;
import com.netflix.spinnaker.cats.cache.DefaultCacheData;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.caching.Keys;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import lombok.Value;

/**
* A collection of CacheItem entries used when building up the items being cached by the Kubernetes
* V2 caching agent. This class supports adding items as well as adding relationships between items.
*
* <p>Once all cache items and relationships have been added, calling toCacheData() will return a
* Collection of CacheData entries that represent all added items and their relationships. The
* operations supported on the class guarantee that the resulting Collection&lt;CacheData&gt; has
* the following properties: (1) Each CacheData has a unique cache key, (2) all relationships
* between CacheData items are bidirectional
*/
public class KubernetesCacheData {
private final Map<Keys.CacheKey, CacheItem> items = new HashMap<>();

/**
* Add an item to the cache with specified key and attributes. If there is already an item with
* the given key, the attributes are merged into the existing item's attributes (with the input
* attributes taking priority).
*/
public void addItem(Keys.CacheKey key, Map<String, Object> attributes) {
CacheItem item = items.computeIfAbsent(key, CacheItem::new);
item.getAttributes().putAll(attributes);
}

/**
* Add a bidirectional relationship between two keys. If either of the keys is not yet in the
* cache, an entry is created for that item with an empty map of attributes.
*/
public void addRelationship(Keys.CacheKey a, Keys.CacheKey b) {
items.computeIfAbsent(a, CacheItem::new).getRelationships().add(b);
items.computeIfAbsent(b, CacheItem::new).getRelationships().add(a);
}

/**
* Add a bidirectional relationships between key a and each of the keys in the Set b. If any of
* the encountered keys is not in the cache, an entry is created for that item with an empty map
* of attributes
*/
public void addRelationships(Keys.CacheKey a, Set<Keys.CacheKey> b) {
items.computeIfAbsent(a, CacheItem::new).getRelationships().addAll(b);
b.forEach(k -> items.computeIfAbsent(k, CacheItem::new).getRelationships().add(a));
}

/** Return a List of CacheData entries representing the current items in the cache. */
public List<CacheData> toCacheData() {
return items.values().stream().map(CacheItem::toCacheData).collect(Collectors.toList());
}

/**
* An item being cached by the Kubernetes V2 provider. This corresponds to a CacheData entry, but
* stores the information in a format that is more efficient to manipulate as we build up the
* cache data.
*
* <p>In particular:the cache key is stored as a Keys.CacheKey object (rather than serialized) so
* we can access properties of the key without re-parsing it and the relationships are stored as a
* flat Set&lt;Keys.CacheKey&gt; instead of as a Map&lt;String, Collection&lt;String&gt;&gt; so
* that we can efficiently add relationships.
*
* <p>A CacheItem can be converted to its corresponding CacheData by calling toCacheData()
*/
@Value
private static class CacheItem {
private final Keys.CacheKey key;
private final Map<String, Object> attributes = new HashMap<>();
private final Set<Keys.CacheKey> relationships = new HashSet<>();

private Map<String, Collection<String>> groupedRelationships() {
Map<String, Collection<String>> groups = new HashMap<>();
for (KubernetesKind kind : KubernetesCacheDataConverter.getStickyKinds()) {
groups.put(kind.toString(), new HashSet<>());
}
for (Keys.CacheKey key : relationships) {
groups.computeIfAbsent(key.getGroup(), k -> new HashSet<>()).add(key.toString());
}
return groups;
}

/** Convert this CacheItem to its corresponding CacheData object */
public CacheData toCacheData() {
int ttlSeconds;
if (Keys.LogicalKind.isLogicalGroup(key.getGroup())) {
// If we are inverting a relationship to create a cache data for either a cluster or an
// application we
// need to insert attributes to ensure the cache data gets entered into the cache.
attributes.putIfAbsent("name", key.getName());
ttlSeconds = KubernetesCacheDataConverter.getLogicalTtlSeconds();
} else {
ttlSeconds = KubernetesCacheDataConverter.getInfrastructureTtlSeconds();
}
return new DefaultCacheData(key.toString(), ttlSeconds, attributes, groupedRelationships());
}
}
}
Loading

0 comments on commit bc4a064

Please sign in to comment.