mirror of
https://github.com/google/nomulus
synced 2026-09-20 06:54:43 +00:00
Fix a subtle issue in BRDA copy caused by Cloud Tasks (#1556)
* Fix a subtle issue in BRDA copy caused by Cloud Tasks After the Cloud Tasks migration and #1508, the BRDA copy job now routinely fail on the first try because the revision update is not commited by the time the Cloud Tasks job enqueued in the same transaction runs for the first time. This is because the enqueueing is a side effect and not part of the transaction. The job eventually succeeds because of retries. This PR attempts to mitigate the initial failure by adding a delay to the enqueued job, and checking the cursor in the job itself to prevent it from running before the transaction is commited.
This commit is contained in:
@@ -16,9 +16,13 @@ package google.registry.rde;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.common.truth.Truth.assertWithMessage;
|
||||
import static google.registry.model.common.Cursor.CursorType.BRDA;
|
||||
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
|
||||
import static google.registry.testing.DatabaseHelper.createTld;
|
||||
import static google.registry.testing.DatabaseHelper.persistResource;
|
||||
import static google.registry.testing.SystemInfo.hasCommand;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assumptions.assumeTrue;
|
||||
|
||||
import com.google.cloud.storage.BlobId;
|
||||
@@ -28,8 +32,11 @@ import com.google.common.io.CharStreams;
|
||||
import com.google.common.io.Files;
|
||||
import google.registry.gcs.GcsUtils;
|
||||
import google.registry.keyring.api.Keyring;
|
||||
import google.registry.model.common.Cursor;
|
||||
import google.registry.model.rde.RdeMode;
|
||||
import google.registry.model.rde.RdeRevision;
|
||||
import google.registry.model.tld.Registry;
|
||||
import google.registry.request.HttpException.NoContentException;
|
||||
import google.registry.testing.AppEngineExtension;
|
||||
import google.registry.testing.BouncyCastleProviderExtension;
|
||||
import google.registry.testing.FakeKeyringModule;
|
||||
@@ -104,6 +111,7 @@ public class BrdaCopyActionTest {
|
||||
|
||||
@BeforeEach
|
||||
void beforeEach() throws Exception {
|
||||
createTld("lol");
|
||||
action.gcsUtils = gcsUtils;
|
||||
action.tld = "lol";
|
||||
action.watermark = DateTime.parse("2010-10-17TZ");
|
||||
@@ -116,6 +124,20 @@ public class BrdaCopyActionTest {
|
||||
() -> {
|
||||
RdeRevision.saveRevision("lol", DateTime.parse("2010-10-17TZ"), RdeMode.THIN, 0);
|
||||
});
|
||||
persistResource(Cursor.create(BRDA, action.watermark.plusDays(1), Registry.get("lol")));
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(strings = {"", "job-name/"})
|
||||
void testRun_stagingNotFinished_throws204(String prefix) throws Exception {
|
||||
persistResource(Cursor.create(BRDA, action.watermark, Registry.get("lol")));
|
||||
NoContentException thrown = assertThrows(NoContentException.class, () -> runAction(prefix));
|
||||
assertThat(thrown)
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
"Waiting on RdeStagingAction for TLD lol to copy BRDA deposit for"
|
||||
+ " 2010-10-17T00:00:00.000Z to GCS; last BRDA staging completion was before"
|
||||
+ " 2010-10-17T00:00:00.000Z");
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
|
||||
@@ -403,7 +403,7 @@ public class RdeUploadActionTest {
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
"Waiting on RdeStagingAction for TLD tld to send 2010-10-17T00:00:00.000Z upload; last"
|
||||
+ " RDE staging completion was at 1970-01-01T00:00:00.000Z");
|
||||
+ " RDE staging completion was before 1970-01-01T00:00:00.000Z");
|
||||
cloudTasksHelper.assertNoTasksEnqueued("rde-upload");
|
||||
assertThat(folder.list()).isEmpty();
|
||||
}
|
||||
@@ -420,7 +420,7 @@ public class RdeUploadActionTest {
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
"Waiting on RdeStagingAction for TLD tld to send 2010-10-17T00:00:00.000Z upload; "
|
||||
+ "last RDE staging completion was at 2010-10-17T00:00:00.000Z");
|
||||
+ "last RDE staging completion was before 2010-10-17T00:00:00.000Z");
|
||||
}
|
||||
|
||||
@TestOfyAndSql
|
||||
@@ -437,8 +437,9 @@ public class RdeUploadActionTest {
|
||||
assertThat(thrown)
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
"Waiting on 120 minute SFTP cooldown for TLD tld to send 2010-10-17T00:00:00.000Z "
|
||||
+ "upload; last upload attempt was at 2010-10-16T22:23:00.000Z (97 minutes ago)");
|
||||
"Waiting on 120 minute SFTP cooldown for TLD tld to send 2010-10-17T00:00:00.000Z"
|
||||
+ " upload; last upload attempt was at 2010-10-16T22:23:00.000Z (97 minutes"
|
||||
+ " ago)");
|
||||
}
|
||||
|
||||
private String slurp(InputStream is) throws IOException {
|
||||
|
||||
Reference in New Issue
Block a user