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: the leak of prometheus metrics #10655

Merged
merged 8 commits into from
Dec 25, 2023
Merged

fix: the leak of prometheus metrics #10655

merged 8 commits into from
Dec 25, 2023

Conversation

Sn0rt
Copy link
Contributor

@Sn0rt Sn0rt commented Dec 15, 2023

Description

fix: #10636

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)

@Sn0rt Sn0rt marked this pull request as draft December 15, 2023 10:44
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
@Sn0rt Sn0rt marked this pull request as ready for review December 21, 2023 09:53
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
@Sn0rt Sn0rt changed the title fix: healthchek target leak fix: metrics.upstream_status node leak Dec 22, 2023
AlinsRan
AlinsRan previously approved these changes Dec 22, 2023
@monkeyDluffy6017 monkeyDluffy6017 changed the title fix: metrics.upstream_status node leak fix: the leak of prometheus metrics Dec 22, 2023
ngx.say(body)
return
end
ngx.sleep(6)
Copy link
Contributor

Choose a reason for hiding this comment

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

It takes too long, try to replace delayed_clear with clear when run test cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This time is set after many attempts. 3, 4, and 5 have a small probability of causing the target to not be cleaned up in time enough.

I don't agree with using clear in test cases, it will make the test cases complicated and difficult to read.

  1. The checker cannot be obtained directly in the test case.
  2. The underlying health check library does not support directly calling functions to delete targets.

Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
obj.delayed_clear = obj.clear
return obj
end
--- timeout: 20
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

local httpc = http.new()
local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ping"
local _, _ = httpc:request_uri(uri, {method = "GET", keepalive = false})
ngx.sleep(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
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.

bug: health checker target leak
3 participants