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

Introduce finer-grained cancellation of operations #107

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

cajun-rat
Copy link
Collaborator

Push the FlowControlToken (which allows aborts of in-progress operations in the CommandQueue) further down the call stack.

This is one part of allowing offline and online updates to co-exist: if an online update is in progress over a very slow GSM modem, then it should be possible to plug in a USB stick, cancel the in-progress update and start installing the new one inside a few seconds.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

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

Thanks for the test!

Just to be sure: canceling IPSecondary updates isn't yet supported, but canceling even the metadata download on the Primary is. Is it possible to test specifically that part as well? Maybe isn't worth it, I dunno, just checking.

@cajun-rat
Copy link
Collaborator Author

Just to be sure: canceling IPSecondary updates isn't yet supported, but canceling even the metadata download on the Primary is.

Yep. The theory is that once the update is on the device, the time to install it is bounded by some known run time due to the internal device architecture and the size of the update, whereas downloading it from the network might be over an arbitrarily slow connection.

There is also a practical aspect which is that I'm not actively using IPSecondary updates and don't have a test rig for them :) The idea of the FlowControlToken is the callee checks it when practical, so the current IPSecondary implementation meets the API, it just doesn't respond 'quickly' (aka 'at all')

Is it possible to test specifically that part as well? Maybe isn't worth it, I dunno, just checking.

I'll have a go!

@pattivacek
Copy link
Collaborator

There is also a practical aspect which is that I'm not actively using IPSecondary updates and don't have a test rig for them

You can test them purely in software, you don't need an "actual" Secondary to test them. However, they're still using the hack in which they download the actual update directly via the internet instead of via the Primary. At any rate, I don't think you need to worry about it, I'm pretty sure the only people using them in production are unu, and we won't be supporting offline updates anytime soon, so it's fine.

The idea of the FlowControlToken is the callee checks it when practical, so the current IPSecondary implementation meets the API, it just doesn't respond 'quickly' (aka 'at all')

That sounds just fine for now!

Push the FlowControlToken (which allows aborts of in-progress operations
in the CommandQueue) further down the call stack.

This is one part of allowing offline and online updates to co-exist:
if an online update is in progress over a very slow GSM modem, then it
should be possible to plug in a USB stick, cancel the in-progress update
and start installing the new one inside a few seconds.
@cajun-rat
Copy link
Collaborator Author

Is it possible to test specifically that part as well? Maybe isn't worth it, I dunno, just checking.

I'll have a go!

So it turns out I already wrote this test in the forked version but missed it when I was creating this PR. I've added it now, in
src/libaktualizr/uptane/uptane_cancellation_test.cc

@cajun-rat cajun-rat merged commit 513aadf into uptane:master Mar 12, 2024
5 checks passed
@pattivacek
Copy link
Collaborator

So it turns out I already wrote this test in the forked version but missed it when I was creating this PR. I've added it now

Nice, thanks! Looks great.

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