summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormdooley <mdooley@google.com>2017-09-05 12:48:03 -0700
committerEric Erfanian <erfanian@google.com>2017-09-11 10:57:47 -0700
commit9abbf7ff9b31dd3420fd2b304b63703b87d843f6 (patch)
treee69118e3b35ea6407d96ad4db3e6686efcc64ca8
parent2928203107c971ff8129f57addddb6b8fa19e065 (diff)
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
-rw-r--r--java/com/android/dialer/logging/dialer_impression.proto4
-rw-r--r--java/com/android/voicemail/impl/transcribe/TranscriptionService.java30
-rw-r--r--java/com/android/voicemail/impl/transcribe/TranscriptionTask.java28
-rw-r--r--java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java14
4 files changed, 65 insertions, 11 deletions
diff --git a/java/com/android/dialer/logging/dialer_impression.proto b/java/com/android/dialer/logging/dialer_impression.proto
index ef249c262..94af6c3fd 100644
--- a/java/com/android/dialer/logging/dialer_impression.proto
+++ b/java/com/android/dialer/logging/dialer_impression.proto
@@ -530,5 +530,9 @@ message DialerImpression {
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 2ca16fbf2..b733928d7 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionService.java
@@ -49,6 +49,8 @@ public class TranscriptionService extends JobService {
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 @@ public class TranscriptionService extends JobService {
@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 @@ public class TranscriptionService extends JobService {
@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 class TranscriptionService extends JobService {
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 fbab07655..60b97dad5 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.annotation.TargetApi;
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 @@ public abstract class TranscriptionTask implements Runnable {
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 @@ public abstract class TranscriptionTask implements Runnable {
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 @@ public abstract class TranscriptionTask implements Runnable {
.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 @@ public abstract class TranscriptionTask implements Runnable {
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 @@ public abstract class TranscriptionTask implements Runnable {
}
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 @@ public abstract class TranscriptionTask implements Runnable {
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 3c41aef89..930d7f113 100644
--- a/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java
+++ b/java/com/android/voicemail/impl/transcribe/TranscriptionTaskAsync.java
@@ -62,7 +62,10 @@ public class TranscriptionTaskAsync extends TranscriptionTask {
(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 @@ public class TranscriptionTaskAsync extends TranscriptionTask {
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));