uptane / aktualizr

C++ Uptane Client
Mozilla Public License 2.0
16 stars 16 forks source link

Introduce finer-grained cancellation of operations #107

Closed cajun-rat closed 6 months ago

cajun-rat commented 7 months ago

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 commented 6 months ago

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 commented 6 months ago

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!

cajun-rat commented 6 months ago

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

pattivacek commented 6 months ago

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.