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

Support HealtchCheck on tcp layer,avoid to produce needless http access log #46

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

shi7161979
Copy link

local ok, err = hc.spawn_checker{
shm = "healthcheck", -- defined by "lua_shared_dict"
upstream = "foo.com", -- defined by "upstream"
type = "tcp",
interval = 2000, -- run the check cycle every 2 sec
timeout = 1000, -- 1 sec is the timeout for network operations
fall = 3, -- # of successive failures before turning a peer down
rise = 2, -- # of successive successes before turning a peer up
concurrency = 10, -- concurrency level for test requests
}

@@ -524,12 +530,12 @@ function _M.spawn_checker(opts)
return nil, "\"type\" option required"
end

if typ ~= "http" then
if typ ~= "http" and typ ~= "tcp" then
Copy link
Contributor

Choose a reason for hiding this comment

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

ran into this while browsing: updating this check also requires updating the error message below

@shi7161979
Copy link
Author

update error message ,thank you for your suggestion, Tieske。

@Colstuwjx
Copy link

why we don't merge this? @Tieske

@Tieske
Copy link
Contributor

Tieske commented Jun 11, 2019

@Colstuwjx I commented on it, but I'm not a maintainer of this lib

@timebug
Copy link

timebug commented Sep 27, 2020

+1 ping @thibaultcha @spacewander

@emirmo
Copy link

emirmo commented Jun 8, 2024

Is there a reason this PR was never merged?

Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

@@ -228,6 +228,12 @@ local function check_peer(ctx, id, peer, is_backup)
errlog("failed to connect to ", name, ": ", err)
end
return peer_fail(ctx, is_backup, id, peer)
else
if ctx.type == "tcp" then
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend to move the if block a level out, so the close & return inside it is more explicitly. Like if not ok ... end; if ctx.type == ....

@spacewander
Copy link
Member

Is there a reason this PR was never merged?

Maybe I just missed it in a lot of notifications... Feel free to ping me again!

@mmkhmmkh
Copy link

Could anyone merge this MR? @spacewander @shi7161979

@spacewander
Copy link
Member

Could anyone merge this MR? @spacewander @shi7161979

The author doesn't adjust the comment in #46 (review). We need test for new feature.

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.

7 participants