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

Remove custom thread pools #365

Merged
merged 2 commits into from May 11, 2021
Merged

Remove custom thread pools #365

merged 2 commits into from May 11, 2021

Conversation

elefeint
Copy link
Contributor

This offers a big improvement in memory use.

Removing custom thread pools means local health check needs a proper indicator of "closed connection", but that was always the right thing to do anyway. I removed the "double transaction manager shutdown" for which there was a TODO in the code.

I've also opportunistically switched to async version of closing transaction manager.

Fixes #258.

@google-cla google-cla bot added the cla: yes label May 11, 2021
@elefeint elefeint requested a review from a team May 11, 2021 00:13
Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the minor codesmell from Sonar. Otherwise, LGTM.

@sonarcloud
Copy link

sonarcloud bot commented May 11, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.3% 90.3% Coverage
0.0% 0.0% Duplication

@elefeint elefeint merged commit 7634191 into main May 11, 2021
@elefeint elefeint deleted the remove-threadpools branch May 11, 2021 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return publisher with shutdown status on connection close
2 participants