cmd/devp2p: submit Route53 changes in batches

cmd/devp2p/dns_route53.go

@@ -19,6 +19,7 @@ package main
 import (
 	"errors"
 	"fmt"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -31,6 +32,10 @@ import (
 	"gopkg.in/urfave/cli.v1"
 )
 
+// The Route53 limits change sets to this size. DNS changes need to be split
+// up into multiple batches to work around the limit.
+const route53ChangeLimit = 32000
+
 var (
 	route53AccessKeyFlag = cli.StringFlag{
 		Name:   "access-key-id",
@@ -78,31 +83,38 @@ func (c *route53Client) deploy(name string, t *dnsdisc.Tree) error {
 	}
 
 	// Compute DNS changes.
-	records := t.ToTXT(name)
-	changes, err := c.computeChanges(name, records)
+	existing, err := c.collectRecords(name)
 	if err != nil {
 		return err
 	}
+	log.Info(fmt.Sprintf("Found %d TXT records", len(existing)))
+	records := t.ToTXT(name)
+	changes := c.computeChanges(name, records, existing)
 	if len(changes) == 0 {
 		log.Info("No DNS changes needed")
 		return nil
 	}
 
-	// Submit change request.
-	log.Info(fmt.Sprintf("Submitting %d changes to Route53", len(changes)))
-	batch := new(route53.ChangeBatch)
-	batch.SetChanges(changes)
-	batch.SetComment(fmt.Sprintf("enrtree update of %s at seq %d", name, t.Seq()))
-	req := &route53.ChangeResourceRecordSetsInput{HostedZoneId: &c.zoneID, ChangeBatch: batch}
-	resp, err := c.api.ChangeResourceRecordSets(req)
-	if err != nil {
-		return err
-	}
+	// Submit change batches.
+	batches := splitChanges(changes, route53ChangeLimit)
+	for i, changes := range batches {
+		log.Info(fmt.Sprintf("Submitting %d changes to Route53", len(changes)))
+		batch := new(route53.ChangeBatch)
+		batch.SetChanges(changes)
+		batch.SetComment(fmt.Sprintf("enrtree update %d/%d of %s at seq %d", i+1, len(batches), name, t.Seq()))
+		req := &route53.ChangeResourceRecordSetsInput{HostedZoneId: &c.zoneID, ChangeBatch: batch}
+		resp, err := c.api.ChangeResourceRecordSets(req)
+		if err != nil {
+			return err
+		}
 
-	// Wait for the change to be applied.
-	log.Info(fmt.Sprintf("Waiting for change request %s", *resp.ChangeInfo.Id))
-	wreq := &route53.GetChangeInput{Id: resp.ChangeInfo.Id}
-	return c.api.WaitUntilResourceRecordSetsChanged(wreq)
+		log.Info(fmt.Sprintf("Waiting for change request %s", *resp.ChangeInfo.Id))
+		wreq := &route53.GetChangeInput{Id: resp.ChangeInfo.Id}
+		if err := c.api.WaitUntilResourceRecordSetsChanged(wreq); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // checkZone verifies zone information for the given domain.
@@ -137,21 +149,14 @@ func (c *route53Client) findZoneID(name string) (string, error) {
 }
 
 // computeChanges creates DNS changes for the given record.
-func (c *route53Client) computeChanges(name string, records map[string]string) ([]*route53.Change, error) {
+func (c *route53Client) computeChanges(name string, records map[string]string, existing map[string][]string) []*route53.Change {
 	// Convert all names to lowercase.
 	lrecords := make(map[string]string, len(records))
 	for name, r := range records {
 		lrecords[strings.ToLower(name)] = r
 	}
 	records = lrecords
 
-	// Get existing records.
-	existing, err := c.collectRecords(name)
-	if err != nil {
-		return nil, err
-	}
-	log.Info(fmt.Sprintf("Found %d TXT records", len(existing)))
-
 	var changes []*route53.Change
 	for path, val := range records {
 		ttl := 1
@@ -183,7 +188,49 @@ func (c *route53Client) computeChanges(name string, records map[string]string) (
 		log.Info(fmt.Sprintf("Deleting %s = %q", path, combineTXT(values)))
 		changes = append(changes, newTXTChange("DELETE", path, 1, values))
 	}
-	return changes, nil
+
+	sortChanges(changes)
+	return changes
+}
+
+// sortChanges ensures DNS changes are in leaf-added -> root-changed -> leaf-deleted order.
+func sortChanges(changes []*route53.Change) {
+	score := map[string]int{"CREATE": 1, "UPSERT": 2, "DELETE": 3}
+	sort.Slice(changes, func(i, j int) bool {
+		if *changes[i].Action == *changes[j].Action {
+			return *changes[i].ResourceRecordSet.Name < *changes[j].ResourceRecordSet.Name
+		}
+		return score[*changes[i].Action] < score[*changes[j].Action]
+	})
+}
+
+// splitChanges splits up DNS changes such that each change batch
+// is smaller than the given RDATA limit.
+func splitChanges(changes []*route53.Change, limit int) [][]*route53.Change {
+	var batches [][]*route53.Change
+	var batchSize int
+	for _, ch := range changes {
+		// Start new batch if this change pushes the current one over the limit.
+		size := changeSize(ch)
+		if len(batches) == 0 || batchSize+size > limit {
+			batches = append(batches, nil)
+			batchSize = 0
+		}
+		batches[len(batches)-1] = append(batches[len(batches)-1], ch)
+		batchSize += size
+	}
+	return batches
+}
+
+// changeSize returns the RDATA size of a DNS change.
+func changeSize(ch *route53.Change) int {
+	size := 0
+	for _, rr := range ch.ResourceRecordSet.ResourceRecords {
+		if rr.Value != nil {
+			size += len(*rr.Value)
+		}
+	}
+	return size
 }
 
 // collectRecords collects all TXT records below the given name.

cmd/devp2p/dns_route53_test.go

@@ -0,0 +1,124 @@
+package main
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/service/route53"
+)
+
+// This test checks that computeChanges/splitChanges create DNS changes in
+// leaf-added -> root-changed -> leaf-deleted order.
+func TestRoute53ChangeSort(t *testing.T) {
+	testTree0 := map[string][]string{
+		"fdxn3sn67na5dka4j2gok7bvqi.n": {"enrtree-branch:"},
+		"n":                            {"enrtree-root:v1 e=FDXN3SN67NA5DKA4J2GOK7BVQI l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=0 sig=Me7k32DXwG06XmqzNW4b3opMrGXQV9Atv6hmJrVPuNx0QZ-PugeoYlMMndDxyN8cOkWGCxOze26WOyD2JAsAvgE"},
+	}
+
+	testTree1 := map[string]string{
+		"n":                            "enrtree-root:v1 e=JWXYDBPXYWG6FX3GMDIBFA6CJ4 l=C7HRFPF3BLGF3YR4DY5KX3SMBE seq=1 sig=o908WmNp7LibOfPsr4btQwatZJ5URBr2ZAuxvK4UWHlsB9sUOTJQaGAlLPVAhM__XJesCHxLISo94z5Z2a463gA",
+		"C7HRFPF3BLGF3YR4DY5KX3SMBE.n": "enrtree://AM5FCQLWIZX2QFPNJAP7VUERCCRNGRHWZG3YYHIUV7BVDQ5FDPRT2@morenodes.example.org",
+		"JWXYDBPXYWG6FX3GMDIBFA6CJ4.n": "enrtree-branch:2XS2367YHAXJFGLZHVAWLQD4ZY,H4FHT4B454P6UXFD7JCYQ5PWDY,MHTDO6TMUBRIA2XWG5LUDACK24",
+		"2XS2367YHAXJFGLZHVAWLQD4ZY.n": "enr:-HW4QOFzoVLaFJnNhbgMoDXPnOvcdVuj7pDpqRvh6BRDO68aVi5ZcjB3vzQRZH2IcLBGHzo8uUN3snqmgTiE56CH3AMBgmlkgnY0iXNlY3AyNTZrMaECC2_24YYkYHEgdzxlSNKQEnHhuNAbNlMlWJxrJxbAFvA",
+		"H4FHT4B454P6UXFD7JCYQ5PWDY.n": "enr:-HW4QAggRauloj2SDLtIHN1XBkvhFZ1vtf1raYQp9TBW2RD5EEawDzbtSmlXUfnaHcvwOizhVYLtr7e6vw7NAf6mTuoCgmlkgnY0iXNlY3AyNTZrMaECjrXI8TLNXU0f8cthpAMxEshUyQlK-AM0PW2wfrnacNI",
+		"MHTDO6TMUBRIA2XWG5LUDACK24.n": "enr:-HW4QLAYqmrwllBEnzWWs7I5Ev2IAs7x_dZlbYdRdMUx5EyKHDXp7AV5CkuPGUPdvbv1_Ms1CPfhcGCvSElSosZmyoqAgmlkgnY0iXNlY3AyNTZrMaECriawHKWdDRk2xeZkrOXBQ0dfMFLHY4eENZwdufn1S1o",
+	}
+
+	wantChanges := []*route53.Change{
+		{
+			Action: sp("CREATE"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("2xs2367yhaxjfglzhvawlqd4zy.n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp(`"enr:-HW4QOFzoVLaFJnNhbgMoDXPnOvcdVuj7pDpqRvh6BRDO68aVi5ZcjB3vzQRZH2IcLBGHzo8uUN3snqmgTiE56CH3AMBgmlkgnY0iXNlY3AyNTZrMaECC2_24YYkYHEgdzxlSNKQEnHhuNAbNlMlWJxrJxbAFvA"`),
+				}},
+				TTL:  ip(2147483647),
+				Type: sp("TXT"),
+			},
+		},
+		{
+			Action: sp("CREATE"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("c7hrfpf3blgf3yr4dy5kx3smbe.n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp(`"enrtree://AM5FCQLWIZX2QFPNJAP7VUERCCRNGRHWZG3YYHIUV7BVDQ5FDPRT2@morenodes.example.org"`),
+				}},
+				TTL:  ip(2147483647),
+				Type: sp("TXT"),
+			},
+		},
+		{
+			Action: sp("CREATE"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("h4fht4b454p6uxfd7jcyq5pwdy.n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp(`"enr:-HW4QAggRauloj2SDLtIHN1XBkvhFZ1vtf1raYQp9TBW2RD5EEawDzbtSmlXUfnaHcvwOizhVYLtr7e6vw7NAf6mTuoCgmlkgnY0iXNlY3AyNTZrMaECjrXI8TLNXU0f8cthpAMxEshUyQlK-AM0PW2wfrnacNI"`),
+				}},
+				TTL:  ip(2147483647),
+				Type: sp("TXT"),
+			},
+		},
+		{
+			Action: sp("CREATE"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("jwxydbpxywg6fx3gmdibfa6cj4.n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp(`"enrtree-branch:2XS2367YHAXJFGLZHVAWLQD4ZY,H4FHT4B454P6UXFD7JCYQ5PWDY,MHTDO6TMUBRIA2XWG5LUDACK24"`),
+				}},
+				TTL:  ip(2147483647),
+				Type: sp("TXT"),
+			},
+		},
+		{
+			Action: sp("CREATE"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("mhtdo6tmubria2xwg5ludack24.n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp(`"enr:-HW4QLAYqmrwllBEnzWWs7I5Ev2IAs7x_dZlbYdRdMUx5EyKHDXp7AV5CkuPGUPdvbv1_Ms1CPfhcGCvSElSosZmyoqAgmlkgnY0iXNlY3AyNTZrMaECriawHKWdDRk2xeZkrOXBQ0dfMFLHY4eENZwdufn1S1o"`),
+				}},
+				TTL:  ip(2147483647),
+				Type: sp("TXT"),
+			},
+		},
+		{
+			Action: sp("UPSERT"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp(`"enrtree-root:v1 e=JWXYDBPXYWG6FX3GMDIBFA6CJ4 l=C7HRFPF3BLGF3YR4DY5KX3SMBE seq=1 sig=o908WmNp7LibOfPsr4btQwatZJ5URBr2ZAuxvK4UWHlsB9sUOTJQaGAlLPVAhM__XJesCHxLISo94z5Z2a463gA"`),
+				}},
+				TTL:  ip(1),
+				Type: sp("TXT"),
+			},
+		},
+		{
+			Action: sp("DELETE"),
+			ResourceRecordSet: &route53.ResourceRecordSet{
+				Name: sp("fdxn3sn67na5dka4j2gok7bvqi.n"),
+				ResourceRecords: []*route53.ResourceRecord{{
+					Value: sp("enrtree-branch:"),
+				}},
+				TTL:  ip(1),
+				Type: sp("TXT"),
+			},
+		},
+	}
+
+	var client route53Client
+	changes := client.computeChanges("n", testTree1, testTree0)
+	if !reflect.DeepEqual(changes, wantChanges) {
+		t.Fatalf("wrong changes (got %d, want %d)", len(changes), len(wantChanges))
+	}
+
+	wantSplit := [][]*route53.Change{
+		wantChanges[:4],
+		wantChanges[4:7],
+	}
+	split := splitChanges(changes, 600)
+	if !reflect.DeepEqual(split, wantSplit) {
+		t.Fatalf("wrong split batches: got %d, want %d", len(split), len(wantSplit))
+	}
+}
+
+func sp(s string) *string { return &s }
+func ip(i int64) *int64   { return &i }