Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dobby replicate images to regions #228

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Davis-A
Copy link
Member

@Davis-A Davis-A commented Mar 11, 2024

I'm not sure what should be handled by what. I can totally see an argument for Dobby having a method transfer_image_to_region(), note singular and have the InABox reactor handle transferring to multiple regions.

I can also see an argument to not change the box create code and instead have a timer that kicks off every 15 minutes to check if the image is in the right place and have it request the image transfer.

Basically I can think of a dozen ways so structure this and I'm not sure what the preferred option is.

I can also see an argument for this case is pretty rare, and keep it as having a human press the button if this happens.

Davis-A added 2 commits March 11, 2024 13:26
I'm not sure what should be handled by what.  I can totally see an
argument for Dobby having a method 'transfer_image_to_region()', note
singular and have the InABox reactor handle transferring to multiple
regions.

I can also see how ignoring those 422 errors is a bit weird to have by
default, might be worth having the method accept an argument on whether
to ignore specific errors or to just 'die "..." unless $res->is_success'
If on box create we detect that the image isn't available in all our
regions, request they be replicated there
@Davis-A Davis-A force-pushed the dobby-replicate-images-to-regions branch from ad7bd35 to 8495249 Compare March 11, 2024 02:26
@@ -260,6 +260,8 @@ async sub handle_create ($self, $event, $switches) {
join ', ',
grep { $snapshot_regions{$_} } $self->box_datacentres->@*;

await $self->dobby->transfer_image_to_regions($snapshot, $self->box_datacentres);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems okay... but won't immediately fix the problem since these transfers can take awhile. What might be better/nice would be if you tried to create it in a region that didn't exist, synergy said: "Hey, that region isn't currently available. I'll start a transfer, and let you know when it finishes. In the meantime, you could create it in a different region with /datacentre ..."

So then she'd need to handle the case where a transfer is already underway and track that instead of starting a new one... etc etc.. (And if more people requested creating boxes, she'd want to add them to the list of people to notify when the region transfer has succeeded...) And then she'd also have to account for noticing a newer inabox image has been created that is in all the regions, so the current waiting things don't matter and she should just notify people everything is good....

This is all... a bit much more work than what you have here, and maybe not worth it (though I like the idea).

So maybe what we do is use this, but have synergy say something like this:

        "I'm unable to create snapshot in region '$region'.  Available compatible regions are $compatible_regions." I've kicked off a transfer of fminabox to that region, and it should be available in the next hour or less."

@wolfsage
Copy link
Contributor

Yeah I'm not sure... We know roughly when new droplets are created nightly, so we could have synergy wake up once a day 2 hours after that and see if it's in all the regions, and if not attempt to fix it. And we could also have this thing here which will catch things earlier, maybe...

I think putting it in Dobby and having an argument to handle error checking or not differently makes sense. You could wrap that with two methods:

  • transfer_image_to_region_strict
  • transfer_image_to_region_lax

Or just transfer_image_to_region($image, $region, { allow_in_progress => 1 });

Your / rik's call :)

@rjbs rjbs force-pushed the main branch 4 times, most recently from aafd26b to eee1d40 Compare July 4, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants