Make LockHandlerImpl.clock transient

SystemClock isn't Serializable (for obvious reasons), whereas LockHandlerImpl is
used as a field on some Serializable mapreduce classes. So mark it transient and
then re-generate it on first use following de-serialization when it happens to
be null.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=201707209
This commit is contained in:
mcilwain 2018-06-22 10:43:21 -07:00 committed by Ben McIlwain
parent 0422205d84
commit 8245d2f1c4
3 changed files with 25 additions and 14 deletions

View file

@ -27,6 +27,8 @@ import google.registry.model.server.Lock;
import google.registry.util.AppEngineTimeLimiter; import google.registry.util.AppEngineTimeLimiter;
import google.registry.util.Clock; import google.registry.util.Clock;
import google.registry.util.RequestStatusChecker; import google.registry.util.RequestStatusChecker;
import google.registry.util.SystemClock;
import java.io.Serializable;
import java.util.HashSet; import java.util.HashSet;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
@ -40,19 +42,30 @@ import org.joda.time.DateTime;
import org.joda.time.Duration; import org.joda.time.Duration;
/** Implementation of {@link LockHandler} that uses the datastore lock. */ /** Implementation of {@link LockHandler} that uses the datastore lock. */
public class LockHandlerImpl implements LockHandler { public class LockHandlerImpl implements LockHandler, Serializable {
private static final long serialVersionUID = 6551645164118637767L;
private static final long serialVersionUID = 5162259753801400985L;
private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final FluentLogger logger = FluentLogger.forEnclosingClass();
/** Fudge factor to make sure we kill threads before a lock actually expires. */ /** Fudge factor to make sure we kill threads before a lock actually expires. */
private static final Duration LOCK_TIMEOUT_FUDGE = Duration.standardSeconds(5); private static final Duration LOCK_TIMEOUT_FUDGE = Duration.standardSeconds(5);
@Inject Clock clock; private final RequestStatusChecker requestStatusChecker;
@Inject RequestStatusChecker requestStatusChecker; @Nullable private transient Clock clock;
@Inject public LockHandlerImpl() {} @Inject
public LockHandlerImpl(RequestStatusChecker requestStatusChecker, Clock clock) {
this.requestStatusChecker = requestStatusChecker;
this.clock = clock;
}
private synchronized Clock getClock() {
// Re-set the clock on first use after de-serialization
if (clock == null) {
clock = new SystemClock();
}
return clock;
}
/** /**
* Acquire one or more locks and execute a Void {@link Callable}. * Acquire one or more locks and execute a Void {@link Callable}.
@ -70,7 +83,7 @@ public class LockHandlerImpl implements LockHandler {
@Nullable String tld, @Nullable String tld,
Duration leaseLength, Duration leaseLength,
String... lockNames) { String... lockNames) {
DateTime startTime = clock.nowUtc(); DateTime startTime = getClock().nowUtc();
String sanitizedTld = Strings.emptyToNull(tld); String sanitizedTld = Strings.emptyToNull(tld);
try { try {
return AppEngineTimeLimiter.create() return AppEngineTimeLimiter.create()
@ -87,7 +100,7 @@ public class LockHandlerImpl implements LockHandler {
"Execution on locks '%s' for TLD '%s' timed out after %s; started at %s", "Execution on locks '%s' for TLD '%s' timed out after %s; started at %s",
Joiner.on(", ").join(lockNames), Joiner.on(", ").join(lockNames),
Optional.ofNullable(sanitizedTld).orElse("(null)"), Optional.ofNullable(sanitizedTld).orElse("(null)"),
new Duration(startTime, clock.nowUtc()), new Duration(startTime, getClock().nowUtc()),
startTime), startTime),
cause); cause);
} }
@ -113,10 +126,7 @@ public class LockHandlerImpl implements LockHandler {
final Set<String> lockNames; final Set<String> lockNames;
LockingCallable( LockingCallable(
Callable<Void> delegate, Callable<Void> delegate, String tld, Duration leaseLength, String... lockNames) {
String tld,
Duration leaseLength,
String... lockNames) {
checkArgument(leaseLength.isLongerThan(LOCK_TIMEOUT_FUDGE)); checkArgument(leaseLength.isLongerThan(LOCK_TIMEOUT_FUDGE));
this.delegate = delegate; this.delegate = delegate;
this.tld = tld; this.tld = tld;

View file

@ -14,6 +14,7 @@ java_library(
deps = [ deps = [
"//java/google/registry/model", "//java/google/registry/model",
"//java/google/registry/request/lock", "//java/google/registry/request/lock",
"//java/google/registry/util",
"//javatests/google/registry/testing", "//javatests/google/registry/testing",
"//third_party/java/appengine:appengine-api-testonly", "//third_party/java/appengine:appengine-api-testonly",
"//third_party/java/appengine:appengine-integration-testing", "//third_party/java/appengine:appengine-integration-testing",

View file

@ -23,6 +23,7 @@ import static org.mockito.Mockito.verify;
import google.registry.model.server.Lock; import google.registry.model.server.Lock;
import google.registry.testing.AppEngineRule; import google.registry.testing.AppEngineRule;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
import google.registry.util.RequestStatusCheckerImpl;
import java.util.Optional; import java.util.Optional;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
@ -72,7 +73,7 @@ public final class LockHandlerImplTest {
} }
private boolean executeWithLocks(Callable<Void> callable, final @Nullable Lock acquiredLock) { private boolean executeWithLocks(Callable<Void> callable, final @Nullable Lock acquiredLock) {
LockHandlerImpl lockHandler = new LockHandlerImpl() { LockHandlerImpl lockHandler = new LockHandlerImpl(new RequestStatusCheckerImpl(), clock) {
private static final long serialVersionUID = 0L; private static final long serialVersionUID = 0L;
@Override @Override
Optional<Lock> acquire(String resourceName, String tld, Duration leaseLength) { Optional<Lock> acquire(String resourceName, String tld, Duration leaseLength) {
@ -82,7 +83,6 @@ public final class LockHandlerImplTest {
return Optional.ofNullable(acquiredLock); return Optional.ofNullable(acquiredLock);
} }
}; };
lockHandler.clock = clock;
return lockHandler.executeWithLocks(callable, "tld", ONE_DAY, "resourceName"); return lockHandler.executeWithLocks(callable, "tld", ONE_DAY, "resourceName");
} }