diff --git a/java/google/registry/dns/BUILD b/java/google/registry/dns/BUILD index bf4302773..bdb5bc3d0 100644 --- a/java/google/registry/dns/BUILD +++ b/java/google/registry/dns/BUILD @@ -26,6 +26,7 @@ java_library( "//java/google/registry/monitoring/metrics", "//java/google/registry/request", "//java/google/registry/request/auth", + "//java/google/registry/request/lock", "//java/google/registry/util", "//third_party/java/objectify:objectify-v4_1", "@com_google_appengine_api_1_0_sdk", diff --git a/java/google/registry/dns/PublishDnsUpdatesAction.java b/java/google/registry/dns/PublishDnsUpdatesAction.java index 2dec6c7b7..6910d7879 100644 --- a/java/google/registry/dns/PublishDnsUpdatesAction.java +++ b/java/google/registry/dns/PublishDnsUpdatesAction.java @@ -14,7 +14,6 @@ package google.registry.dns; -import static google.registry.model.server.Lock.executeWithLocks; import static google.registry.request.Action.Method.POST; import static google.registry.request.RequestParameters.PARAM_TLD; import static google.registry.util.CollectionUtils.nullToEmpty; @@ -28,6 +27,7 @@ import google.registry.request.Action; import google.registry.request.HttpException.ServiceUnavailableException; import google.registry.request.Parameter; import google.registry.request.auth.Auth; +import google.registry.request.lock.LockHandler; import google.registry.util.DomainNameUtils; import google.registry.util.FormattingLogger; import java.util.Set; @@ -69,6 +69,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { @Inject @Parameter(PARAM_DOMAINS) Set domains; @Inject @Parameter(PARAM_HOSTS) Set hosts; @Inject @Parameter(PARAM_TLD) String tld; + @Inject LockHandler lockHandler; @Inject PublishDnsUpdatesAction() {} /** Runs the task. */ @@ -79,7 +80,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { // false. We need to make sure to take note of this error; otherwise, a failed lock might result // in the update task being dequeued and dropped. A message will already have been logged // to indicate the problem. - if (!executeWithLocks(this, tld, timeout, lockName)) { + if (!lockHandler.executeWithLocks(this, tld, timeout, lockName)) { throw new ServiceUnavailableException("Lock failure"); } } diff --git a/java/google/registry/rde/BUILD b/java/google/registry/rde/BUILD index d3d68ca97..4e56b0aad 100644 --- a/java/google/registry/rde/BUILD +++ b/java/google/registry/rde/BUILD @@ -16,6 +16,7 @@ java_library( "//java/google/registry/model", "//java/google/registry/request", "//java/google/registry/request/auth", + "//java/google/registry/request/lock", "//java/google/registry/tldconfig/idn", "//java/google/registry/util", "//java/google/registry/xjc", diff --git a/java/google/registry/rde/EscrowTaskRunner.java b/java/google/registry/rde/EscrowTaskRunner.java index bfa94e96b..24ca4d536 100644 --- a/java/google/registry/rde/EscrowTaskRunner.java +++ b/java/google/registry/rde/EscrowTaskRunner.java @@ -20,11 +20,11 @@ import com.googlecode.objectify.VoidWork; import google.registry.model.common.Cursor; import google.registry.model.common.Cursor.CursorType; import google.registry.model.registry.Registry; -import google.registry.model.server.Lock; import google.registry.request.HttpException.NoContentException; import google.registry.request.HttpException.ServiceUnavailableException; import google.registry.request.Parameter; import google.registry.request.RequestParameters; +import google.registry.request.lock.LockHandler; import google.registry.util.Clock; import google.registry.util.FormattingLogger; import java.util.concurrent.Callable; @@ -38,7 +38,7 @@ import org.joda.time.Duration; *

This class implements the Locking Rolling Cursor pattern, which solves the problem of * how to reliably execute App Engine tasks which can't be made idempotent. * - *

{@link Lock} is used to ensure only one task executes at a time for a given + *

{@link LockHandler} is used to ensure only one task executes at a time for a given * {@code LockedCursorTask} subclass + TLD combination. This is necessary because App Engine tasks * might double-execute. Normally tasks solve this by being idempotent, but that's not possible for * RDE, which writes to a GCS filename with a deterministic name. So Datastore is used to to @@ -71,6 +71,7 @@ class EscrowTaskRunner { @Inject Clock clock; @Inject @Parameter(RequestParameters.PARAM_TLD) String tld; + @Inject LockHandler lockHandler; @Inject EscrowTaskRunner() {} /** @@ -109,7 +110,7 @@ class EscrowTaskRunner { return null; }}; String lockName = String.format("%s %s", task.getClass().getSimpleName(), registry.getTld()); - if (!Lock.executeWithLocks(lockRunner, tld, timeout, lockName)) { + if (!lockHandler.executeWithLocks(lockRunner, tld, timeout, lockName)) { // This will happen if either: a) the task is double-executed; b) the task takes a long time // to run and the retry task got executed while the first one is still running. In both // situations the safest thing to do is to just return 503 so the task gets retried later. diff --git a/java/google/registry/rde/RdeStagingReducer.java b/java/google/registry/rde/RdeStagingReducer.java index 9a0687586..3bda382f9 100644 --- a/java/google/registry/rde/RdeStagingReducer.java +++ b/java/google/registry/rde/RdeStagingReducer.java @@ -40,9 +40,9 @@ import google.registry.model.rde.RdeMode; import google.registry.model.rde.RdeNamingUtils; import google.registry.model.rde.RdeRevision; import google.registry.model.registry.Registry; -import google.registry.model.server.Lock; import google.registry.request.Parameter; import google.registry.request.RequestParameters; +import google.registry.request.lock.LockHandler; import google.registry.tldconfig.idn.IdnTableEnum; import google.registry.util.FormattingLogger; import google.registry.util.TaskEnqueuer; @@ -66,11 +66,12 @@ import org.joda.time.Duration; /** Reducer for {@link RdeStagingAction}. */ public final class RdeStagingReducer extends Reducer { - private static final long serialVersionUID = -3366189042770402345L; + private static final long serialVersionUID = 60326234579091203L; private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private final TaskEnqueuer taskEnqueuer; + private final LockHandler lockHandler; private final int gcsBufferSize; private final String bucket; private final int ghostrydeBufferSize; @@ -81,6 +82,7 @@ public final class RdeStagingReducer extends Reducer() { - @Override - public Void call() throws Exception { - runner.lockRunAndRollForward( - task, registry, standardSeconds(30), CursorType.RDE_STAGING, standardDays(1)); - return null; - }}, - "lol", - standardSeconds(30), - lockName); + runner.lockHandler = new FakeLockHandler(false); + runner.lockRunAndRollForward( + task, registry, standardSeconds(30), CursorType.RDE_STAGING, standardDays(1)); } } diff --git a/javatests/google/registry/rde/RdeStagingActionTest.java b/javatests/google/registry/rde/RdeStagingActionTest.java index cb1e07324..f966acbe4 100644 --- a/javatests/google/registry/rde/RdeStagingActionTest.java +++ b/javatests/google/registry/rde/RdeStagingActionTest.java @@ -59,6 +59,7 @@ import google.registry.request.RequestParameters; import google.registry.testing.ExceptionRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeKeyringModule; +import google.registry.testing.FakeLockHandler; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; import google.registry.testing.TaskQueueHelper.TaskMatcher; @@ -137,6 +138,7 @@ public class RdeStagingActionTest extends MapreduceTestCase { action.lenient = false; action.reducer = new RdeStagingReducer( new TaskEnqueuer(new Retrier(new SystemSleeper(), 1)), // taskEnqueuer + new FakeLockHandler(true), 0, // gcsBufferSize "rde-bucket", // bucket 31337, // ghostrydeBufferSize