Skip to content

ROUTE53: Rewrite to use diff2#2012

Merged
tlimoncelli merged 25 commits intomasterfrom
tlim_r53
Jan 31, 2023
Merged

ROUTE53: Rewrite to use diff2#2012
tlimoncelli merged 25 commits intomasterfrom
tlim_r53

Conversation

@tlimoncelli
Copy link
Copy Markdown
Contributor

CC @tresni

It is roughly working! Now I just have to clean up the little stuff.

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

This is ready for review, @tresni !

It doesn't do batching (i.e. it sends 1 RPC per change) but it passes all tests. I'd like to merge this (pending any feedback) then come back later and add batching. (Or, not add batching? I dunno how important it is.)

@tresni
Copy link
Copy Markdown
Contributor

tresni commented Jan 30, 2023

R53's API limits are very low (5 requests per second). For mass operations, batching is basically a requirement.

Retries may alleviate this for some cases, but I'm not sure it would be a good strategy overall. (e.g. if you have 1000 records changing, without batching you are looking at ~5 minutes to process all changes, vs a potential single request with batching.)

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

ok, good point!

I've added batching support. Time went from 1m35s to 1m25s. However if I include the tests like 601 and 1201 the time goes from 20m to 1.5. A huge improvement!

It was extremely easy to add batched support because of the existing batcher support. According to git blame, I that was added by @jtdoepke. Thanks!

This is ready for review. Lemme know what you think!

@tlimoncelli tlimoncelli merged commit d8d25bf into master Jan 31, 2023
@tlimoncelli tlimoncelli deleted the tlim_r53 branch January 31, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants