internal: track starting of tasks

Update exported TaskState to include a "started" boolean. Record the
task started state in the database and show a different icon in the UI.

Tracking started enables several improvements. This change improves
resuming of workflows to mark tasks that started without finishing as
incomplete. This will prevent workflows resuming from an unknown state,
where a task may not be safe to resume by default, such as sending a
public announcement.

Also, this change makes minor tweaks to the UI for task ordering,
putting the most recently updated tasks at the top of the list. It also
fixes a minor but confusing bug around retrying "approval" tasks.

Updates golang/go#53382

Change-Id: Icff1c0df541a6e11a7bb0ab55beef60cd18a074a
Reviewed-on: https://go-review.googlesource.com/c/build/+/417221
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Jenny Rakoczy <[email protected]>
Auto-Submit: Jenny Rakoczy <[email protected]>

internal/relui/listener.go

@@ -45,6 +45,7 @@ func (l *PGListener) TaskStateChanged(workflowID uuid.UUID, taskName string, sta
 		_, err := q.UpsertTask(ctx, db.UpsertTaskParams{
 			WorkflowID: workflowID,
 			Name:       taskName,
+			Started:    state.Started,
 			Finished:   state.Finished,
 			Result:     sql.NullString{String: string(result), Valid: len(result) > 0},
 			Error:      sql.NullString{String: state.Error, Valid: state.Error != ""},

internal/relui/migrations/20220712205734_add_started_to_tasks.down.sql

@@ -0,0 +1,6 @@
+-- Copyright 2022 The Go Authors. All rights reserved.
+-- Use of this source code is governed by a BSD-style
+-- license that can be found in the LICENSE file.
+
+ALTER TABLE tasks
+    DROP COLUMN started;

internal/relui/migrations/20220712205734_add_started_to_tasks.up.sql

@@ -0,0 +1,6 @@
+-- Copyright 2022 The Go Authors. All rights reserved.
+-- Use of this source code is governed by a BSD-style
+-- license that can be found in the LICENSE file.
+
+ALTER TABLE tasks
+    ADD COLUMN started bool NOT NULL DEFAULT FALSE;

internal/relui/queries/workflows.sql

@@ -18,26 +18,36 @@ VALUES ($1, $2, $3, $4, $5)
 RETURNING *;
 
 -- name: CreateTask :one
-INSERT INTO tasks (workflow_id, name, finished, result, error, created_at, updated_at, approved_at, ready_for_approval)
+INSERT INTO tasks (workflow_id, name, finished, result, error, created_at, updated_at, approved_at,
+                   ready_for_approval)
 VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)
 RETURNING *;
 
 -- name: UpsertTask :one
-INSERT INTO tasks (workflow_id, name, finished, result, error, created_at, updated_at)
-VALUES ($1, $2, $3, $4, $5, $6, $7)
+INSERT INTO tasks (workflow_id, name, started, finished, result, error, created_at, updated_at)
+VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
 ON CONFLICT (workflow_id, name) DO UPDATE
     SET workflow_id = excluded.workflow_id,
         name        = excluded.name,
+        started     = excluded.started,
         finished    = excluded.finished,
         result      = excluded.result,
         error       = excluded.error,
         updated_at  = excluded.updated_at
 RETURNING *;
 
 -- name: Tasks :many
-SELECT tasks.*
+WITH most_recent_logs AS (
+    SELECT workflow_id, task_name, MAX(updated_at) AS updated_at
+    FROM task_logs
+    GROUP BY workflow_id, task_name
+)
+SELECT tasks.*,
+       GREATEST(most_recent_logs.updated_at, tasks.updated_at)::timestamptz AS most_recent_update
 FROM tasks
-ORDER BY updated_at;
+LEFT JOIN most_recent_logs ON tasks.workflow_id = most_recent_logs.workflow_id AND
+                              tasks.name = most_recent_logs.task_name
+ORDER BY most_recent_update DESC;
 
 -- name: TasksForWorkflow :many
 SELECT tasks.*
@@ -85,10 +95,12 @@ RETURNING *;
 
 -- name: ResetTask :one
 UPDATE tasks
-SET finished   = FALSE,
-    result     = DEFAULT,
-    error      = DEFAULT,
-    updated_at = $3
+SET finished    = FALSE,
+    started     = FALSE,
+    approved_at = DEFAULT,
+    result      = DEFAULT,
+    error       = DEFAULT,
+    updated_at  = $3
 WHERE workflow_id = $1
   AND name = $2
 RETURNING *;

internal/relui/templates/task_list.html

@@ -37,11 +37,16 @@
                 class="TaskList-itemStateIcon"
                 alt="finished"
                 src="{{baseLink "/static/images/check_circle_green_24dp.svg"}}" />
+            {{else if $task.Started}}
+              <img
+                class="TaskList-itemStateIcon"
+                alt="started"
+                src="{{baseLink "/static/images/pending_yellow_24dp.svg"}}" />
             {{else}}
               <img
                 class="TaskList-itemStateIcon"
                 alt="pending"
-                src="{{baseLink "/static/images/pending_yellow_24dp.svg"}}" />
+                src="{{baseLink "/static/images/pending_grey_24dp.svg"}}" />
             {{end}}
           </td>
           <td class="TaskList-itemCol TaskList-itemName">
@@ -51,7 +56,7 @@
             {{$task.CreatedAt.UTC.Format "Mon Jan _2 2006 15:04:05"}}
           </td>
           <td class="TaskList-itemCol TaskList-itemUpdated">
-            {{$task.UpdatedAt.UTC.Format "Mon Jan _2 2006 15:04:05"}}
+            {{$task.MostRecentUpdate.UTC.Format "Mon Jan _2 2006 15:04:05"}}
           </td>
           <td class="TaskList-itemCol TaskList-itemResult">
             {{if $task.ApprovedAt.Valid}}
@@ -68,8 +73,7 @@
                   <input class="Button Button--small" name="task.reset" type="submit" value="Retry" onclick="return this.form.reportValidity() && confirm('This will retry the task and clear workflow errors.\n\nReady to proceed?')" />
                 </form>
               </div>
-            {{end}}
-            {{if and (not $task.ApprovedAt.Valid) ($task.ReadyForApproval)}}
+            {{else if and (not $task.ApprovedAt.Valid) ($task.ReadyForApproval)}}
               <div class="TaskList-approveTask">
                 <form action="{{baseLink (printf "/workflows/%s/tasks/%s/approve" $workflow.ID $task.Name)}}" method="post">
                   <input type="hidden" id="workflow.id" name="workflow.id" value="{{$workflow.ID}}" />

internal/relui/web.go

@@ -139,7 +139,7 @@ func (s *Server) BaseLink(target string) string {
 
 type workflowDetail struct {
 	Workflow db.Workflow
-	Tasks    []db.Task
+	Tasks    []db.TasksRow
 	// TaskLogs is a map of all logs for a db.Task, keyed on
 	// (db.Task).Name
 	TaskLogs map[string][]db.TaskLog

internal/relui/worker.go

@@ -192,14 +192,25 @@ func (w *Worker) Resume(ctx context.Context, id uuid.UUID) error {
 	}
 	taskStates := make(map[string]*workflow.TaskState)
 	for _, t := range tasks {
-		taskStates[t.Name] = &workflow.TaskState{
+		ts := &workflow.TaskState{
 			Name:     t.Name,
 			Finished: t.Finished,
 			Error:    t.Error.String,
 		}
+		// The worker may have crashed, or been re-deployed. Any
+		// started but unfinished tasks are in an unknown state.
+		// Mark them as such for human review.
+		if t.Started && !t.Finished {
+			ts.Finished = true
+			ts.Error = "task interrupted before completion"
+			if t.Error.Valid {
+				ts.Error = fmt.Sprintf("%s. Previous error: %s", ts.Error, t.Error.String)
+			}
+		}
 		if t.Result.Valid {
-			taskStates[t.Name].SerializedResult = []byte(t.Result.String)
+			ts.SerializedResult = []byte(t.Result.String)
 		}
+		taskStates[t.Name] = ts
 	}
 	res, err := workflow.Resume(d, state, taskStates)
 	if err != nil {