Skip to content
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

Running out of file descriptors due to Mutex.pm #8

Closed
tm604 opened this issue Nov 23, 2023 · 1 comment
Closed

Running out of file descriptors due to Mutex.pm #8

tm604 opened this issue Nov 23, 2023 · 1 comment

Comments

@tm604
Copy link

tm604 commented Nov 23, 2023

The current implementation of lib/OpenTelemetry/SDK/Trace/Span.pm includes a $lock Mutex, which by default uses a pipe()-based mutex mechanism.

This leads to a lot of filedescriptor churn, and when multiple spans are active (particularly likely with async code) it's easy to hit the FD limits.

Although Mutex is used elsewhere - batches, for example - that seems less problematic.

Since Mutex.pm introduces considerable overhead there, and there should be no need for locking for the span itself (are those ever expected to be shared between processes/threads?), perhaps this could be configurable? Either the ability to skip the mutex-lock, or perhaps a lighter-weight alternative for similar protections.

@jjatria
Copy link
Owner

jjatria commented Nov 23, 2023

I'm not sure how we could make this configurable (I mean, we either need it or we don't), but it might be that we should remove it. The OTel specification says this about the span's end method (which is the only one using the lock at the moment):

Expect this operation to be called in the “hot path” of production applications. It needs to be designed to complete fast, if not immediately. This operation itself MUST NOT perform blocking I/O on the calling thread. Any locking used needs be minimized and SHOULD be removed entirely if possible.

That said: the compliance matrix does state that span methods should be "safe for concurrent calls", so there may be a tension there.

If this can be alleviated with a different lock implementation (Future::Mutex, maybe?) then that might be another possibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants