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

Unit-test thread cleanup #4764

Merged

Conversation

devinrsmith
Copy link
Member

@devinrsmith devinrsmith commented Nov 2, 2023

During testing of #4756, I discovered that io.deephaven.server.util.Scheduler and io.deephaven.util.reference.CleanupReferenceProcessor threads were leaking. CI testing was throwing "can't create more threads" errors due to the addition of a large number of new java client async tests which pushed the leaked thread counts over some JVM limit.

As concrete evidence of the leaks, I ran

./gradlew java-client-session-dagger:test -PforceTest=true --max-workers 1
jps
jstack <pid-for-GradleWorkerMain> | grep os_prio | awk -F " " '{print $1}' | sort | uniq -c | sort -nr

and got

    432 "DeephavenApiServer-Scheduler-Concurrent-4"
    432 "DeephavenApiServer-Scheduler-Concurrent-3"
    432 "DeephavenApiServer-Scheduler-Concurrent-2"
    432 "DeephavenApiServer-Scheduler-Concurrent-1"
     43 "CleanupReferenceProcessor-liveness-drainingThread"
     18 "GC
     10 "G1
      4 "C1
      3 "C2
      3 "/127.0.0.1:42968
      2 "VM
      1 "Test
      1 "Sweeper
      1 "StrDedup"
      1 "Signal
      1 "Service
      1 "Reference
      1 "process
      1 "PeriodicUpdateGraph-updateExecutor-1"
      1 "grpc-timer-0"
      1 "grpc-shared-destroyer-0"
      1 "grpc-default-executor-6"
      1 "grpc-default-executor-5"
      1 "grpc-default-executor-4"
      1 "grpc-default-executor-3"
      1 "grpc-default-executor-2"
      1 "grpc-default-executor-1"
      1 "grpc-default-executor-0"
      1 "Finalizer"
      1 "Common-Cleaner"
      1 "Attach

This PR adds an unit-test-only shutdown procedure for the Scheduler; if we wanted to actually change the Scheduler interface to support this, it would require more work and testing. This also ensures that CleanupReferenceProcessor are promptly cleaned up (instead of eventually).

@devinrsmith devinrsmith requested a review from rcaudy November 3, 2023 17:18
@devinrsmith devinrsmith merged commit 27b6061 into deephaven:main Nov 3, 2023
@devinrsmith devinrsmith deleted the nightly/unit-test-thread-cleanup branch November 3, 2023 22:11
@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants