Skip to content

Rate limitting, atomic transactions, and batching #154

@gucci-on-fleek

Description

@gucci-on-fleek

/cc @mholt

A few semi-related thoughts:

Rate limitting

Essentially every provider will have a rate limit of some sort. If you hit the rate limit, should the provider catch that itself and automatically retry, or should it just return an error and make that the user's problem?

The provider is probably in a better place to handle this since it can catch a 429 Too Many Requests and wait specifically until Retry-After; however, if the API that the provider calls doesn't send a Retry-After, then the provider will have to do exponential backoff (or something similar), which could block for a very long time and potentially surprise any downstream users.

Atomic transactions

Should we guarantee that SetRecords is atomic/a complete transaction? That is, if err == nil, then all of the changes in SetRecords were applied; and if err != nil, then calling SetRecords didn't change the zone at all.

Many providers will need to loop over the records passed to SetRecords and make multiple individual API calls to set the records. If the provider fails to set one of these records, should we require it to go back and revert all of the changes that it made in earlier loop iterations? And what should it do if it gets an error while reverting a change?

I think that having SetRecords be atomic is the only sensible solution, since otherwise err != nil means that the zone is in a completely unknown state. But implementing these semantics would be pretty tricky for some libdns providers if the upstream APIs that they call don't provide the necessary primitives.

Batched updates

Should we add a function that allows you to collect multiple updates in a single batch? Right now, you can combine multiple SetRecords and AppendRecords calls into a single SetRecords call (possibly preceded by a GetRecords call), but there's no way to combine this with any DeleteRecords calls.

Without a method that lets you batch multiple actions into a single atomic transaction, then downstream users would need code like this:

// Assume the zone begins with the following contents:
//
//	alpha  3600  IN  A      192.0.2.111
//	beta   3600  IN  A      192.0.2.222
//	www    3600  IN  CNAME  alpha
//
// and you want for the final contents to be:
//
//	beta  3600  IN  A  192.0.2.111
//	www   3600  IN  A  192.0.2.111
//	www   3600  IN  A  192.0.2.222
//
// You want this to either fully succeed or fully fail.
func AtomicSetZone() error {
	provider := &Provider{}
	ctx := context.Background()
	zone := "example.com."

	records, err := provider.GetRecords(ctx, zone)
	if err != nil {
		return err
	}

	_, err = provider.DeleteRecords(ctx, zone, []Record{
		RR{
			Name: "www",
			Type: "CNAME",
		},
	})
	if err != nil {
		for _, record := range records {
			rr, _ := record.RR()
			if rr.Name == "www" && rr.Type == "CNAME" {
				provider.SetRecords(ctx, zone, []Record{rr})
			}
		}
		return err
	}

	_, err = provider.SetRecords(ctx, zone, []Record{
		A{
			Name: "beta",
			TTL:  3600,
			IP:   netip.MustParseAddr("192.0.2.111"),
		},
	})
	if err != nil {
		for _, record := range records {
			rr, _ := record.RR()
			if rr.Name == "beta" && rr.Type == "A" {
				provider.SetRecords(ctx, zone, []Record{rr})
			}
			if rr.Name == "www" && rr.Type == "CNAME" {
				provider.SetRecords(ctx, zone, []Record{rr})
			}
		}
		return err
	}

	_, err = provider.AppendRecords(ctx, zone, []Record{
		A{
			Name: "www",
			TTL:  3600,
			IP:   netip.MustParseAddr("192.0.2.111"),
		},
		A{
			Name: "www",
			TTL:  3600,
			IP:   netip.MustParseAddr("192.0.2.222"),
		},
	})
	if err != nil {
		provider.DeleteRecords(ctx, zone, []Record{
			RR{
				Name: "www",
				Type: "A",
			},
		})
		for _, record := range records {
			rr, _ := record.RR()
			if rr.Name == "beta" && rr.Type == "A" {
				provider.SetRecords(ctx, zone, []Record{rr})
			}
			if rr.Name == "www" && rr.Type == "CNAME" {
				provider.SetRecords(ctx, zone, []Record{rr})
			}
		}
		return err
	}

	return nil
}

But if we had a batching/atomic transaction function, then downstream users would only need something like this:

func AtomicSetZone() error {
	provider := &Provider{}
	ctx := context.Background()
	zone := "example.com."

	_, err := provider.ActRecords(ctx, zone,
		[]Action{
			Delete{[]Record{
				RR{
					Name: "www",
					Type: "CNAME",
				},
			}},

			Set{[]Record{
				A{
					Name: "beta",
					TTL:  3600,
					IP:   netip.MustParseAddr("192.0.2.111"),
				},
			}},

			Append{[]Record{
				A{
					Name: "www",
					TTL:  3600,
					IP:   netip.MustParseAddr("192.0.2.111"),
				},
				A{
					Name: "www",
					TTL:  3600,
					IP:   netip.MustParseAddr("192.0.2.222"),
				},
			}},
		})

	return err
}

I would guess that most of the upstream APIs that the libdns providers call offer some way to send multiple updates together in a single API call, so this should be fairly easy to implement for them. And for libdns providers where the upstream API doesn't offer something like this, we could offer a helper function that will loop through the list of actions, call (Get/Set/Append)Records as necessary, and if any of them fails, loop through the actions and reverse them.

This would also reduce the likelihood of being rate limited since the libdns provider will need to make fewer calls to its upstream API, if the API supports multiple actions in one call.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions