zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-06-20 03:10:47 +02:00
Code Issues Projects Releases Packages Wiki Activity

Remove Lock Dual Read and Dual Write (#568)

Browse source
This commit is contained in:
sarahcaseybot 2020-04-27 17:30:51 -04:00 • committed by GitHub
parent 5af4655c62
commit 85adf61f06
2 changed files with 0 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

79
core/src/main/java/google/registry/model/server/Lock.java
View file

@ -16,7 +16,6 @@ package google.registry.model.server;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.util.DateTimeUtils.isAtOrAfter; import static google.registry.util.DateTimeUtils.isAtOrAfter;
@ -29,7 +28,6 @@ import com.googlecode.objectify.annotation.Id;
import google.registry.model.ImmutableObject; import google.registry.model.ImmutableObject;
import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp;
import google.registry.model.annotations.NotBackedUp.Reason; import google.registry.model.annotations.NotBackedUp.Reason;
import google.registry.schema.server.LockDao;
import google.registry.util.RequestStatusChecker; import google.registry.util.RequestStatusChecker;
import google.registry.util.RequestStatusCheckerImpl; import google.registry.util.RequestStatusCheckerImpl;
import java.io.Serializable; import java.io.Serializable;
@ -197,22 +195,6 @@ public class Lock extends ImmutableObject implements Serializable {
// Checking if an unexpired lock still exists - if so, the lock can't be acquired. // Checking if an unexpired lock still exists - if so, the lock can't be acquired.
Lock lock = ofy().load().type(Lock.class).id(lockId).now(); Lock lock = ofy().load().type(Lock.class).id(lockId).now();
try {
jpaTm()
.transact(
() -> {
Optional<google.registry.schema.server.Lock> cloudSqlLockOptional;
if (tld == null) {
cloudSqlLockOptional = LockDao.load(resourceName);
} else {
cloudSqlLockOptional = LockDao.load(resourceName, tld);
}
LockDao.compare(Optional.ofNullable(lock), cloudSqlLockOptional);
});
} catch (Exception e) {
logger.atSevere().withCause(e).log(
"Issue loading and comparing lock from Cloud SQL");
}
if (lock != null) { if (lock != null) {
logger.atInfo().log( logger.atInfo().log(
"Loaded existing lock: %s for request: %s", lock.lockId, lock.requestLogId); "Loaded existing lock: %s for request: %s", lock.lockId, lock.requestLogId);
@ -237,35 +219,6 @@ public class Lock extends ImmutableObject implements Serializable {
// don't need to be backed up. // don't need to be backed up.
ofy().saveWithoutBackup().entity(newLock); ofy().saveWithoutBackup().entity(newLock);
// create and save the lock to Cloud SQL
try {
jpaTm()
.transact(
() -> {
google.registry.schema.server.Lock cloudSqlLock;
if (tld == null) {
cloudSqlLock =
google.registry.schema.server.Lock.createGlobal(
resourceName,
requestStatusChecker.getLogId(),
now,
leaseLength);
} else {
cloudSqlLock =
google.registry.schema.server.Lock.create(
resourceName,
tld,
requestStatusChecker.getLogId(),
now,
leaseLength);
}
LockDao.save(cloudSqlLock);
});
} catch (Exception e) {
logger.atSevere().withCause(e).log(
"Error saving lock to Cloud SQL: %s", newLock);
}
return AcquireResult.create(now, lock, newLock, lockState); return AcquireResult.create(now, lock, newLock, lockState);
}); });
@ -284,44 +237,12 @@ public class Lock extends ImmutableObject implements Serializable {
// this can happen if release() is called around the expiration time and the lock // this can happen if release() is called around the expiration time and the lock
// expires underneath us. // expires underneath us.
Lock loadedLock = ofy().load().type(Lock.class).id(lockId).now(); Lock loadedLock = ofy().load().type(Lock.class).id(lockId).now();
try {
jpaTm()
.transact(
() -> {
Optional<google.registry.schema.server.Lock> cloudSqlLockOptional;
if (tld == null) {
cloudSqlLockOptional = LockDao.load(resourceName);
} else {
cloudSqlLockOptional = LockDao.load(resourceName, tld);
}
LockDao.compare(Optional.ofNullable(loadedLock), cloudSqlLockOptional);
});
} catch (Exception e) {
logger.atSevere().withCause(e).log(
"Issue loading and comparing lock from Cloud SQL");
}
if (Lock.this.equals(loadedLock)) { if (Lock.this.equals(loadedLock)) {
// Use noBackupOfy() so that we don't create a commit log entry for deleting the // Use noBackupOfy() so that we don't create a commit log entry for deleting the
// lock. // lock.
logger.atInfo().log("Deleting lock: %s", lockId); logger.atInfo().log("Deleting lock: %s", lockId);
ofy().deleteWithoutBackup().entity(Lock.this); ofy().deleteWithoutBackup().entity(Lock.this);
// Remove the lock from Cloud SQL
try {
jpaTm()
.transact(
() -> {
if (tld == null) {
LockDao.delete(resourceName);
} else {
LockDao.delete(resourceName, tld);
}
});
} catch (Exception e) {
logger.atSevere().withCause(e).log(
"Error deleting lock from Cloud SQL: %s", loadedLock);
}
lockMetrics.recordRelease( lockMetrics.recordRelease(
resourceName, tld, new Duration(acquiredTime, tm().getTransactionTime())); resourceName, tld, new Duration(acquiredTime, tm().getTransactionTime()));
} else { } else {

4
core/src/test/java/google/registry/model/server/LockTest.java
View file

@ -28,7 +28,6 @@ import static org.mockito.Mockito.when;
import google.registry.model.ofy.Ofy; import google.registry.model.ofy.Ofy;
import google.registry.model.server.Lock.LockState; import google.registry.model.server.Lock.LockState;
import google.registry.schema.server.LockDao;
import google.registry.testing.AppEngineRule; import google.registry.testing.AppEngineRule;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
import google.registry.testing.InjectRule; import google.registry.testing.InjectRule;
@ -65,19 +64,16 @@ public class LockTest {
Optional<Lock> lock = Lock.acquire(RESOURCE_NAME, tld, leaseLength, requestStatusChecker, true); Optional<Lock> lock = Lock.acquire(RESOURCE_NAME, tld, leaseLength, requestStatusChecker, true);
verify(Lock.lockMetrics).recordAcquire(RESOURCE_NAME, tld, expectedLockState); verify(Lock.lockMetrics).recordAcquire(RESOURCE_NAME, tld, expectedLockState);
verifyNoMoreInteractions(Lock.lockMetrics); verifyNoMoreInteractions(Lock.lockMetrics);
assertThat(LockDao.load(RESOURCE_NAME, tld)).isPresent();
Lock.lockMetrics = null; Lock.lockMetrics = null;
return lock; return lock;
} }
private void release(Lock lock, String expectedTld, long expectedMillis) { private void release(Lock lock, String expectedTld, long expectedMillis) {
assertThat(LockDao.load(RESOURCE_NAME, expectedTld)).isPresent();
Lock.lockMetrics = mock(LockMetrics.class); Lock.lockMetrics = mock(LockMetrics.class);
lock.release(); lock.release();
verify(Lock.lockMetrics) verify(Lock.lockMetrics)
.recordRelease(RESOURCE_NAME, expectedTld, Duration.millis(expectedMillis)); .recordRelease(RESOURCE_NAME, expectedTld, Duration.millis(expectedMillis));
verifyNoMoreInteractions(Lock.lockMetrics); verifyNoMoreInteractions(Lock.lockMetrics);
assertThat(LockDao.load(RESOURCE_NAME, expectedTld)).isEmpty();
Lock.lockMetrics = null; Lock.lockMetrics = null;
} }