http-lib: add backtrace to logs on connection without response #6028
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
robhoes
approved these changes
Oct 1, 2024
|
Backport at xapi-project/xen-api-libs-transitional#117 |
lindig
approved these changes
Oct 2, 2024
snwoods
approved these changes
Oct 3, 2024
|
I'm changing the tests around the http client and server to try to test this change |
psafont
force-pushed
the
logconn
branch
3 times, most recently
from
9b5060a to
a757166
Compare
|
Note that the D.Debug is not printed in the default configuration, but the backtraces will be printed. I believe this should not not usually cause logspam as connections are only established to known servers, so parse error (and other unknown exceptions) should be rare in practice/ |
robhoes
reviewed
Oct 7, 2024
| (package xapi) | ||
| (deps | ||
| test_client.exe | ||
| test_server.exe | ||
| client_server_test.sh |
There was a problem hiding this comment.
how does it find the new test_client_server.t?
There was a problem hiding this comment.
The cram atom processes all the .t files in the directory
robhoes
approved these changes
Oct 8, 2024
Signed-off-by: Pau Ruiz Safont <[email protected]>
Gives more flexibility in tests. Now the results from the client aren't printed, but weren't important to pass the test anyway. Signed-off-by: Pau Ruiz Safont <[email protected]>
Current behaviour for displaying stats is done with the --perf parameter Signed-off-by: Pau Ruiz Safont <[email protected]>
While this does not exercise the exact error that can happen in long migrations, it gets logged in a similar way. There's no easy way to trigger the issue, the best chance is to send a malformed response to trigger a Parse_error. I did modify the code in http_client and verified that current code can produce the logging, with backtraces successfully, when set up properly (like in the test client) Signed-off-by: Pau Ruiz Safont <[email protected]>
No functional difference Signed-off-by: Pau Ruiz Safont <[email protected]>
Taking measurements in practice doesn't lead to improved accuracy. Also change the tests so more than one sample is collected and can know how noisy the measurements really are. Here's an example of a run, including the result before the change: ``` $ ./test_client.exe --perf - 1 thread non-persistent connections: 4896.0 +/- 0.0 RPCs/sec - 1 thread non-persistent connections (query): 4811.0 +/- 0.0 RPCs/sec - 10 threads non-persistent connections: 7175.0 +/- 0.0 RPCs/sec - 1 thread persistent connection: 16047.0 +/- 0.0 RPCs/sec - 10 threads persistent connections: 7713.0 +/- 0.0 RPCs/sec + 1 thread non-persistent connections: 5042.0 +/- 247.5 RPCs/sec + 1 thread non-persistent connections (query): 5173.0 +/- 216.0 RPCs/sec + 10 threads non-persistent connections: 7678.0 +/- 2241.2 RPCs/sec + 1 thread persistent connection: 21814.0 +/- 2124.6 RPCs/sec + 10 threads persistent connections: 10154.0 +/- 2461.9 RPCs/sec ``` Signed-off-by: Pau Ruiz Safont <[email protected]>
lindig
approved these changes
Oct 10, 2024
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.
Some long-running migrations stop because of a loss of connection, log more information when it happens.
I couldn't find a way to get the backtrace to be printed in a nice way without adding too much code, this also makes the change backportable.
I would prefer to log it at a debug level, but the function doesn't expose it, and it would complicate backpoerting as well.