Skip to content
This repository was archived by the owner on Jan 19, 2022. It is now read-only.

Merge to 1.1.x: Make Pub/Sub thread pools Spring-managed beans (#1505) #1518

Merged
merged 1 commit into from
Mar 11, 2019

Conversation

elefeint
Copy link
Contributor

@elefeint elefeint commented Mar 11, 2019

The two thread pools are hardcoded in autoconfiguration, so they fall through the cracks when a Spring application shuts down:

  • Spring does not manage them because they are not context-managed beans.
  • gRPC stub does not manage them because FixedExecutorProvider.shouldAutoClose() is false (also, we don't call SubscriberStub.shutdown(), which we arguably should do, as well.

This does not manifest on the publisher side because that thread pool is never started, but the subscriber thread pool is started by way of subscriber stub creation when PubSubSubscriberTemplate is instantiated.

Cherrypicked from #1505.
Fixes #1503.
Addresses the Pub/Sub part of #1506.

* Make thread pools Spring-managed
* Use ThreadPoolTaskScheduler with named threads.
@elefeint elefeint changed the title Make Pub/Sub thread pools Spring-managed beans (#1505) Merge to 1.1.x: Make Pub/Sub thread pools Spring-managed beans (#1505) Mar 11, 2019
@elefeint elefeint requested a review from meltsufin March 11, 2019 18:57
@codecov-io
Copy link

codecov-io commented Mar 11, 2019

Codecov Report

Merging #1518 into 1.1.x will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##              1.1.x    #1518      +/-   ##
============================================
+ Coverage     68.29%   68.33%   +0.03%     
- Complexity     1429     1431       +2     
============================================
  Files           203      203              
  Lines          5728     5735       +7     
  Branches        578      578              
============================================
+ Hits           3912     3919       +7     
  Misses         1579     1579              
  Partials        237      237
Flag Coverage Δ Complexity Δ
#integration ? ?
#unittests 68.33% <100%> (+0.03%) 1431 <4> (+2) ⬆️
Impacted Files Coverage Δ Complexity Δ
...toconfigure/pubsub/GcpPubSubAutoConfiguration.java 90.47% <100%> (+0.56%) 38 <4> (+2) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b48e8d0...0bdabdc. Read the comment docs.

@elefeint elefeint merged commit 81c9593 into 1.1.x Mar 11, 2019
@elefeint elefeint deleted the merge-pubsub-hanging-threadpool-1.1 branch March 11, 2019 19:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants