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.

Sign up for GitHub

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

Jump to bottom

[feature] When the nacos-server fails to start, shutdown all http clients #3832

Closed
Maijh97 opened this issue Sep 15, 2020 · 4 comments · Fixed by #4150
Closed

[feature] When the nacos-server fails to start, shutdown all http clients #3832

Maijh97 opened this issue Sep 15, 2020 · 4 comments · Fixed by #4150
Labels
kind/enhancement Category issues or prs related to enhancement.
Milestone
1.4.1

Comments

@Maijh97
Copy link
Collaborator

Maijh97 commented Sep 15, 2020 •
edited
Loading

Is your feature request related to a problem? Please describe.
When nacos-server fails to start, the http client should be shut down to release related resources.

Describe the solution you'd like

  1. Add the shutdown all http client method in HttpClientBeanHolder.

  2. Call HttpClientBeanHolder#shutdown in the StartingSpringApplicationRunListener#failed method to shut down all http clients.

Describe alternatives you've considered

Additional context
@KomachiSion KomachiSion added the kind/enhancement Category issues or prs related to enhancement. label Sep 16, 2020
@Maijh97
Copy link
Collaborator Author

Maijh97 commented Nov 3, 2020 •
edited
Loading

Considering that when the server fails to start, it usually stops the current service and restarts it. Currently, the http client uses ThreadUtils#addShutdownHook to close, so there is no need to add the http client shutdown method in StartingSpringApplicationRunListener#failed.

@Maijh97 Maijh97 closed this as completed Nov 3, 2020
@Maijh97
Copy link
Collaborator Author

Maijh97 commented Nov 4, 2020 •
edited
Loading

Because there is a shared http client implementation, it is still necessary to supplement the shared http client shutdown.

@Maijh97 Maijh97 reopened this Nov 4, 2020
This was referenced Nov 4, 2020
Merged
Merged
KomachiSion pushed a commit that referenced this issue Nov 6, 2020
@Maijh97
for #3832, add common http client shutdown method. (#4150)
51fa1c0
@KomachiSion
Copy link
Collaborator

Has this issue completed? If so, close it.

@KomachiSion KomachiSion added this to the 1.4.1 milestone Nov 6, 2020
@Maijh97 Maijh97 linked a pull request Nov 6, 2020 that will close this issue
[ISSUE#3832] add common http client shutdown method #4150
Merged
5 tasks
@Maijh97
Copy link
Collaborator Author

Maijh97 commented Nov 6, 2020

Has this issue completed? If so, close it.

Resolved

@Maijh97 Maijh97 closed this as completed Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Milestone
1.4.1
Development

Successfully merging a pull request may close this issue.

[ISSUE#3832] add common http client shutdown method
2 participants
@KomachiSion @Maijh97