Fixing transcription crashes caused by job stoppage
Apparently, scheduling a new job when one is already running (even using the enqueue api) causes the running job to be stopped. We weren't handling that case correctly. This cl makes sure no more work is attempted after a job is stopped by cancelling any
active transcription task. We request that stopped task be rescheduled by the job scheduler, so it will get run eventually.
I was able to verify this fix by sending a new voicemail while backfill old transcription tasks were running.
Bug: 64908823,63524274,65129734,63803709
Test: manual and unit tests
PiperOrigin-RevId: 167617191
Change-Id: Icc92997c2687e61bef9b3b7f9ff572da2cb4ed2e
diff --git a/java/com/android/dialer/logging/dialer_impression.proto b/java/com/android/dialer/logging/dialer_impression.proto
index ef249c2..94af6c3 100644
--- a/java/com/android/dialer/logging/dialer_impression.proto
+++ b/java/com/android/dialer/logging/dialer_impression.proto
@@ -530,5 +530,9 @@
IN_CALL_DIALPAD_NUMBER_BUTTON_PRESSED = 1265;
IN_CALL_DIALPAD_HANG_UP_BUTTON_PRESSED = 1266;
IN_CALL_DIALPAD_CLOSE_BUTTON_PRESSED = 1267;
+
+ // More voicemail transcription impressions
+ VVM_TRANSCRIPTION_JOB_STOPPED = 1268;
+ VVM_TRANSCRIPTION_TASK_CANCELLED = 1269;
}
}
diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
index 2ca16fb..b733928 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
@@ -49,6 +49,8 @@
private JobParameters jobParameters;
private TranscriptionClientFactory clientFactory;
private TranscriptionConfigProvider configProvider;
+ private TranscriptionTask activeTask;
+ private boolean stopped;
/** Callback used by a task to indicate it has finished processing its work item */
interface JobCallback {
@@ -134,8 +136,14 @@
@MainThread
public boolean onStopJob(JobParameters params) {
Assert.isMainThread();
- LogUtil.enterBlock("TranscriptionService.onStopJob");
- cleanup();
+ LogUtil.i("TranscriptionService.onStopJob", "params: " + params);
+ stopped = true;
+ Logger.get(this).logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_JOB_STOPPED);
+ if (activeTask != null) {
+ LogUtil.i("TranscriptionService.onStopJob", "cancelling active task");
+ activeTask.cancel();
+ Logger.get(this).logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_TASK_CANCELLED);
+ }
return true;
}
@@ -161,15 +169,20 @@
@MainThread
private boolean checkForWork() {
Assert.isMainThread();
+ if (stopped) {
+ LogUtil.i("TranscriptionService.checkForWork", "stopped");
+ return false;
+ }
JobWorkItem workItem = jobParameters.dequeueWork();
if (workItem != null) {
- TranscriptionTask task =
+ Assert.checkState(activeTask == null);
+ activeTask =
configProvider.shouldUseSyncApi()
? new TranscriptionTaskSync(
this, new Callback(), workItem, getClientFactory(), configProvider)
: new TranscriptionTaskAsync(
this, new Callback(), workItem, getClientFactory(), configProvider);
- getExecutorService().execute(task);
+ getExecutorService().execute(activeTask);
return true;
} else {
return false;
@@ -196,8 +209,13 @@
public void onWorkCompleted(JobWorkItem completedWorkItem) {
Assert.isMainThread();
LogUtil.i("TranscriptionService.Callback.onWorkCompleted", completedWorkItem.toString());
- jobParameters.completeWork(completedWorkItem);
- checkForWork();
+ activeTask = null;
+ if (stopped) {
+ LogUtil.i("TranscriptionService.Callback.onWorkCompleted", "stopped");
+ } else {
+ jobParameters.completeWork(completedWorkItem);
+ checkForWork();
+ }
}
}
diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java b/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java
index fbab076..60b97da 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionTask.java
@@ -19,8 +19,10 @@
import android.app.job.JobWorkItem;
import android.content.Context;
import android.net.Uri;
+import android.support.annotation.MainThread;
import android.text.TextUtils;
import android.util.Pair;
+import com.android.dialer.common.Assert;
import com.android.dialer.common.concurrent.ThreadUtil;
import com.android.dialer.compat.android.provider.VoicemailCompat;
import com.android.dialer.logging.DialerImpression;
@@ -64,6 +66,7 @@
protected final TranscriptionConfigProvider configProvider;
protected ByteString audioData;
protected AudioFormat encoding;
+ protected volatile boolean cancelled;
static final String AMR_PREFIX = "#!AMR\n";
@@ -87,6 +90,13 @@
databaseHelper = new TranscriptionDbHelper(context, voicemailUri);
}
+ @MainThread
+ void cancel() {
+ Assert.isMainThread();
+ VvmLog.i(TAG, "cancel");
+ cancelled = true;
+ }
+
@Override
public void run() {
VvmLog.i(TAG, "run");
@@ -144,7 +154,11 @@
.logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_RESPONSE_EXPIRED);
break;
default:
- updateTranscriptionAndState(transcript, VoicemailCompat.TRANSCRIPTION_FAILED);
+ updateTranscriptionAndState(
+ transcript,
+ cancelled
+ ? VoicemailCompat.TRANSCRIPTION_NOT_STARTED
+ : VoicemailCompat.TRANSCRIPTION_FAILED);
Logger.get(context).logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_RESPONSE_EMPTY);
break;
}
@@ -155,6 +169,11 @@
VvmLog.i(TAG, "sendRequest");
TranscriptionClient client = clientFactory.getClient();
for (int i = 0; i < configProvider.getMaxTranscriptionRetries(); i++) {
+ if (cancelled) {
+ VvmLog.i(TAG, "sendRequest, cancelled");
+ return null;
+ }
+
VvmLog.i(TAG, "sendRequest, try: " + (i + 1));
if (i == 0) {
Logger.get(context).logImpression(getRequestSentImpression());
@@ -163,7 +182,10 @@
}
TranscriptionResponse response = request.getResponse(client);
- if (response.hasRecoverableError()) {
+ if (cancelled) {
+ VvmLog.i(TAG, "sendRequest, cancelled");
+ return null;
+ } else if (response.hasRecoverableError()) {
Logger.get(context)
.logImpression(DialerImpression.Type.VVM_TRANSCRIPTION_RESPONSE_RECOVERABLE_ERROR);
backoff(i);
@@ -187,7 +209,7 @@
try {
Thread.sleep(millis);
} catch (InterruptedException e) {
- VvmLog.w(TAG, "interrupted");
+ VvmLog.e(TAG, "interrupted", e);
Thread.currentThread().interrupt();
}
}
diff --git a/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java b/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java
index 3c41aef..930d7f1 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java
@@ -62,7 +62,10 @@
(TranscriptionResponseAsync)
sendRequest((client) -> client.sendUploadRequest(getUploadRequest()));
- if (uploadResponse == null) {
+ if (cancelled) {
+ VvmLog.i(TAG, "getTranscription, cancelled.");
+ return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY);
+ } else if (uploadResponse == null) {
VvmLog.i(TAG, "getTranscription, failed to upload voicemail.");
return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY);
} else {
@@ -87,10 +90,17 @@
VvmLog.i(TAG, "pollForTranscription");
GetTranscriptRequest request = getGetTranscriptRequest(uploadResponse);
for (int i = 0; i < configProvider.getMaxGetTranscriptPolls(); i++) {
+ if (cancelled) {
+ VvmLog.i(TAG, "pollForTranscription, cancelled.");
+ return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY);
+ }
GetTranscriptResponseAsync response =
(GetTranscriptResponseAsync)
sendRequest((client) -> client.sendGetTranscriptRequest(request));
- if (response == null) {
+ if (cancelled) {
+ VvmLog.i(TAG, "pollForTranscription, cancelled.");
+ return new Pair<>(null, TranscriptionStatus.FAILED_NO_RETRY);
+ } else if (response == null) {
VvmLog.i(TAG, "pollForTranscription, no transcription result.");
} else if (response.isTranscribing()) {
VvmLog.i(TAG, "pollForTranscription, poll count: " + (i + 1));