Remove reference to TLD in Locks' ResourceName

The TLD is added separately to the lock (the lock is unique per
TLD-ResourceName pair), so there's no need to add it to the resourceName.

The current status is that the TLD was included twice in the lockId. After the fix - it'll only be included once.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=172659986
This commit is contained in:
guyben 2017-10-18 14:57:45 -07:00 committed by jianglai
parent e62e1af863
commit 06f0ec4f2f
3 changed files with 8 additions and 12 deletions

View file

@ -22,8 +22,6 @@ import google.registry.model.common.Cursor.CursorType;
import google.registry.model.registry.Registry; import google.registry.model.registry.Registry;
import google.registry.request.HttpException.NoContentException; import google.registry.request.HttpException.NoContentException;
import google.registry.request.HttpException.ServiceUnavailableException; 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.request.lock.LockHandler;
import google.registry.util.Clock; import google.registry.util.Clock;
import google.registry.util.FormattingLogger; import google.registry.util.FormattingLogger;
@ -70,7 +68,6 @@ class EscrowTaskRunner {
private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
@Inject Clock clock; @Inject Clock clock;
@Inject @Parameter(RequestParameters.PARAM_TLD) String tld;
@Inject LockHandler lockHandler; @Inject LockHandler lockHandler;
@Inject EscrowTaskRunner() {} @Inject EscrowTaskRunner() {}
@ -113,12 +110,13 @@ class EscrowTaskRunner {
}); });
return null; return null;
}; };
String lockName = String.format("%s %s", task.getClass().getSimpleName(), registry.getTld()); String lockName = String.format("EscrowTaskRunner %s", task.getClass().getSimpleName());
if (!lockHandler.executeWithLocks(lockRunner, tld, timeout, lockName)) { if (!lockHandler.executeWithLocks(lockRunner, registry.getTldStr(), timeout, lockName)) {
// This will happen if either: a) the task is double-executed; b) the task takes a long time // 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 // 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. // situations the safest thing to do is to just return 503 so the task gets retried later.
throw new ServiceUnavailableException("Lock in use: " + lockName); throw new ServiceUnavailableException(
String.format("Lock in use: %s for TLD: %s", lockName, registry.getTldStr()));
} }
} }
} }

View file

@ -107,8 +107,8 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
reduceWithLock(key, fragments); reduceWithLock(key, fragments);
return null; return null;
}; };
String lockName = String.format("RdeStaging %s %s", key.tld(), key.mode()); String lockName = String.format("RdeStaging %s", key.mode());
if (!lockHandler.executeWithLocks(lockRunner, null, lockTimeout, lockName)) { if (!lockHandler.executeWithLocks(lockRunner, key.tld(), lockTimeout, lockName)) {
logger.warningfmt("Lock in use: %s", lockName); logger.warningfmt("Lock in use: %s", lockName);
} }
} }

View file

@ -15,7 +15,6 @@
package google.registry.rde; package google.registry.rde;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DatastoreHelper.persistResource;
@ -67,7 +66,6 @@ public class EscrowTaskRunnerTest {
registry = Registry.get("lol"); registry = Registry.get("lol");
runner = new EscrowTaskRunner(); runner = new EscrowTaskRunner();
runner.clock = clock; runner.clock = clock;
runner.tld = "lol";
runner.lockHandler = new FakeLockHandler(true); runner.lockHandler = new FakeLockHandler(true);
DateTimeZone.setDefault(DateTimeZone.forID("America/New_York")); // Make sure UTC stuff works. DateTimeZone.setDefault(DateTimeZone.forID("America/New_York")); // Make sure UTC stuff works.
} }
@ -108,11 +106,11 @@ public class EscrowTaskRunnerTest {
@Test @Test
public void testRun_lockIsntAvailable_throws503() throws Exception { public void testRun_lockIsntAvailable_throws503() throws Exception {
String lockName = task.getClass().getSimpleName() + " lol"; String lockName = "EscrowTaskRunner " + task.getClass().getSimpleName();
clock.setTo(DateTime.parse("2006-06-06T00:30:00Z")); clock.setTo(DateTime.parse("2006-06-06T00:30:00Z"));
persistResource( persistResource(
Cursor.create(CursorType.RDE_STAGING, DateTime.parse("2006-06-06TZ"), registry)); Cursor.create(CursorType.RDE_STAGING, DateTime.parse("2006-06-06TZ"), registry));
thrown.expect(ServiceUnavailableException.class, "Lock in use: " + lockName); thrown.expect(ServiceUnavailableException.class, "Lock in use: " + lockName + " for TLD: lol");
runner.lockHandler = new FakeLockHandler(false); runner.lockHandler = new FakeLockHandler(false);
runner.lockRunAndRollForward( runner.lockRunAndRollForward(
task, registry, standardSeconds(30), CursorType.RDE_STAGING, standardDays(1)); task, registry, standardSeconds(30), CursorType.RDE_STAGING, standardDays(1));