Merge pull request #4042 from vespa-engine/bratseth/job-run-reason-tweaks

Bratseth/job run reason tweaks

9 files changed, 37 insertions, 38 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java
index 379116a5d94..50798a82b60 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ContainerThreadFactory.java
@@ -8,7 +8,7 @@ import com.yahoo.jdisc.application.MetricConsumer;
 import java.util.concurrent.ThreadFactory;
 
 /**
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a>
+ * @author Simon Thoresen Hult
  */
 public class ContainerThreadFactory implements ThreadFactory {
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index 2cd273c82f3..3d2c2ed0319 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -103,16 +103,13 @@ public class DeploymentTrigger {
             // Trigger next
             if (report.success())
                 application = trigger(order.nextAfter(report.jobType(), application), application,
-                                      String.format("%s completed successfully in build %d",
-                                                    report.jobType(), report.buildNumber()));
+                                      report.jobType().jobName() + " completed");
             else if (isCapacityConstrained(report.jobType()) && shouldRetryOnOutOfCapacity(application, report.jobType()))
                 application = trigger(report.jobType(), application, true,
-                                      String.format("Retrying due to out of capacity in build %d",
-                                                    report.buildNumber()));
+                                      "Retrying on out of capacity");
             else if (shouldRetryNow(application))
                 application = trigger(report.jobType(), application, false,
-                                      String.format("Retrying as build %d just started failing",
-                                                    report.buildNumber()));
+                                      "Immediate retry on failure");
 
             applications().store(application);
         }
@@ -164,7 +161,7 @@ public class DeploymentTrigger {
                     nextToTrigger.add(nextJobType);
             }
             // Trigger them in parallel
-            application = trigger(nextToTrigger, application, "Triggering previously blocked jobs");
+            application = trigger(nextToTrigger, application, "Available change in " + jobType.jobName());
             controller.applications().store(application);
         }
     }
@@ -271,7 +268,7 @@ public class DeploymentTrigger {
             application = application.withDeploying(Optional.of(change));
             if (change instanceof Change.ApplicationChange)
                 application = application.withOutstandingChange(false);
-            application = trigger(JobType.systemTest, application, false, "Deploying change");
+            application = trigger(JobType.systemTest, application, false, "Deploying " + change);
             applications().store(application);
         }
     }
@@ -365,18 +362,18 @@ public class DeploymentTrigger {
      * @param jobType the type of the job to trigger, or null to trigger nothing
      * @param application the application to trigger the job for
      * @param first whether to trigger the job before other jobs
-     * @param cause describes why the job is triggered
+     * @param reason describes why the job is triggered
      * @return the application in the triggered state, which *must* be stored by the caller
      */
-    private LockedApplication trigger(JobType jobType, LockedApplication application, boolean first, String cause) {
+    private LockedApplication trigger(JobType jobType, LockedApplication application, boolean first, String reason) {
         if (isRunningProductionJob(application)) return application;
-        return triggerAllowParallel(jobType, application, first, false, cause);
+        return triggerAllowParallel(jobType, application, first, false, reason);
     }
 
-    private LockedApplication trigger(List<JobType> jobs, LockedApplication application, String cause) {
+    private LockedApplication trigger(List<JobType> jobs, LockedApplication application, String reason) {
         if (isRunningProductionJob(application)) return application;
         for (JobType job : jobs)
-            application = triggerAllowParallel(job, application, false, false, cause);
+            application = triggerAllowParallel(job, application, false, false, reason);
         return application;
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
index be30d0d4ac3..a92e2533092 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
@@ -114,7 +114,7 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler {
             // front of the queue
             application = controller.applications().deploymentTrigger().triggerAllowParallel(
                     jobType, application, true, true,
-                    "Triggered from the screwdriver/v1 web service"
+                    "Triggered from screwdriver/v1"
             );
             controller.applications().store(application);
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index f6a650e71be..7875c1f4964 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -176,7 +176,7 @@ public class ControllerTest {
         JobStatus jobStatus = applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1);
         assertNotNull("Deployment job was not removed", jobStatus);
         assertEquals(42, jobStatus.lastCompleted().get().id());
-        assertEquals("stagingTest completed successfully in build 42", jobStatus.lastCompleted().get().reason());
+        assertEquals("staging-test completed", jobStatus.lastCompleted().get().reason());
 
         // prod zone removal is allowed with override
         applicationPackage = new ApplicationPackageBuilder()
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
index fe9c373b7d5..a1bcc3eeb67 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
@@ -30,7 +30,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed successfully in build 42",
+        "reason": "component completed",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -44,7 +44,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed successfully in build 42",
+        "reason": "component completed",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -58,7 +58,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed successfully in build 42",
+        "reason": "component completed",
         "at": "(ignore)"
       }
     },
@@ -76,7 +76,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"systemTest completed successfully in build 42",
+        "reason":"system-test completed",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -90,7 +90,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"systemTest completed successfully in build 42",
+        "reason":"system-test completed",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -104,7 +104,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"systemTest completed successfully in build 42",
+        "reason":"system-test completed",
         "at": "(ignore)"
       }
     },
@@ -122,7 +122,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"stagingTest completed successfully in build 42",
+        "reason":"staging-test completed",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -136,7 +136,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"stagingTest completed successfully in build 42",
+        "reason":"staging-test completed",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -150,7 +150,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"stagingTest completed successfully in build 42",
+        "reason":"staging-test completed",
         "at": "(ignore)"
       }
     },
@@ -168,7 +168,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"productionUsWest1 completed successfully in build 42",
+        "reason":"production-us-west-1 completed",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -182,7 +182,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"productionUsWest1 completed successfully in build 42",
+        "reason":"production-us-west-1 completed",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -196,7 +196,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason":"productionUsWest1 completed successfully in build 42",
+        "reason":"production-us-west-1 completed",
         "at": "(ignore)"
       }
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
index 3dca8103ed7..f399d6c9188 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
@@ -63,7 +63,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "systemTest completed successfully in build 42",
+        "reason": "system-test completed",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -77,7 +77,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "systemTest completed successfully in build 42",
+        "reason": "system-test completed",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -91,7 +91,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "systemTest completed successfully in build 42",
+        "reason": "system-test completed",
         "at": "(ignore)"
       }
     },
@@ -109,7 +109,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Retrying as build 42 just started failing",
+        "reason": "Immediate retry on failure",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -123,7 +123,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "stagingTest completed successfully in build 42",
+        "reason": "staging-test completed",
         "at": "(ignore)"
       },
       "firstFailing": {
@@ -137,7 +137,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "stagingTest completed successfully in build 42",
+        "reason": "staging-test completed",
         "at": "(ignore)"
       }
     }
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java
index 4506a63ac6d..1e947d44fcd 100644
--- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerThread.java
@@ -22,8 +22,8 @@ public class ContainerThread extends Thread {
      * Allocates a new ContainerThread object. This constructor calls the parent {@link Thread#Thread(Runnable)}
      * constructor.
      *
-     * @param target   The object whose <code>run</code> method is called.
-     * @param consumer The MetricConsumer of this thread.
+     * @param target   the object whose <code>run</code> method is called.
+     * @param consumer the MetricConsumer of this thread.
      */
     public ContainerThread(Runnable target, MetricConsumer consumer) {
         super(target);
@@ -56,6 +56,7 @@ public class ContainerThread extends Thread {
         public Thread newThread(Runnable target) {
             return new ContainerThread(target, provider.get());
         }
+
     }
 
 }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 509bf42d466..7feca14ef29 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -324,8 +324,8 @@ public class JettyHttpServer extends AbstractServerProvider {
         return bundleContext.getService(ref);
     }
 
-    private static ExecutorService newJanitor(final ThreadFactory factory) {
-        final int threadPoolSize = Runtime.getRuntime().availableProcessors();
+    private static ExecutorService newJanitor(ThreadFactory factory) {
+        int threadPoolSize = Runtime.getRuntime().availableProcessors();
         log.info("Creating janitor executor with " + threadPoolSize + " threads");
         return Executors.newFixedThreadPool(
                 threadPoolSize,
diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java b/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java
index 4c15b6e2365..6c5dd5e3ba5 100644
--- a/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java
@@ -45,4 +45,5 @@ public class DaemonThreadFactory implements ThreadFactory {
         }
         return t;
     }
+
 }