progress-fd issue tracker #1016
Comments
|
Hmmm fundamentally if we make sure it does not block and is stateless, isn't it enough if the pipe fits a single message? ie size is less than 60k? Then the client can read it at its own pace, up to 5hz? Then, even if its more than 65k, to make sure it blocks correctly you could spin up a thread that writes the current progress when the pipe has space if it has changed and e.g., at a max speed of 5Hz? This way the client could read the state every 2 seconds without affecting the update. The progress bar I did works really well. I will add three issues with the protocol for tracking though. Blind UpdatesRight now, you're supposed to do Blind RebasesSame as above, Would be nice to replace the hash with the version or somehow include it in there. Can block updatesAs mentioned above, currently if the client does not read the output at 5hz, it can stall the update. Would be nice to fix that with e.g., a thread. I had to implement a thread to read the output so that this does not happen since I end up updating the UI at 1hz (updating the UI is very expensive). It would be ideal if callers can read the state on demand on their own pace. |
Compared to a thread wakeup or even calculating a single hash i think the overhead is minimal. If the update takes e.g., 10 minutes, even if the reader does all the updates thats 10 * 60 * 5 * 12k=36MB, where you currently dual hash 3GB-10GB of data to deploy an image (as both the layer and the files if derived need to be hashed). Focusing on downloading multiple layers at a time is more important IMO.
And at the point where you download multiple layers, it would become a bigger problem both for you as a sender as you have to deserialize that if its not normalized. I really like my 30 line code that works perfectly because its normalized. Let me correct what I said above, I update at 3Hz, which is plenty. |
I think we just need |
I don't understand what you mean here.
Yeah, I would say it's trading off code simplicity for consumers versus CPU usage for every client. Dunno. Since you kicked this off and have been driving it I am OK with the status quo...worst case we have to introduce something like |
Is there a stateful protocol that would work while there are 4-5 layers downloading that would work for both a client that wants to update 5x per second and a client that wants to update every 2 seconds? We did attempt a stateful variant in the PR.
Pretty much. I would contest how much that is though. Perhaps if we are talking about bootc being used in microprocessors. Although at that point, 100 layers? A modern UI with something like react when binding to the final JSON and rerendering would use 100-1000x more processing power than reading the json string. Both python and javascript would also use more processing power reconstructing the state if the protocol is stateful, as the json parser is compiled and would only require a single wakeup to read the whole thing. You still have |
|
It looks to me like the current |
The relationship or concern you have between state and update frequency is not obvious to me. Remember in the stateful version I did originally, we still always emitted an event for each byte-progress that was started or ended, so a client can't miss it if that's what you're thinking about. It's basically just: do we emit the prior history of steps each time or not. If we don't, then a client which wants to render that state needs to remember it. Note a terminal renderer wouldn't need to care because the terminal itself stores rendering history! But a GUI that wanted to show progress-bar per layer would need to hold that state. Which honestly I just don't think is very onerous... |
Yeah... hm thats unfortunate as you have both a subtasks and a steps variable so all of them would need to be renamed. Subtask is used within the tasks themselves so renaming subtask to step is not that clearcut. Would probably need to rename steps to subtasks instead. If this happens, then you need to find a name for holding the currently executing subtasks.
In your example you express concern for downloading 100 layers. If the process was stateful, you would have 205 blocking events over the course of the download (2 per layer + extras). You would also need to make sure that each of the 5 layers that is downloading gets 5 events per second ie you'd need to do 25 updates per second and make sure you are fair to all layers if there is a block With the stateless approach you have 0-3 depending on how its implemented. If the client uses my code you actually never have to block, as the last event can be used to show full progress state. This is because each stage has a fixed starting offset. My 2 cents is it would be too complicated for implementing both on the bootc side and client side |
|
OK #961 does two important things:
I'd like to get a new release out with just the few fixes that have landed, and then we can aim to focus on progress-fd and stabilize it in the next release. |
What should the behavior be if the cached digest is no longer available? Fail or fallback to non-cache upgrade? |
Taking inspiration from the Matrix protocol, what if we emit the prior history with an option of either the starting id(starting to latest), latest (single), or all? Each step could provide current steps and total steps. |
The protocol is unidirectional - we operate over a pipe. I don't see how we add any options today other than either something like |
(Or probably better |
|
A lot of related discussion in e.g. containers/podman#12341 (also see specifically containers/podman#20328) It'd clearly make sense for us to try to align with the Docker event schema as much as possible to make it easy for people to reuse rendering code. Now, if anyone can find actual docs on what specifically docker puts in its events, let me know |
https://github.com/containers/podman/blob/main/pkg/api/handlers/utils/images.go#L159 |
Followup to #921 which we merged, and I was hoping to do some post-merge followup on. This issue will track some of that followup.
--progress-fd#961size concerns
In the PR we kept going back and forth on whether the progress data should require the client to keep track of state or not. @antheas was calling this "normalization". There's clear simplicity in having the protocol be stateless.
However...looking at the layer progress (one of the most important phases) in a local run here during a download, we keep accumulating and emitting more data (completed subtasks) for each layer. In this run the final
ProgressBytesI got was 12 KiB which...is a bit more than I was expecting.Click for (prettified) example:
Does that size actually matter? Well...obviously we're talking about local IPC so 12k isn't large exactly, but we are also trying to emit this data at a relatively high frequency. Note that a default Linux pipe buffer is 64k, so if the reader slows down a bit that's only a maximum of 5 messages queued before we fill the pipe.
The size of this progress data will grow with the number of layers...let's say in the future we start using 100 layers (not at all an unreasonable thing!) that'd double to 25k of JSON etc.
This is data we're serializing and having the caller to deserialize ~5 times a second, competing with actual real work (like decompression, sha256 etc.).
OK well, benchmarking this obviously CPUs are super fast and python parses it in
44.4 usechere...but...still...it wouldn't be a huge burden at all to have a stateful protocol either.The text was updated successfully, but these errors were encountered: