zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-07-23 19:20:44 +02:00
Code Issues Projects Releases Packages Wiki Activity

Fix Cloud Tasks retry failure (#2118)

Browse source 
* Fix Cloud Tasks failure to retry

Replace `SC_NOT_MODIFIED` (304) with `SC_SERVICE_UNAVAILABLE` (503) when
data is not available yet. Affected actions are invoice- and spec11-publishing.

It is confirmed that Cloud Tasks currently does not retry with code 304,
despite the public documentation stating so. We will use 503 for now,
pending the decision by Cloud Tasks whether to change behavior or
documentation.

The code `TOO_EARLY` (425) is another alternative. It is not meant for
our use case but at least sounds like it is. However, it is not in any
javax.servlet jar. We don't want to define our own constant, and we cannot upgrade
to jakarta.servlet yet.

Also revert previous mitigation.
This commit is contained in:
Weimin Yu 2023-08-23 10:55:51 -04:00 • committed by GitHub
parent 97676d1a1f
commit ffd952a60e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 10 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

4
core/src/main/java/google/registry/reporting/billing/PublishInvoicesAction.java
View file

@ -17,9 +17,9 @@ package google.registry.reporting.billing;
import static google.registry.reporting.ReportingModule.PARAM_YEAR_MONTH;
import static google.registry.request.Action.Method.POST;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import com.google.api.services.dataflow.Dataflow;
import com.google.api.services.dataflow.model.Job;
@ -111,7 +111,7 @@ public class PublishInvoicesAction implements Runnable {
break;
default:
logger.atInfo().log("Job in non-terminal state %s, retrying:", state);
response.setStatus(SC_NOT_MODIFIED);
response.setStatus(SC_SERVICE_UNAVAILABLE);
break;
}
} catch (IOException e) {

3
core/src/main/java/google/registry/reporting/spec11/GenerateSpec11ReportAction.java
View file

@ -141,8 +141,7 @@ public class GenerateSpec11ReportAction implements Runnable {
jobId,
ReportingModule.PARAM_DATE,
date.toString()),
// TODO(b/296582836): mitigating retry problem. Remove `+10` when bug is fixed.
Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES + 10)));
Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES)));
}
response.setStatus(SC_OK);
response.setPayload(String.format("Launched Spec11 pipeline: %s", jobId));

4
core/src/main/java/google/registry/reporting/spec11/PublishSpec11ReportAction.java
View file

@ -18,9 +18,9 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.reporting.ReportingModule.PARAM_DATE;
import static google.registry.request.Action.Method.POST;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import com.google.api.services.dataflow.Dataflow;
import com.google.api.services.dataflow.model.Job;
@ -135,7 +135,7 @@ public class PublishSpec11ReportAction implements Runnable {
break;
default:
logger.atInfo().log("Job in non-terminal state %s, retrying:", state);
response.setStatus(SC_NOT_MODIFIED);
response.setStatus(SC_SERVICE_UNAVAILABLE);
break;
}
} catch (IOException | JSONException e) {

4
core/src/test/java/google/registry/reporting/billing/PublishInvoicesActionTest.java
View file

@ -16,9 +16,9 @@ package google.registry.reporting.billing;
import static com.google.common.truth.Truth.assertThat;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ -101,7 +101,7 @@ class PublishInvoicesActionTest {
void testJobIndeterminate_returnsRetriableResponse() {
expectedJob.setCurrentState("JOB_STATE_RUNNING");
uploadAction.run();
assertThat(response.getStatus()).isEqualTo(SC_NOT_MODIFIED);
assertThat(response.getStatus()).isEqualTo(SC_SERVICE_UNAVAILABLE);
}
@Test

3
core/src/test/java/google/registry/reporting/spec11/GenerateSpec11ReportActionTest.java
View file

@ -93,8 +93,7 @@ class GenerateSpec11ReportActionTest extends BeamActionTestBase {
.scheduleTime(
clock
.nowUtc()
// TODO(b/296582836): mitigating retry problem. Remove `+10` when bug is fixed.
.plus(Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES + 10))));
.plus(Duration.standardMinutes(ReportingModule.ENQUEUE_DELAY_MINUTES))));
}
@Test

4
core/src/test/java/google/registry/reporting/spec11/PublishSpec11ReportActionTest.java
View file

@ -19,9 +19,9 @@ import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParse
import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchB;
import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.sampleThreatMatches;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@ -134,7 +134,7 @@ class PublishSpec11ReportActionTest {
void testJobIndeterminate_returnsRetriableResponse() {
expectedJob.setCurrentState("JOB_STATE_RUNNING");
publishAction.run();
assertThat(response.getStatus()).isEqualTo(SC_NOT_MODIFIED);
assertThat(response.getStatus()).isEqualTo(SC_SERVICE_UNAVAILABLE);
verifyNoMoreInteractions(emailUtils);
}