fix(redis): Broken test with perf refactor (spinnaker#2269)

nscmcgrath · May 25, 2018 · d182a39 · d182a39
1 parent 0bd1ccc
commit d182a39
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 15 deletions.
diff --git a/...com/netflix/spinnaker/orca/pipeline/persistence/PollingAgentExecutionRepositoryTck.groovy b/...com/netflix/spinnaker/orca/pipeline/persistence/PollingAgentExecutionRepositoryTck.groovy
@@ -44,7 +44,7 @@ abstract class PollingAgentExecutionRepositoryTck<T extends PollingAgentExecutio
 
   abstract T createExecutionRepositoryPrevious()
 
-  def "can retrieve all application names in database"() {
+  def "can retrieve all application names in database, type: #executionType, min: #minExecutions"() {
     given:
     def execution1 = pipeline {
       application = "spindemo"

diff --git a/.../java/com/netflix/spinnaker/orca/pipeline/persistence/jedis/RedisExecutionRepository.java b/.../java/com/netflix/spinnaker/orca/pipeline/persistence/jedis/RedisExecutionRepository.java
@@ -26,7 +26,10 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.stereotype.Component;
-import redis.clients.jedis.*;
+import redis.clients.jedis.BinaryClient;
+import redis.clients.jedis.Response;
+import redis.clients.jedis.ScanParams;
+import redis.clients.jedis.ScanResult;
 import rx.Observable;
 import rx.Scheduler;
 import rx.functions.Func0;
@@ -571,7 +574,7 @@ public List<String> retrieveAllApplicationNames(@Nullable ExecutionType type, in
       ScanParams scanParams = new ScanParams().match(executionKeyPattern(type)).count(2000);
       String cursor = "0";
 
-      List<String> apps = new ArrayList<>();
+      Map<String, Long> apps = new HashMap<>();
       while (true) {
         String finalCursor = cursor;
         ScanResult<String> chunk = mc.scan(finalCursor, scanParams);
@@ -587,23 +590,22 @@ public List<String> retrieveAllApplicationNames(@Nullable ExecutionType type, in
             p.sync();
           });
 
-          apps.addAll(
-            pipelineResults
-              .entrySet()
-              .stream()
-              .filter(it -> it.getValue().get() >= minExecutions)
-              .map(Map.Entry::getKey)
-              .collect(Collectors.toList())
-          );
+          pipelineResults
+            .entrySet()
+            .forEach(e -> apps.compute(
+              e.getKey(),
+              (app, numExecutions) -> Optional.ofNullable(numExecutions).orElse(0L) + e.getValue().get())
+            );
         } else {
           redisClientDelegate.withCommandsClient(cc -> {
             chunk.getResult().forEach(id -> {
               String[] parts = id.split(":");
               String app = parts[2];
               long cardinality = cc.scard(id);
-              if (cardinality >= minExecutions) {
-                apps.add(app);
-              }
+              apps.compute(
+                app,
+                (appKey, numExecutions) -> Optional.ofNullable(numExecutions).orElse(0L) + cardinality
+              );
             });
           });
         }
@@ -614,7 +616,11 @@ public List<String> retrieveAllApplicationNames(@Nullable ExecutionType type, in
         }
       }
 
-      return apps;
+      return apps.entrySet()
+        .stream()
+        .filter(e -> e.getValue().intValue() >= minExecutions)
+        .map(Map.Entry::getKey)
+        .collect(Collectors.toList());
     });
   }