Closed sanchda closed 2 weeks ago
Benchmark execution time: 2024-11-13 03:12:19
Comparing candidate commit f8021249 in PR branch sanchda/fix_crashtracking_socket
with baseline commit 873ea858 in branch main
.
Found 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 2 unstable metrics.
Omitted due to size.
Attention: Patch coverage is 7.76699%
with 95 lines
in your changes missing coverage. Please review.
Project coverage is 71.06%. Comparing base (
873ea85
) to head (f802124
).
Don't merge yet, need to verify the thing works end-to-end with a native test
Looking good, thank you @bwoebi!
I did some local tests and found that it's possible for the collector to lock up indefinitely if the receiver should ever hit a condition where it doesn't drain the unix domain socket (the write will block in the syscall), but I only discovered that by accident (custom receiver implementation with a bug).
Except for freezing the receiver, I can't really think of a condition which would allow this to arise in normal code. I have an incoming PR which will push the coordinator into its own forked process, allowing the original process to keep track of its time budget. That will fix the cause of these (speculative!!!) hangs.
What does this PR do?
In the patches leading up to v14, I had tried to consolidate some of the receiver code paths. In so doing, I accidentally removed the entire ability for a collector to speak with an async receiver.
Sorry, PHP!
Rather than adding the same
ddog_crasht_init_with_unix_socket
as before, this patch adds a newddog_crasht_init_without_receiver
interface. All this does is check that the passedconfig
has a validunix_socket_path
.Motivation
Fixing past mistakes.