mirror of
https://github.com/google/nomulus.git
synced 2025-05-13 16:07:15 +02:00
Disable memcache completely
We've determined that getting correctness semantics right, even in the few cases that it is possible to do so (see linked bug for audit) is not worth the bother in terms of highly complicated code and potential bugs. This CL turns off memcache at the Ofy level but doesn't rip out the annotations etc. so that we can quickly turn it back on if this turns out to have been a mistake. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=155227761
This commit is contained in:
cgoldfeder
Ben McIlwain
parent
608e121a37
commit
275d6ddc10
7 changed files with 3 additions and 133 deletions
|
|
@ -140,8 +140,7 @@ public class Ofy {
|
|||
* worth the extra complexity of reasoning about caching.
|
||||
*/
|
||||
public Loader load() {
|
||||
// TODO(b/27424173): change to false when memcache audit changes are implemented.
|
||||
return ofy().cache(true).load();
|
||||
return ofy().cache(false).load();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -154,7 +153,8 @@ public class Ofy {
|
|||
* worth the extra complexity of reasoning about caching.
|
||||
*/
|
||||
public Loader loadWithMemcache() {
|
||||
return ofy().cache(true).load();
|
||||
// TODO(b/27424173): Remove this method if we determine we are ok with no memcache.
|
||||
return ofy().cache(false).load();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@ import static google.registry.testing.DatastoreHelper.persistReservedList;
|
|||
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||
import static google.registry.testing.DomainApplicationSubject.assertAboutApplications;
|
||||
import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.util.DateTimeUtils.END_OF_TIME;
|
||||
import static google.registry.util.DateTimeUtils.START_OF_TIME;
|
||||
import static org.joda.money.CurrencyUnit.EUR;
|
||||
|
|
@ -108,7 +107,6 @@ import google.registry.flows.domain.DomainFlowUtils.UnsupportedFeeAttributeExcep
|
|||
import google.registry.flows.domain.DomainFlowUtils.UnsupportedMarkTypeException;
|
||||
import google.registry.flows.exceptions.ResourceAlreadyExistsException;
|
||||
import google.registry.model.domain.DomainApplication;
|
||||
import google.registry.model.domain.DomainResource;
|
||||
import google.registry.model.domain.GracePeriod;
|
||||
import google.registry.model.domain.LrpTokenEntity;
|
||||
import google.registry.model.domain.launch.ApplicationStatus;
|
||||
|
|
@ -116,8 +114,6 @@ import google.registry.model.domain.launch.LaunchNotice;
|
|||
import google.registry.model.domain.launch.LaunchPhase;
|
||||
import google.registry.model.domain.rgp.GracePeriodStatus;
|
||||
import google.registry.model.domain.secdns.DelegationSignerData;
|
||||
import google.registry.model.index.ForeignKeyIndex;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.model.registrar.Registrar;
|
||||
import google.registry.model.registry.Registry;
|
||||
import google.registry.model.registry.Registry.TldState;
|
||||
|
|
@ -1547,30 +1543,6 @@ public class DomainApplicationCreateFlowTest
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailfast_withMemcachedDomainAndFki_hitsMemcache() throws Exception {
|
||||
persistContactsAndHosts();
|
||||
clock.advanceOneMilli();
|
||||
DomainResource domain = persistActiveDomain(getUniqueIdFromCommand());
|
||||
setMemcacheContents(Key.create(domain), ForeignKeyIndex.createKey(domain));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
try {
|
||||
runFlow();
|
||||
assert_().fail("Expected to throw ResourceAlreadyExistsException");
|
||||
} catch (ResourceAlreadyExistsException e) {
|
||||
assertThat(e.isFailfast()).isTrue();
|
||||
}
|
||||
// Everything should have been loaded from memcache so nothing should hit datastore.
|
||||
int numReadsInFlow =
|
||||
RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
// TODO(b/27424173): This is 1 because there is no @Cache annotation on DomainBase, and we
|
||||
// don't want to blindly add it because that's a production change that adds potentially
|
||||
// dangerous caching. When the recommendations from the audit in b/27424173 are done and we've
|
||||
// tested the new safer caching this should be set to 0.
|
||||
assertThat(numReadsInFlow).isEqualTo(1);
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void testFailure_registrantNotWhitelisted() throws Exception {
|
||||
persistActiveContact("someone");
|
||||
|
|
|
|||
|
|
@ -24,7 +24,6 @@ import static google.registry.testing.DatastoreHelper.persistActiveContact;
|
|||
import static google.registry.testing.DatastoreHelper.persistActiveHost;
|
||||
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||
import static google.registry.testing.MemcacheHelper.clearMemcache;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
|
||||
import static google.registry.util.DatastoreServiceUtils.KEY_TO_KIND_FUNCTION;
|
||||
|
||||
|
|
@ -53,7 +52,6 @@ import google.registry.model.domain.secdns.DelegationSignerData;
|
|||
import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
|
||||
import google.registry.model.eppcommon.StatusValue;
|
||||
import google.registry.model.host.HostResource;
|
||||
import google.registry.model.index.DomainApplicationIndex;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.model.registry.Registry.TldState;
|
||||
import google.registry.model.smd.EncodedSignedMark;
|
||||
|
|
@ -330,28 +328,6 @@ public class DomainApplicationInfoFlowTest
|
|||
runFlow();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadsComeFromMemcache() throws Exception {
|
||||
persistTestEntities(HostsState.HOSTS_EXIST, MarksState.NO_MARKS_EXIST);
|
||||
setMemcacheContents(
|
||||
Key.create(application),
|
||||
DomainApplicationIndex.createKey(application),
|
||||
Key.create(registrant),
|
||||
Key.create(contact),
|
||||
Key.create(host1),
|
||||
Key.create(host2));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
doSuccessfulTest("domain_info_sunrise_response.xml", HostsState.HOSTS_EXIST);
|
||||
|
||||
// Everything should be in memcache so nothing should hit datastore.
|
||||
int numReadsInFlow = RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
// TODO(b/27424173): This is 1 because there is no @Cache annotation on DomainBase, and we
|
||||
// don't want to blindly add it because that's a production change that adds potentially
|
||||
// dangerous caching. When the recommendations from the audit in b/27424173 are done and we've
|
||||
// tested the new safer caching this should be set to 0.
|
||||
assertThat(numReadsInFlow).isEqualTo(1);
|
||||
}
|
||||
|
||||
/** Test that we load contacts and hosts as a batch rather than individually. */
|
||||
@Test
|
||||
public void testBatchLoadingOfReferences() throws Exception {
|
||||
|
|
|
|||
|
|
@ -41,7 +41,6 @@ import static google.registry.testing.DatastoreHelper.persistReservedList;
|
|||
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||
import static google.registry.testing.DomainResourceSubject.assertAboutDomains;
|
||||
import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued;
|
||||
import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued;
|
||||
import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued;
|
||||
|
|
@ -58,7 +57,6 @@ import com.google.common.base.Strings;
|
|||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.common.collect.ImmutableSortedMap;
|
||||
import com.googlecode.objectify.Key;
|
||||
import google.registry.flows.EppException.UnimplementedExtensionException;
|
||||
import google.registry.flows.EppRequestSource;
|
||||
import google.registry.flows.ExtensionManager.UndeclaredServiceExtensionException;
|
||||
|
|
@ -124,8 +122,6 @@ import google.registry.model.domain.launch.LaunchNotice;
|
|||
import google.registry.model.domain.rgp.GracePeriodStatus;
|
||||
import google.registry.model.domain.secdns.DelegationSignerData;
|
||||
import google.registry.model.eppcommon.StatusValue;
|
||||
import google.registry.model.index.ForeignKeyIndex;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.model.poll.PollMessage;
|
||||
import google.registry.model.registrar.Registrar;
|
||||
import google.registry.model.registry.Registry;
|
||||
|
|
@ -912,28 +908,6 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailfast_withMemcachedDomainAndFki_hitsMemcache() throws Exception {
|
||||
persistContactsAndHosts();
|
||||
DomainResource domain = persistActiveDomain(getUniqueIdFromCommand());
|
||||
setMemcacheContents(Key.create(domain), ForeignKeyIndex.createKey(domain));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
try {
|
||||
runFlow();
|
||||
assert_().fail("Expected to throw ResourceAlreadyExistsException");
|
||||
} catch (ResourceAlreadyExistsException e) {
|
||||
assertThat(e.isFailfast()).isTrue();
|
||||
}
|
||||
// Everything should have been loaded from memcache so nothing should hit datastore.
|
||||
int numReadsInFlow =
|
||||
RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
// TODO(b/27424173): This is 1 because there is no @Cache annotation on DomainBase, and we
|
||||
// don't want to blindly add it because that's a production change that adds potentially
|
||||
// dangerous caching. When the recommendations from the audit in b/27424173 are done and we've
|
||||
// tested the new safer caching this should be set to 0.
|
||||
assertThat(numReadsInFlow).isEqualTo(1);
|
||||
}
|
||||
|
||||
/**
|
||||
* There is special logic that disallows a failfast for domains in add grace period and sunrush
|
||||
* add grace period, so make sure that they fail anyways in the actual flow.
|
||||
|
|
|
|||
|
|
@ -25,7 +25,6 @@ import static google.registry.testing.DatastoreHelper.persistActiveContact;
|
|||
import static google.registry.testing.DatastoreHelper.persistActiveHost;
|
||||
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||
import static google.registry.testing.MemcacheHelper.clearMemcache;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
|
||||
import static google.registry.util.DatastoreServiceUtils.KEY_TO_KIND_FUNCTION;
|
||||
|
||||
|
|
@ -56,7 +55,6 @@ import google.registry.model.domain.secdns.DelegationSignerData;
|
|||
import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
|
||||
import google.registry.model.eppcommon.StatusValue;
|
||||
import google.registry.model.host.HostResource;
|
||||
import google.registry.model.index.ForeignKeyIndex;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.testing.AppEngineRule;
|
||||
import java.util.List;
|
||||
|
|
@ -635,29 +633,6 @@ public class DomainInfoFlowTest extends ResourceFlowTestCase<DomainInfoFlow, Dom
|
|||
runFlow();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadsComeFromMemcache() throws Exception {
|
||||
persistTestEntities(false);
|
||||
setMemcacheContents(
|
||||
Key.create(domain),
|
||||
ForeignKeyIndex.createKey(domain),
|
||||
Key.create(registrant),
|
||||
Key.create(contact),
|
||||
Key.create(host1),
|
||||
Key.create(host2),
|
||||
Key.create(host3));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
runFlowAssertResponse(loadFileWithSubstitutions(
|
||||
getClass(), "domain_info_response.xml", ImmutableMap.of("ROID", "2FF-TLD")));
|
||||
// Everything should be in memcache so nothing should hit datastore.
|
||||
int numReadsInFlow = RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
// TODO(b/27424173): This is 1 because there is no @Cache annotation on DomainBase, and we
|
||||
// don't want to blindly add it because that's a production change that adds potentially
|
||||
// dangerous caching. When the recommendations from the audit in b/27424173 are done and we've
|
||||
// tested the new safer caching this should be set to 0.
|
||||
assertThat(numReadsInFlow).isEqualTo(1);
|
||||
}
|
||||
|
||||
/** Test that we load contacts and hosts as a batch rather than individually. */
|
||||
@Test
|
||||
public void testBatchLoadingOfReferences() throws Exception {
|
||||
|
|
|
|||
|
|
@ -25,7 +25,6 @@ import static google.registry.testing.DatastoreHelper.createTld;
|
|||
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||
import static google.registry.testing.DatastoreHelper.persistSimpleResource;
|
||||
import static google.registry.testing.DatastoreHelper.persistSimpleResources;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
|
|
@ -35,7 +34,6 @@ import com.googlecode.objectify.Key;
|
|||
import com.googlecode.objectify.VoidWork;
|
||||
import google.registry.model.EntityTestCase;
|
||||
import google.registry.model.common.EntityGroupRoot;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.model.registrar.Registrar.State;
|
||||
import google.registry.model.registrar.Registrar.Type;
|
||||
import google.registry.testing.ExceptionRule;
|
||||
|
|
@ -396,13 +394,4 @@ public class RegistrarTest extends EntityTestCase {
|
|||
assertThat(Registrar.loadByClientId("registrar")).isNotNull();
|
||||
assertThat(ofy().getSessionKeys()).contains(Key.create(registrar));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadByClientId_usesMemcache() {
|
||||
setMemcacheContents(Key.create(registrar));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
Registrar.loadByClientId("registrar");
|
||||
int numReadsInLoad = RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
assertThat(numReadsInLoad).isEqualTo(0);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -24,7 +24,6 @@ import static google.registry.testing.DatastoreHelper.createTld;
|
|||
import static google.registry.testing.DatastoreHelper.newRegistry;
|
||||
import static google.registry.testing.DatastoreHelper.persistPremiumList;
|
||||
import static google.registry.testing.DatastoreHelper.persistReservedList;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.util.DateTimeUtils.END_OF_TIME;
|
||||
import static google.registry.util.DateTimeUtils.START_OF_TIME;
|
||||
import static org.joda.money.CurrencyUnit.EUR;
|
||||
|
|
@ -34,7 +33,6 @@ import com.google.common.collect.ImmutableSet;
|
|||
import com.google.common.collect.ImmutableSortedMap;
|
||||
import com.googlecode.objectify.Key;
|
||||
import google.registry.model.EntityTestCase;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.model.registry.Registry.RegistryNotFoundException;
|
||||
import google.registry.model.registry.Registry.TldState;
|
||||
import google.registry.model.registry.label.PremiumList;
|
||||
|
|
@ -480,18 +478,4 @@ public class RegistryTest extends EntityTestCase {
|
|||
.setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(EUR)))
|
||||
.build();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadHitsMemcache() {
|
||||
Registry registry = Registry.get("tld");
|
||||
// In unit tests the in-memory cache has a duration of 0, so even without this line we should
|
||||
// expect this to hit memcache. Nevertheless, remove it from the cache explicitly so that if the
|
||||
// unit test caching is ever set to nonzero this won't spuriously pass.
|
||||
registry.invalidateInCache();
|
||||
setMemcacheContents(Key.create(getCrossTldKey(), Registry.class, "tld"));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
Registry.get("tld");
|
||||
int numReadsForGet = RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
assertThat(numReadsForGet).isEqualTo(0);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue