Update task status first before rescheduling (flutter#3598)

Help flutter/flutter#145736
ricardoamador · Mar 26, 2024 · 98c9c09 · 98c9c09
1 parent 776e64c
commit 98c9c09
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 14 deletions.
diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart
@@ -692,6 +692,27 @@ class LuciBuildService {
     tags ??= <String, List<String>>{};
     tags['trigger_type'] ??= <String>['auto_retry'];
 
+    // Updating task status first to avoid endless rerun when datastore transaction aborts.
+    try {
+      // Updates task status in Datastore.
+      task.attempts = (task.attempts ?? 0) + 1;
+      // Mark task as in progress to ensure it isn't scheduled over
+      task.status = Task.statusInProgress;
+      await datastore.insert(<Task>[task]);
+
+      // Updates task status in Firestore.
+      final int newAttempt = int.parse(taskDocument.name!.split('_').last) + 1;
+      tags['current_attempt'] = <String>[newAttempt.toString()];
+      taskDocument.resetAsRetry(attempt: newAttempt);
+      taskDocument.setStatus(firestore.Task.statusInProgress);
+      final List<Write> writes = documentsToWrites([taskDocument], exists: false);
+      await firestoreService.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase);
+    } catch (error) {
+      log.severe(
+        'updating task ${taskDocument.taskName} of commit ${taskDocument.commitSha} failure: $error. Skipping rescheduling.',
+      );
+      return false;
+    }
     final BatchRequest request = BatchRequest(
       requests: <Request>[
         Request(
@@ -708,20 +729,6 @@ class LuciBuildService {
     );
     await pubsub.publish('scheduler-requests', request);
 
-    // Updates task status in Datastore.
-    task.attempts = (task.attempts ?? 0) + 1;
-    // Mark task as in progress to ensure it isn't scheduled over
-    task.status = Task.statusInProgress;
-    await datastore.insert(<Task>[task]);
-
-    // Updates task status in Firestore.
-    final int newAttempt = int.parse(taskDocument.name!.split('_').last) + 1;
-    tags['current_attempt'] = <String>[newAttempt.toString()];
-    taskDocument.resetAsRetry(attempt: newAttempt);
-    taskDocument.setStatus(firestore.Task.statusInProgress);
-    final List<Write> writes = documentsToWrites([taskDocument], exists: false);
-    await firestoreService.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase);
-
     return true;
   }
 

diff --git a/app_dart/test/service/luci_build_service_test.dart b/app_dart/test/service/luci_build_service_test.dart
@@ -16,6 +16,7 @@ import 'package:cocoon_service/src/model/luci/buildbucket.dart';
 import 'package:cocoon_service/src/model/luci/push_message.dart' as push_message;
 import 'package:cocoon_service/src/service/exceptions.dart';
 import 'package:cocoon_service/src/service/datastore.dart';
+import 'package:gcloud/datastore.dart';
 import 'package:github/github.dart';
 import 'package:googleapis/firestore/v1.dart' hide Status;
 import 'package:mockito/mockito.dart';
@@ -1058,6 +1059,39 @@ void main() {
       expect(rerunFlag, isTrue);
     });
 
+    test('Skip rerun a failed test when task status update hit exception', () async {
+      firestoreTask = generateFirestoreTask(1, attempts: 1, status: firestore.Task.statusInfraFailure);
+      when(
+        mockFirestoreService.batchWriteDocuments(
+          captureAny,
+          captureAny,
+        ),
+      ).thenAnswer((Invocation invocation) {
+        throw InternalError();
+      });
+      firestoreCommit = generateFirestoreCommit(1);
+      totCommit = generateCommit(1);
+      config.db.values[totCommit.key] = totCommit;
+      config.maxLuciTaskRetriesValue = 1;
+      final Task task = generateTask(
+        1,
+        status: Task.statusFailed,
+        parent: totCommit,
+        buildNumber: 1,
+      );
+      final Target target = generateTarget(1);
+      final bool rerunFlag = await service.checkRerunBuilder(
+        commit: totCommit,
+        task: task,
+        target: target,
+        datastore: datastore,
+        firestoreService: mockFirestoreService,
+        taskDocument: firestoreTask!,
+      );
+      expect(rerunFlag, isFalse);
+      expect(pubsub.messages.length, 0);
+    });
+
     test('Do not rerun a successful builder', () async {
       firestoreTask = generateFirestoreTask(1, attempts: 1);
       totCommit = generateCommit(1);