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

fix: fix consul plugin worker will not exit while reload or quit #10342

Merged

Conversation

theweakgod
Copy link
Contributor

@theweakgod theweakgod commented Oct 16, 2023

Description

1.configure

config.yaml

discovery:
consul:
servers:
- "http://localhost:8500"
keepalive: true

Version: 3.6.0

2.situation
Using the consul service, I found that there is a process leak. When reloading or quitting, there will be a nginx process shutting down and never exit.
When nginx reloads, the consul worker with keepalive turned on will not exit normally, will be in shutting down state, and will always access consul's health api.
You only need to make a judgment before creating a new timer to solve the process leak.
This problem does not exist when keepalive is false, but there will be an obvious memory leak.

htop -p $(pgrep -d',' -f "nginx")
image
process will not exit!

To process exit when testing, please change consul node state.

3.testing
start consul discovery, and try to apisix reload && htop -p $(pgrep -d',' -f "nginx"). The process in shutting down, and nerver exit even if you set timeout.
after pull this commit, set shutting down timeout 300 seconds or change consul node info, the shutting down process will exit after node info changing or timeout.

Fixes # (issue)

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@theweakgod
Copy link
Contributor Author

theweakgod commented Oct 16, 2023

During testing, if nginx worker is still shutting down after reload, you can update the consul node so that the worker will exit, because the worker is in the http request state at this time and will exit after the request is completed.😬

@theweakgod theweakgod changed the title fix: fix consul worker will not exit while reload or quit fix: fix consul plugin worker will not exit while reload or quit Oct 17, 2023
@shreemaan-abhishek
Copy link
Contributor

I checked this, the worker process still does not exit. Do you have a clue on how we can make the worker process exit automatically instead of worker reset?

@theweakgod
Copy link
Contributor Author

theweakgod commented Oct 18, 2023

I checked this, the worker process still does not exit. Do you have a clue on how we can make the worker process exit automatically instead of worker reset?

You can update consul node(eg. register a server). Because worker is blocked by http request for monitoring consul node update. Then worker will exit.
Or waiting a few minutes. worker will exit also.
reference

@theweakgod
Copy link
Contributor Author

During testing, if nginx is still shutting down after reload, you can update the consul node so that the worker will exit, because the worker is in the http request state at this time and will exit after the request is completed.😬

Check this. @shreemaan-abhishek

@monkeyDluffy6017 monkeyDluffy6017 merged commit 3b16fce into apache:master Oct 20, 2023
31 checks passed
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request Nov 1, 2023
* upstream/master: (83 commits)
  fix: make install failed on mac (apache#10403)
  feat(zipkin): add variable (apache#10361)
  test(clickhouse-logger): to show that different endpoints will be chosen randomly (apache#8777)
  chore(deps): bump actions/setup-node from 3.8.1 to 4.0.0 (apache#10381)
  ci: fix the grpc test error (apache#10388)
  ci: trigger ci when doc-lint.yml changes (apache#10382)
  docs: fix usage of incorrect default admin api port (apache#10391)
  feat: Add authorization params to openid-connect plugin (apache#10058)
  feat: integrate authz-keycloak with secrets resource (apache#10353)
  fix(traffic-split): post_arg match fails because content-type contains charset (apache#10372)
  fix(consul): worker will not exit while reload or quit (apache#10342)
  chore: update rules for unresponded issues (apache#10354)
  docs: Update APISIX usecases in README (apache#10358)
  test: use http2 to test limit-req plugin (apache#10334)
  test: use http2 to test limit-conn plugin (apache#10332)
  chore: remove stream_proxy.only in config-default.yaml (apache#10337)
  docs: update underscore to hyphen in HTTP headers in `response-rewrite` plugin (apache#10347)
  fix: typos in comments (apache#10330)
  feat: support config stream_route upstream in service (apache#10298)
  fix: keep healthcheck target state when upstream changes (apache#10312)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants