sentry-java icon indicating copy to clipboard operation
sentry-java copied to clipboard

Virtual thread pinning on MainEventProcessor.ensureHostnameCache

Open pavelbelonosov opened this issue 1 year ago • 3 comments

Problem Statement

We use Java 21 and Spring Boot 3.2.4 with enabled virtual threads in plain web application. First request after app init leads to thread pinning: io.sentry.MainEventProcessor.ensureHostnameCache(MainEventProcessor.java:192) <== monitors:1. Subsequent requests are fine.

Thread[#143,ForkJoinPool-1-worker-1,5,CarrierThreads]
    java.base/java.lang.VirtualThread$VThreadContinuation.onPinned(VirtualThread.java:185)
    java.base/jdk.internal.vm.Continuation.onPinned0(Continuation.java:393)
    java.base/java.lang.VirtualThread.parkNanos(VirtualThread.java:631)
    java.base/java.lang.System$2.parkVirtualThread(System.java:2648)
    java.base/jdk.internal.misc.VirtualThreads.park(VirtualThreads.java:67)
    java.base/java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:267)
    java.base/java.util.concurrent.FutureTask.awaitDone(FutureTask.java:497)
    java.base/java.util.concurrent.FutureTask.get(FutureTask.java:203)
    io.sentry.HostnameCache.updateCache(HostnameCache.java:121)
    io.sentry.HostnameCache.<init>(HostnameCache.java:77)
    io.sentry.HostnameCache.<init>(HostnameCache.java:64)
    io.sentry.HostnameCache.<init>(HostnameCache.java:58)
    io.sentry.HostnameCache.getInstance(HostnameCache.java:52)
    io.sentry.MainEventProcessor.ensureHostnameCache(MainEventProcessor.java:192) <== monitors:1
    io.sentry.MainEventProcessor.setServerName(MainEventProcessor.java:181)
    io.sentry.MainEventProcessor.processNonCachedEvent(MainEventProcessor.java:132)
    io.sentry.MainEventProcessor.process(MainEventProcessor.java:146)
    io.sentry.SentryClient.processTransaction(SentryClient.java:416)
    io.sentry.SentryClient.captureTransaction(SentryClient.java:661)
    io.sentry.Hub.captureTransaction(Hub.java:700)
    io.sentry.SentryTracer.finish(SentryTracer.java:264)
    io.sentry.SentryTracer.finish(SentryTracer.java:565)
    io.sentry.SentryTracer.finish(SentryTracer.java:558)
    io.sentry.SentryTracer.finish(SentryTracer.java:553)
    io.sentry.spring.jakarta.tracing.SentryTracingFilter.doFilterWithTransaction(SentryTracingFilter.java:127)
    io.sentry.spring.jakarta.tracing.SentryTracingFilter.doFilterInternal(SentryTracingFilter.java:87)
    org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
    org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
    org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
    org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
    org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
    org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
    org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
    io.sentry.spring.jakarta.SentrySpringFilter.doFilterInternal(SentrySpringFilter.java:71)
    org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
    org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174)
    org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149)
    org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:167)
    org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90)
    org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:482)
    org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:115)
    org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
    org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:344)
    org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:391)
    org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63)
    org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:896)
    org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1744)
    org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52)
    java.base/java.lang.VirtualThread.run(VirtualThread.java:311)

This behavior is observed in Sentry 7.6.0 and 6.26.0

Solution Brainstorm

Would be nice if Sentry became Loom-friendly and replace synchronized methods to ReentrantLock for virtual thread support.

From this

private void ensureHostnameCache() {
    if (hostnameCache == null) {
      synchronized (this) {
        if (hostnameCache == null) {
          hostnameCache = HostnameCache.getInstance();
        }
      }
    }
  }

To this:

    private final ReentrantLock lock = new ReentrantLock();

    private void ensureHostnameCache() {
        if (hostnameCache == null) {
            lock.lock();
            try {
                if (hostnameCache == null) {
                    hostnameCache = HostnameCache.getInstance();
                }
            } finally {
                lock.unlock();
            }
        }
    }

pavelbelonosov avatar Apr 02 '24 15:04 pavelbelonosov

Hey @pavelbelonosov thanks for opening this issue. Sounds like a straight forward replacement. Maybe we can fit this into the next major which we're starting to work on - no promises though. We're currently changing a lot of things on the SDK internally, so we can't do it right away or we'd have lots of merge conflicts. ReentrantLock seems to be available for old versions of Android as well.

This class in pgjdbc seems like an interesting approach.

adinauer avatar Apr 05 '24 11:04 adinauer

Hello, for any future devs looking at hotfix, we run into similar problem (Spring Boot 3.2.2 / Java 21 / Sentry 7.14.0)

While having virtual threads on, the application would just randomly "stuck", not respond to API calls and also not responding to k8s /actuator/health readyness check, which led me down a rabbit hole of finding what could be wrong.

This behavior was happening completely random, under 0 load on multiple test environents.

Only @Scheduled tasks were running. We use @Scheduled and @SentryTransaction together.

I tracked this to mentioned function ensureHostnameCache(). As soon as I turned attaching server name (and thus calling this function) the problem went away.

The temporary solution for us is to set in application.yaml sentry.attach-server-name: false.

I would thus like to ask very kindly, if it would be possible to merge solution, that would not pin virtual threads. Thank you very much!

tkrason avatar Aug 20 '24 19:08 tkrason

Thanks @tkrason we'll bump prio on this and try to inclue it in the 8.0 release.

adinauer avatar Aug 23 '24 07:08 adinauer