From 3f1a8eebdc04b9c2b6a23d38e801651ad74e599e Mon Sep 17 00:00:00 2001 From: mcilwain Date: Tue, 6 Feb 2018 19:02:25 -0800 Subject: [PATCH] Decrease flakiness of multipart TLD lifecycle tests As part of our commit log layer that we have built on top of Objectify, we enforce the constraint of a monotonically increasing transaction time with a millisecond granularity. Thus, if two transactions occur at exactly the same millisecond, as had been the case with these tests, one will get a TimestampInversionException and retry. However, since we're mocking time in these tests as well, they will retry at exactly the same millisecond, and thus continue failing for the same reason until the max retry threshold is hit. The EPP flow then ultimately fails with a generic "Command failed" response. All of this is actual findings from looking at test logs from a flake. It's a mystery to me why these tests were merely flaky; it seems like they should have always been failing for this reason, but they were still only sometimes failing. Who knows. The fix is simple -- Adjust the tests so that no two commands are run at exactly the same millisecond. Note that this is a test-only problem; in the real world, a command that temporarily fails will simply then succeed the next time it is retried, since time is actually elapsing. This implies that our commit log system imposes a max mutation rate of 1,000 QPS across our entire system. This is unlikely to be a problem in practice for any existing registry of any size. Also note that, as far the EPP XML itself is concerned, times only have second granularity, so up to a thousand commands can execute in the same second and still "appear" to have taken place at the same time as far as EPP is concerned. That's why this CL only adds millisecond precision to the actual run time, not to the expected values in the commands. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=184777558 --- .../flows/EppLifecycleDomainTest.java | 34 ++++++------- .../registry/flows/EppLifecycleHostTest.java | 48 ++++++++----------- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/javatests/google/registry/flows/EppLifecycleDomainTest.java b/javatests/google/registry/flows/EppLifecycleDomainTest.java index f0d5816cc..1352170de 100644 --- a/javatests/google/registry/flows/EppLifecycleDomainTest.java +++ b/javatests/google/registry/flows/EppLifecycleDomainTest.java @@ -1014,7 +1014,7 @@ public class EppLifecycleDomainTest extends EppTestCase { createTlds("bar.foo.tld", "foo.tld"); assertCommandAndResponse("login_valid.xml", "login_response.xml"); - createContacts(DateTime.parse("2000-06-01T00:00:00Z")); + createContacts(DateTime.parse("2000-06-01T00:00:00.000Z")); // Create domain example.bar.foo.tld assertCommandAndResponse( @@ -1023,9 +1023,9 @@ public class EppLifecycleDomainTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.bar.foo.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.001Z")); // Create domain example.foo.tld assertCommandAndResponse( @@ -1034,9 +1034,9 @@ public class EppLifecycleDomainTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.foo.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.002Z")); // Create domain example.tld assertCommandAndResponse( @@ -1045,9 +1045,9 @@ public class EppLifecycleDomainTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.003Z")); assertCommandAndResponse("logout.xml", "logout_response.xml"); } @@ -1057,7 +1057,7 @@ public class EppLifecycleDomainTest extends EppTestCase { createTld("tld.foo"); assertCommandAndResponse("login_valid.xml", "login_response.xml"); - createContacts(DateTime.parse("2000-06-01T00:00:00Z")); + createContacts(DateTime.parse("2000-06-01T00:00:00.000Z")); // Create domain example.tld.foo assertCommandAndResponse( @@ -1066,9 +1066,9 @@ public class EppLifecycleDomainTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.tld.foo", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.001Z")); // Create domain example.tld assertCommandAndResponse( @@ -1077,9 +1077,9 @@ public class EppLifecycleDomainTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.002Z")); assertCommandAndResponse("logout.xml", "logout_response.xml"); } diff --git a/javatests/google/registry/flows/EppLifecycleHostTest.java b/javatests/google/registry/flows/EppLifecycleHostTest.java index 9a296569e..e56a041a9 100644 --- a/javatests/google/registry/flows/EppLifecycleHostTest.java +++ b/javatests/google/registry/flows/EppLifecycleHostTest.java @@ -188,9 +188,9 @@ public class EppLifecycleHostTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.bar.foo.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.000Z")); // Create domain example.foo.tld assertCommandAndResponse( @@ -199,9 +199,9 @@ public class EppLifecycleHostTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.foo.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.001Z")); // Create domain example.tld assertCommandAndResponse( @@ -210,9 +210,9 @@ public class EppLifecycleHostTest extends EppTestCase { "domain_create_response.xml", ImmutableMap.of( "NAME", "example.tld", - "CRDATE", "2000-06-01T00:02:00.0Z", - "EXDATE", "2002-06-01T00:02:00.0Z"), - DateTime.parse("2000-06-01T00:02:00Z")); + "CRDATE", "2000-06-01T00:02:00Z", + "EXDATE", "2002-06-01T00:02:00Z"), + DateTime.parse("2000-06-01T00:02:00.002Z")); // Create host ns1.example.bar.foo.tld assertCommandAndResponse( @@ -235,15 +235,15 @@ public class EppLifecycleHostTest extends EppTestCase { "host_create_with_ips.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld"), "host_create_response.xml", - ImmutableMap.of("HOSTNAME", "ns1.example.tld", "CRDATE", "2000-06-01T00:04:00Z"), - DateTime.parse("2000-06-01T00:04:00Z")); + ImmutableMap.of("HOSTNAME", "ns1.example.tld", "CRDATE", "2000-06-01T00:05:00Z"), + DateTime.parse("2000-06-01T00:05:00Z")); + + DateTime timeAfterCreates = DateTime.parse("2000-06-01T00:06:00Z"); HostResource exampleBarFooTldHost = - loadByForeignKey( - HostResource.class, "ns1.example.bar.foo.tld", DateTime.parse("2000-06-01T00:05:00Z")); + loadByForeignKey(HostResource.class, "ns1.example.bar.foo.tld", timeAfterCreates); DomainResource exampleBarFooTldDomain = - loadByForeignKey( - DomainResource.class, "example.bar.foo.tld", DateTime.parse("2000-06-01T00:05:00Z")); + loadByForeignKey(DomainResource.class, "example.bar.foo.tld", timeAfterCreates); assertAboutHosts() .that(exampleBarFooTldHost) .hasSuperordinateDomain(Key.create(exampleBarFooTldDomain)); @@ -251,26 +251,20 @@ public class EppLifecycleHostTest extends EppTestCase { .containsExactly("ns1.example.bar.foo.tld"); HostResource exampleFooTldHost = - loadByForeignKey( - HostResource.class, "ns1.example.foo.tld", DateTime.parse("2000-06-01T00:05:00Z")); + loadByForeignKey(HostResource.class, "ns1.example.foo.tld", timeAfterCreates); DomainResource exampleFooTldDomain = - loadByForeignKey( - DomainResource.class, "example.foo.tld", DateTime.parse("2000-06-01T00:05:00Z")); + loadByForeignKey(DomainResource.class, "example.foo.tld", timeAfterCreates); assertAboutHosts() .that(exampleFooTldHost) .hasSuperordinateDomain(Key.create(exampleFooTldDomain)); - assertThat(exampleFooTldDomain.getSubordinateHosts()) - .containsExactly("ns1.example.foo.tld"); + assertThat(exampleFooTldDomain.getSubordinateHosts()).containsExactly("ns1.example.foo.tld"); HostResource exampleTldHost = - loadByForeignKey( - HostResource.class, "ns1.example.tld", DateTime.parse("2000-06-01T00:05:00Z")); + loadByForeignKey(HostResource.class, "ns1.example.tld", timeAfterCreates); DomainResource exampleTldDomain = - loadByForeignKey( - DomainResource.class, "example.tld", DateTime.parse("2000-06-01T00:05:00Z")); + loadByForeignKey(DomainResource.class, "example.tld", timeAfterCreates); assertAboutHosts().that(exampleTldHost).hasSuperordinateDomain(Key.create(exampleTldDomain)); - assertThat(exampleTldDomain.getSubordinateHosts()) - .containsExactly("ns1.example.tld"); + assertThat(exampleTldDomain.getSubordinateHosts()).containsExactly("ns1.example.tld"); assertCommandAndResponse("logout.xml", "logout_response.xml"); }