refactor(storage): report actual bytes written and seeked (vs position) #538
Conversation
Writes are not guaranteed to be sequential with bmap, we should therefore report bytes that were written to the storage instead of the position within the storage. [Felix: also account seeks into writes to fullfill written==size assertion on client side. Downgrade to refactoring] Suggested by: Felix Moessbauer <[email protected]> Signed-off-by: Cedric Hombourger <[email protected]>
|
This replaces #495. To remain API compatible, we also account seeks into the writes. But at least this is a start to get rid of the global storage state. |
Signed-off-by: Felix Moessbauer <[email protected]>
|
@chombourger My plan for this PR is to completely replace the Once we have storage status ( |
We already get the progress via the event channel and also call the progress reporting callback there. By that, we no longer explicitly need to check the storage status for progress (but we need to do it for correctness / status). We now drop the callback invocations along with the now empty progress method. Signed-off-by: Felix Moessbauer <[email protected]>
The agent.storage_flush is a blocking call that waits for completion of the storage write operation. By that, there is no need to continously check for the storage status after returning. Signed-off-by: Felix Moessbauer <[email protected]>
In a617348, a final storage write event was added to ensure the progress bar completes instead of stopping below 100%. This however broke the state machine of the web UI, as it uses the INITIALIZED / CORRUPTED event to inform the user about the termination of the storage write. We fix this by moving the final write event into the writer itself and prior to the termination storage event. A nice side effect is a simplified code with less cross-module calls. Fixes: a617348 ("perf(cli): use event channel for "storage write") Signed-off-by: Felix Moessbauer <[email protected]>
We only need to report if the storage write operation was successful or not. As we already get this information from the storage_flush method, we don't need to call storage_status. By that we can't report the exact position at which the storage write failed, but if this is of any interest in the future, we still can get it from the final storage write event. Signed-off-by: Felix Moessbauer <[email protected]>
fmoessbauer
force-pushed
the
storage-bytes-written
branch
from
aecb994 to
91d988d
Compare
|
@chombourger This should not have any API breaking changes (at least not compared to the previous MTDA version). I would like to merge this prior to bigger refactorings, as this helps to reduce the review effort. |
|
@chombourger I have one last commit that fixes the non-linearity of the progress reporting in an ABI compatible way. Shall I add it here as well, or do you prefer a follow up MR? |
2 participants
Writes are not guaranteed to be sequential with bmap, we should therefore report bytes that were written to the storage instead of the position within the storage.
[Felix: also account seeks into writes to fullfill written==size assertion on client side. Downgrade to refactoring]
Suggested by: Felix Moessbauer [email protected]