Improve other_operation_in_progress error users, fix handling of its delays
#6588
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ion_in_progress error Compare before, where it's unclear which operation is precluding progress: ``` Caught exception while marking SR for VDI.clone in message forwarder: OTHER_OPERATION_IN_PROGRESS: [ SR; OpaqueRef:d019f26a-b2e8-529b-bb66-fd4f008d4f82; VDI.clone ] ``` And after, where the ID of the operation makes it easier to navigate the logs: ``` Caught exception while marking SR for VDI.clone in message forwarder: OTHER_OPERATION_IN_PROGRESS: [ SR; OpaqueRef:b0f54a40-485f-f48f-fdc7-6db28a64d3fd; scan; OpaqueRef:832fb22d-2ce5-0d26-8a00-a4165e78b34d ] ``` Also, avoid iterating over all the current_operations when looking for non-parallelisable ops, stop on the first one. Report the non-parallelisable op instead of the first current operation as the reason for the error as well. Signed-off-by: Andrii Sultanov <[email protected]>
last-genius
force-pushed
the
asv/improve-other-operation-logs
branch
from
b122c68 to
b332213
Compare
psafont
reviewed
Jul 14, 2025
| set_errors Api_errors.other_operation_in_progress | ||
| ["VBD"; _ref; vbd_operations_to_string (List.hd current_ops)] | ||
| [`unpause] ; | ||
| let set_difference a b = List.filter (fun (_, x) -> not (List.mem x b)) a in |
There was a problem hiding this comment.
Listext.List.set_difference is used further up ahead: you could replace all users, or keep using listext here.
There was a problem hiding this comment.
this is a reimplementation of set_difference for this particular use case - I need to ignore the reference, only comparing operation types.
psafont
reviewed
Jul 14, 2025
| @@ -277,7 +282,15 @@ let check_operation_error ~__context ?sr_records:_ ?(pbd_records = []) | |||
| mechanism of message forwarding and only use the event loop. *) | |||
| my_has_current_operation_vbd_records <> [] && op <> `data_destroy | |||
| then | |||
| Error (Api_errors.other_operation_in_progress, ["VDI"; _ref]) | |||
| let op_ref, op_type = | |||
| List.hd | |||
There was a problem hiding this comment.
two more list.hd, a bit awkward that is has to be coded this way
There was a problem hiding this comment.
I agree, but sadly replacing this with match over options as elsewhere makes the logic more inscrutable, so I've kept it as is.
psafont
approved these changes
Jul 14, 2025
changlei-li
reviewed
Jul 16, 2025
other_operation_in_progress has separate fields for the main class of the
object ("Cluster" or "Cluster_host") and the operation that's blocking
progress, do not concatenate these into one string. Also report the task ref
that's blocking progress.
Signed-off-by: Andrii Sultanov <[email protected]>
other_operation_in_progress has separate fields for the main class of the
object ("VM") and the operation that's blocking progress, do not concatenate
these into one string. Also report the task ref that's blocking progress.
Signed-off-by: Andrii Sultanov <[email protected]>
…n_in_progress Fix incorrect operations being reported - e.g. when other_operation_in_progress is reported only when there are non-parallelisable operations currently happening, we can't just report the first current op, we need to report the first *non-parallelisable* op. This required some refactoring, moving to options instead of bools, etc. Additionally report the blocking operation's reference when raising the error. Signed-off-by: Andrii Sultanov <[email protected]>
…_in_progress Signed-off-by: Andrii Sultanov <[email protected]>
…other_operation_in_progress Instead of logging all the blocking operations with 'debug', just report them with the error. Signed-off-by: Andrii Sultanov <[email protected]>
…progress Signed-off-by: Andrii Sultanov <[email protected]>
…rogress error Signed-off-by: Andrii Sultanov <[email protected]>
…rror Signed-off-by: Andrii Sultanov <[email protected]>
…ation_in_progress error Signed-off-by: Andrii Sultanov <[email protected]>
…gress error Signed-off-by: Andrii Sultanov <[email protected]>
…on_in_progress error Signed-off-by: Andrii Sultanov <[email protected]>
Not all of other_operation_in_progress error contain two objects in the list ([cls; objref]), some contain three or four, but the match would miss them and they would be put into a non-interruptible Thread.sleep instead of the interruptible Delay.sleep that provides a mechanism for the other task to wake us up on its way out. Signed-off-by: Andrii Sultanov <[email protected]>
…progress Most of the users report these when appropriate, so document them here. Signed-off-by: Andrii Sultanov <[email protected]>
quality-gate: Reduce expected List.hd count test_clustering: test for the blocking operation to be reported Signed-off-by: Andrii Sultanov <[email protected]>
last-genius
force-pushed
the
asv/improve-other-operation-logs
branch
from
b332213 to
9be1b7d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
other_operation_in_progressis reported inconsistently, with some users only specifying[class; class_ref], and others alsoop_type. Make it consistent, changing all users to report[class; class_ref; op_type; op_ref]. This makes it much easier to navigate the logs, since the operation blocking progress is clearly referenced.Compare before, where it's unclear which operation is precluding progress:
And after, with the ID of the operation present:
In addition, fix incorrect operation reporting, so that when progress is blocked only when particular types of operations are present in
current_operations, these specific operations are reported, and not just any current operation.Also fix handling of
other_operation_in_progressdelays - some of the users would miss thematcharm and use the non-interruptibleThread.sleepinstead of the interruptibleDelay.sleepthat provides a mechanism for the blocking task to wake us up on its way out.