IH-747 - Reduce pollution in migration logs #6186
Merged
Conversation
This file contains 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
|
Will run some tests over the weekend just to make sure I didn't accidentally touch something I shouldn't have, but overall there should be no intended functional changes here. |
robhoes
reviewed
Dec 13, 2024
robhoes
reviewed
Dec 13, 2024
ocaml/xapi/import.ml
Outdated
| Found_VBD vbd | ||
| | None -> ( | ||
| warn | ||
| "Did not find an already existing VBD with the same uuid=%s, try to \ |
There was a problem hiding this comment.
Or we didn't look for an existing one, if this isn't a full restore. Also I'd call this info, not warn.
There was a problem hiding this comment.
"Full restore" is an odd name when it really means "preserve uuids"...
last-genius
force-pushed
the
private/asultanov/get-uuid-opt
branch
2 times, most recently
from
9a7643d to
c3785a0
Compare
|
Addressed the suggestions |
Tests look good (4178964, 4178952) |
Vincent-lau
approved these changes
Dec 16, 2024
lindig
approved these changes
Dec 16, 2024
| with | ||
| | [r] -> | ||
| Some r | ||
| | _ -> |
There was a problem hiding this comment.
Is this some error condition that we would want to log?
There was a problem hiding this comment.
I think returning None is enough - it represents an error condition already
There was a problem hiding this comment.
Consumers ought to log its result if they think it important, but I hesitate to say we should introduce any more latency into the DB layer itself.
last-genius
force-pushed
the
private/asultanov/get-uuid-opt
branch
from
c3785a0 to
f4fe50d
Compare
Instead of logging errors like this and then immediately handling them: ``` [error|1141 |VM metadata import |import] Found no VDI with location = dedeeb44-62b3-460e-b55c-6de45ba10cc0: treating as fatal and abandoning import [debug|1141 |VM metadata import |import] Ignoring missing disk Ref:4 - this will be mirrored during a real live migration. ``` Log once in the handler: ``` [ warn|VM metadata import |import] Ignoring missing disk Ref:16 - this will be mirrored during a real live migration. (Suppressed error: 'Found no VDI with location = c208b47c-cf87-495f-bd3c-a4bc8167ef83') ``` Signed-off-by: Andrii Sultanov <[email protected]>
Instead of raising an exception in case of an error like get_by_uuid, return None to be handled gracefully later. Do not expose it in the datamodel. This will later be used when an object is checked to exist before its creation (during migration, for example), and so its absence is expected - no need to raise a backtrace and pollute the logs with errors. Signed-off-by: Andrii Sultanov <[email protected]>
…failure is expected
Migration logs are always full of exceptions that are expected and immediately
handled:
```
[error|backtrace] SR.get_by_uuid D:8651cc0c9fb6 failed with exception Db_exn.Read_missing_uuid("SR", "", "a94bf103-0169-6d70-8874-334261f5098e")
[error|backtrace] Raised Db_exn.Read_missing_uuid("SR", "", "a94bf103-0169-6d70-8874-334261f5098e")
[error|backtrace] 1/9 xapi Raised at file ocaml/database/db_cache_impl.ml, line 237
[error|backtrace] 2/9 xapi Called from file ocaml/xapi/db_actions.ml, line 13309
[error|backtrace] 3/9 xapi Called from file ocaml/xapi/rbac.ml, line 188
[error|backtrace] 4/9 xapi Called from file ocaml/xapi/rbac.ml, line 197
[error|backtrace] 5/9 xapi Called from file ocaml/xapi/server_helpers.ml, line 74
[error|backtrace] 6/9 xapi Called from file ocaml/xapi/server_helpers.ml, line 96
[error|backtrace] 7/9 xapi Called from file ocaml/libs/xapi-stdext/lib/xapi-stdext-pervasives/pervasiveext.ml, line 24
[error|backtrace] 8/9 xapi Called from file ocaml/libs/xapi-stdext/lib/xapi-stdext-pervasives/pervasiveext.ml, line 39
[error|backtrace] 9/9 xapi Called from file ocaml/libs/log/debug.ml, line 250
[ warn|import] Failed to find SR with UUID: a94bf103-0169-6d70-8874-334261f5098e content-type: user - will still try to find individual VDIs
[....]
[debug|import] Importing 1 VM_guest_metrics(s)
[debug|import] Importing 1 VM_metrics(s)
[debug|import] Importing 1 VM(s)
[debug|import] Importing 1 network(s)
[debug|import] Importing 0 GPU_group(s)
[debug|import] Importing 1 VBD(s)
[error|backtrace] VBD.get_by_uuid D:3a12311e8be4 failed with exception Db_exn.Read_missing_uuid("VBD", "", "026d61e9-ed8a-fc72-7fd3-77422585baff")
[error|backtrace] Raised Db_exn.Read_missing_uuid("VBD", "", "026d61e9-ed8a-fc72-7fd3-77422585baff")
[error|backtrace] 1/9 xapi Raised at file ocaml/database/db_cache_impl.ml, line 237
[error|backtrace] 2/9 xapi Called from file ocaml/xapi/db_actions.ml, line 14485
[error|backtrace] 3/9 xapi Called from file ocaml/xapi/rbac.ml, line 188
[error|backtrace] 4/9 xapi Called from file ocaml/xapi/rbac.ml, line 197
[error|backtrace] 5/9 xapi Called from file ocaml/xapi/server_helpers.ml, line 74
[error|backtrace] 6/9 xapi Called from file ocaml/xapi/server_helpers.ml, line 96
[error|backtrace] 7/9 xapi Called from file ocaml/libs/xapi-stdext/lib/xapi-stdext-pervasives/pervasiveext.ml, line 24
[error|backtrace] 8/9 xapi Called from file ocaml/libs/xapi-stdext/lib/xapi-stdext-pervasives/pervasiveext.ml, line 39
[error|backtrace] 9/9 xapi Called from file ocaml/libs/log/debug.ml, line 250
[debug|import] Importing 1 VIF(s)
[error|backtrace] VIF.get_by_uuid D:2bc78449e0bc failed with exception Db_exn.Read_missing_uuid("VIF", "", "7d14aee4-47a4-e271-4f64-fe9f9ef6d50b")
[error|backtrace] Raised Db_exn.Read_missing_uuid("VIF", "", "7d14aee4-47a4-e271-4f64-fe9f9ef6d50b")
[error|backtrace] 1/9 xapi Raised at file ocaml/database/db_cache_impl.ml, line 237
[error|backtrace] 2/9 xapi Called from file ocaml/xapi/db_actions.ml, line 10813
[error|backtrace] 3/9 xapi Called from file ocaml/xapi/rbac.ml, line 188
[error|backtrace] 4/9 xapi Called from file ocaml/xapi/rbac.ml, line 197
[error|backtrace] 5/9 xapi Called from file ocaml/xapi/server_helpers.ml, line 74
[error|backtrace] 6/9 xapi Called from file ocaml/xapi/server_helpers.ml, line 96
[error|backtrace] 7/9 xapi Called from file ocaml/libs/xapi-stdext/lib/xapi-stdext-pervasives/pervasiveext.ml, line 24
[error|backtrace] 8/9 xapi Called from file ocaml/libs/xapi-stdext/lib/xapi-stdext-pervasives/pervasiveext.ml, line 39
[error|backtrace] 9/9 xapi Called from file ocaml/libs/log/debug.ml, line 250
```
Use an internal get_by_uuid_opt call and match on the Option instead,
with the logs looking much clearer:
```
[debug|import] Importing 1 host(s)
[debug|import] Importing 2 SR(s)
[ warn|import] Failed to find SR with UUID: 8568e308-c61c-3b10-3953-45606cfecede content-type: - will still try to find individual VDIs
[ warn|import] Failed to find SR with UUID: 40e9e252-46ac-ed3d-7a4c-6db175212195 content-type: user - will still try to find individual VDIs
[...]
[debug|import] Importing 2 VM_guest_metrics(s)
[debug|import] Importing 2 VM(s)
[debug|import] Importing 1 network(s)
[debug|import] Importing 1 GPU_group(s)
[debug|import] Importing 4 VBD(s)
[ info|import] Did not find an already existing VBD with the same uuid=569d0e60-6a89-d1fa-2ed6-38b8eebe9065, try to create a new one
[ info|import] Did not find an already existing VBD with the same uuid=533306da-cff1-7ada-71f7-2c4de8a0065b, try to create a new one
[ info|import] Did not find an already existing VBD with the same uuid=f9dec620-0180-f67f-6711-7f9e5222a682, try to create a new one
[ info|import] Did not find an already existing VBD with the same uuid=05e55076-b559-9b49-c247-e7850984ddae, try to create a new one
[debug|import] Importing 2 VIF(s)
[ info|import] Did not find an already existing VIF with the same uuid=a5a731d5-622c-5ca5-5b2a-a0053a11ef07, try to create a new one
[ info|import] Did not find an already existing VIF with the same uuid=1738bf20-8d16-0d69-48cd-8f3d9e7ea791, try to create a new one
```
Signed-off-by: Andrii Sultanov <[email protected]>
last-genius
force-pushed
the
private/asultanov/get-uuid-opt
branch
from
f4fe50d to
81c2a6a
Compare
|
New version was tagged, so merging this (no functional changes intended, but still wanted to stay on the safe side) |
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.
Best reviewed by commit - the first one does not depend on database changes, only the last one does.
In short, simplifies and clarifies migration logs from this:
to this:
And from this:
to this: