-
Notifications
You must be signed in to change notification settings - Fork 284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http-lib: add backtrace to logs on connection without response #6028
base: master
Are you sure you want to change the base?
Conversation
Backport at xapi-project/xen-api-libs-transitional#117 |
I'm changing the tests around the http client and server to try to test this change |
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/ |
(package xapi) | ||
(deps | ||
test_client.exe | ||
test_server.exe | ||
client_server_test.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does it find the new test_client_server.t?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cram
atom processes all the .t
files in the directory
Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@cloud.com>
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 <pau.ruizsafont@cloud.com>
Current behaviour for displaying stats is done with the --perf parameter Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@cloud.com>
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 <pau.ruizsafont@cloud.com>
No functional difference Signed-off-by: Pau Ruiz Safont <pau.ruizsafont@cloud.com>
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 <pau.ruizsafont@cloud.com>
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.