openresty / lua-resty-upstream-healthcheck

Health Checker for Nginx Upstream Servers in Pure Lua
515 stars 134 forks source link

no_timer option can work in non debug mode #68

Closed wangrzneu closed 3 years ago

wangrzneu commented 4 years ago

remove debug_mode limit of no_timer option

wangrzneu commented 4 years ago

PTAL @thibaultcha

wangrzneu commented 4 years ago

/rebuild

rainingmaster commented 4 years ago

maybe you can add some test case, so we can know the effect?

doujiang24 commented 4 years ago

@wangrzneu will you take care of the Travis failure thing? https://travis-ci.org/github/openresty/lua-resty-upstream-healthcheck/jobs/670113565?utm_medium=notification&utm_source=github_status

wangrzneu commented 4 years ago

@wangrzneu will you take care of the Travis failure thing? https://travis-ci.org/github/openresty/lua-resty-upstream-healthcheck/jobs/670113565?utm_medium=notification&utm_source=github_status

Yes, I add a new case and I will try to make it pass.

wangrzneu commented 4 years ago

maybe you can add some test case, so we can know the effect?

Done. I add TEST 16.

wangrzneu commented 4 years ago

@wangrzneu will you take care of the Travis failure thing? https://travis-ci.org/github/openresty/lua-resty-upstream-healthcheck/jobs/670113565?utm_medium=notification&utm_source=github_status

I have fixed it. But something wrong happens in travis CI.

wangrzneu commented 4 years ago

@wangrzneu will you take care of the Travis failure thing? https://travis-ci.org/github/openresty/lua-resty-upstream-healthcheck/jobs/670113565?utm_medium=notification&utm_source=github_status

I have fixed it. But something wrong happens in travis CI.

I fix CI by updating OPENSSL_VER.

doujiang24 commented 4 years ago

@wangrzneu can you explain why you need this change?

rainingmaster commented 4 years ago

I think the no_timer should only run in the debug mode, otherwise the health check will run one time only, and it can not check the issue of upsteams, right?

wangrzneu commented 4 years ago

@wangrzneu can you explain why you need this change?

This change will help me change the health check options without reloading the openresty process.

And the health check can be triggered using a timer outside the openresty process.

wangrzneu commented 4 years ago

I think the no_timer should only run in the debug mode, otherwise the health check will run one time only, and it can not check the issue of upsteams, right?

The health check can be triggered using a timer outside the openresty process.

doujiang24 commented 4 years ago

And the health check can be triggered using a timer outside the openresty process.

@wangrzneu can you give a sample that how you use it? I don't think I got you.

wangrzneu commented 4 years ago

And the health check can be triggered using a timer outside the openresty process.

@wangrzneu can you give a sample that how you use it? I don't think I got you.

My nginx.conf is like:

worker_processes  1;
error_log logs/error.log warn;

events {
    worker_connections 1024;
}

http {
    upstream foo.com {
        server 127.0.0.1:12345;
    }

    server {
        listen 12345;

        location = /status {
            content_by_lua_block {
                ngx.say("ok")
            }
        }
        location / {
            content_by_lua_block {
                ngx.say("foo.com")
            }
        }
    }
    lua_shared_dict healthcheck 1m;
    server {
        listen 8080;
        location / {
            proxy_pass http://foo.com;
        }
    }

    server {
        listen 9090;
        location /healthcheck {
            content_by_lua_block {
                local hc = require "resty.upstream.healthcheck"
                local args = ngx.req.get_uri_args()
                local ok, err = hc.spawn_checker{
                    shm = "healthcheck",
                    upstream = args["upstream"],
                    type = args["type"],
                    http_req = args["probe"] .. " HTTP/1.0\r\nHost: localhost\r\n\r\n",
                    timeout = tonumber(args["timeout"]),
                    fall = tonumber(args["fail"]),
                    no_timer = true,
                }
                if not ok then
                    ngx.log(ngx.ERR, "failed to spawn health checker: ", err)
                    return
                end
                ngx.say(hc.status_page()) 
            }
        }
    }
}

I use another program (implemented by golang) to request the /healthcheck periodically, and the arguments can be changed without restart openresty worker process.

The healthcheck request is like http://127.0.0.1:9090/healthcheck?upstream=foo.com&probe=GET%20/status&timeout=100&fail=1&type=http.

doujiang24 commented 4 years ago

@wangrzneu I think I got you now.

my opinion, it should be ok to merge it into master though it's not the recommended way to use this library. this PR is a small change and it really helps @wangrzneu, so I vote for it. @thibaultcha @spacewander @rainingmaster thoughts?

spacewander commented 4 years ago

no_timer is confusing for produce, maybe we can rename it as once?

rainingmaster commented 4 years ago

@doujiang24 I agree with you, the change is small and no impact on existing functions and parameters. And I think if we not recommend use this lib like this way, maybe the test cases should be more simple and check no_timer(or once is better) only? Because test case like here maybe make other confuse.

rainingmaster commented 4 years ago

@wangrzneu I think maybe you should test the case when has multiple upstream, because upstream_checker_statuses is init in content_by_lua_block, so maybe some worker do not have the specific key in upstream_checker_statuses.

In first request, check with foo.com, hit 0 worker:

Upstream foo.com
    Primary Peers
        127.0.0.1:12354 up
    Backup Peers

Upstream boo.com (NO checkers)
    Primary Peers
        127.0.0.1:12354 up
    Backup Peers

In second request, check with boo.com, hit 1 worker:

Upstream foo.com (NO checkers)
    Primary Peers
        127.0.0.1:12354 up
    Backup Peers

Upstream boo.com
    Primary Peers
        127.0.0.1:12354 up
    Backup Peers

In addition, welcome to the official form https://forum.openresty.us/ to share and discuss