Skip RRS update if existing records are equal to desired records

This is done first and formost to stop "empty" commits that cause errors in
publishDnsUpdates. The reason being that the Cloud DNS api fails when there are
no updates at all in a change.

Allowing this is a requirement for the writer to be idempotent - if we delete a
domain, then run the writer to delete it again - we'll get 0 additions and 0
deletions which fails.

This isn't theoretical either - we've seen it happen, causing a
publishDnsUpdates to fail over and over again.

While fixing this, we also remove all RRS that are common between additions and
deletions. This is just an optimization and shouldn't affect behavior.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=179525218
This commit is contained in:
guyben 2017-12-19 01:53:58 -08:00 committed by Ben McIlwain
parent 42795074a8
commit 633eb3179a
2 changed files with 24 additions and 1 deletions

View file

@ -28,6 +28,7 @@ import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.net.InternetDomainName; import com.google.common.net.InternetDomainName;
import com.google.common.util.concurrent.RateLimiter; import com.google.common.util.concurrent.RateLimiter;
import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryConfig.Config;
@ -379,7 +380,23 @@ public class CloudDnsWriter extends BaseDnsWriter {
*/ */
private void updateResourceRecords( private void updateResourceRecords(
ImmutableSet<ResourceRecordSet> additions, ImmutableSet<ResourceRecordSet> deletions) { ImmutableSet<ResourceRecordSet> additions, ImmutableSet<ResourceRecordSet> deletions) {
Change change = new Change().setAdditions(additions.asList()).setDeletions(deletions.asList()); // Find records that are both in additions and deletions, so we can remove them from both before
// requesting the change. This is mostly for optimization reasons - not doing so doesn't affect
// the result.
ImmutableSet<ResourceRecordSet> intersection =
Sets.intersection(additions, deletions).immutableCopy();
logger.infofmt(
"There are %s common items out of the %s items in 'additions' and %s items in 'deletions'",
intersection.size(), additions.size(), deletions.size());
// Exit early if we have nothing to update - dnsConnection doesn't work on empty changes
if (additions.equals(deletions)) {
logger.infofmt("Returning early because additions is the same as deletions");
return;
}
Change change =
new Change()
.setAdditions(ImmutableList.copyOf(Sets.difference(additions, intersection)))
.setDeletions(ImmutableList.copyOf(Sets.difference(deletions, intersection)));
rateLimiter.acquire(); rateLimiter.acquire();
try { try {

View file

@ -490,4 +490,10 @@ public class CloudDnsWriterTest {
writer.commit(); writer.commit();
assertThat(zoneNameCaptor.getValue()).isEqualTo("triple-secret-tld"); assertThat(zoneNameCaptor.getValue()).isEqualTo("triple-secret-tld");
} }
@Test
public void testEmptyCommit() {
writer.commit();
verify(dnsConnection, times(0)).changes();
}
} }