matomo-org / matomo-java-tracker

Official Java implementation of the Matomo Tracking HTTP API.
https://matomo-org.github.io/matomo-java-tracker/
BSD 3-Clause "New" or "Revised" License
69 stars 52 forks source link

[BUG] unable to create native thread #215

Closed bbpennel closed 8 months ago

bbpennel commented 8 months ago

Describe the bug We recently updated to version 3 of the matomo-java-tracker library and have started having issues with resources being exhausted. We are sending a server side event to matomo when one of our API endpoints is accessed. Initially we were using the jre8 jar before we realized there was a separate jre11 version, but we are still experiencing the issue after switching. It seems to cause our server to run out of resources and require a restart periodically. The error looks like:

java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
        at java.base/java.lang.Thread.start0(Native Method)
        at java.base/java.lang.Thread.start(Thread.java:798)
        at java.net.http/jdk.internal.net.http.HttpClientImpl.start(HttpClientImpl.java:320)
        at java.net.http/jdk.internal.net.http.HttpClientImpl.create(HttpClientImpl.java:254)
        at java.net.http/jdk.internal.net.http.HttpClientBuilderImpl.build(HttpClientBuilderImpl.java:135)
        at org.matomo.java.tracking.Java11SenderProvider.provideSender(Java11SenderProvider.java:57)
        at org.matomo.java.tracking.ServiceLoaderSenderFactory.createSender(ServiceLoaderSenderFactory.java:25)
        at org.matomo.java.tracking.MatomoTracker.initializeSender(MatomoTracker.java:155)
        at org.matomo.java.tracking.MatomoTracker.sendRequestAsync(MatomoTracker.java:197)
        at org.matomo.java.tracking.MatomoTracker.sendRequestAsync(MatomoTracker.java:172)
        at edu.unc.lib.boxc.web.common.utils.AnalyticsTrackerUtil.sendMatomoRequest(AnalyticsTrackerUtil.java:89)
        at edu.unc.lib.boxc.web.common.utils.AnalyticsTrackerUtil.trackEvent(AnalyticsTrackerUtil.java:60)
        at edu.unc.lib.boxc.web.access.controllers.FedoraContentController.recordDownloadEvent(FedoraContentController.java:127)
        at edu.unc.lib.boxc.web.access.controllers.FedoraContentController.streamData(FedoraContentController.java:101)
        at edu.unc.lib.boxc.web.access.controllers.FedoraContentController.getDefaultDatastream(FedoraContentController.java:64)

Code snippets Our code that triggers the events is found here: https://github.com/UNC-Libraries/box-c/blob/v5.31.2/web-common/src/main/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtil.java#L67-L93 Which is very similar to our usage prior to matomo-java-tracker v3: https://github.com/UNC-Libraries/box-c/pull/1655/files

Expected behavior Sending events should not cause our application to stop working.

Additional context It seems like the resource exhaustion started happening shortly after we made adjustments to how the visitorId was being set since we were getting many of the following errors:

java.lang.NullPointerException: inputHex is marked non-null but is null
    at org.matomo.java.tracking.parameters.VisitorId.fromHex(VisitorId.java:87)
    at edu.unc.lib.boxc.web.common.utils.AnalyticsTrackerUtil.buildMatomoRequest(AnalyticsTrackerUtil.java:70)

Which was being triggered by bot traffic when "Proxy-Client-IP: 0.0.0.0" was set, which caused us to pass a null visitorId to matomo. We changed it to passing in a randomly generated visitorId, which resolved the inputHex is marked non-null but is null error, but no resources are being exhausted instead, possibly because many more requests are being sent to matomo.

I'm trying to replicate the issue on our testing servers but haven't been able to do so yet, but we've had it happen in production twice in a week.

We have not changed the default threadPoolSize or any of the timeouts, but it seems like increasing the number might make it exhaust all threads sooner?

dheid commented 8 months ago

@bbpennel Thanks for reporting! I will look into it soon.

bbpennel commented 8 months ago

I am wondering if it could be related to a combination of the fact we are creating new MatomoTrackers for every request, and this block where it looks like a new FixedThreadPool gets initialized for each tracker but doesn't appear to get shut down? https://github.com/matomo-org/matomo-java-tracker/blob/main/java11/src/main/java/org/matomo/java/tracking/Java11SenderProvider.java#L25

It seems like we should probably reuse a single instance of MatomoTracker

bbpennel commented 8 months ago

I was able to replicate the issue on our test server after about 16k requests. I switched over to initializing a single MatomoTracker and haven't seen the issue recur after about 64k requests. So our usage was the main issue, but it might still be a good idea to add a method for shutting down the executor for cases where clients use multiple MatomoTrackers.

dheid commented 8 months ago

I am wondering if it could be related to a combination of the fact we are creating new MatomoTrackers for every request, and this block where it looks like a new FixedThreadPool gets initialized for each tracker but doesn't appear to get shut down? https://github.com/matomo-org/matomo-java-tracker/blob/main/java11/src/main/java/org/matomo/java/tracking/Java11SenderProvider.java#L25

It seems like we should probably reuse a single instance of MatomoTracker

Yeah, I think that's the reason for it. MatomoTracker should be reused.

dheid commented 8 months ago

I was able to replicate the issue on our test server after about 16k requests. I switched over to initializing a single MatomoTracker and haven't seen the issue recur after about 64k requests. So our usage was the main issue, but it might still be a good idea to add a method for shutting down the executor for cases where clients use multiple MatomoTrackers.

Thanks so much! I will look how I can improve that.

dheid commented 8 months ago

@bbpennel The MatomoTracker class now implements AutoClosable to ensure, that the users close it after usage. This is blocking and will shutdown the threads used by async requests and frees the memory. Thanks for making Matomo Java Tracker better!