wdaike / ngx_upstream_jdomain

An asynchronous domain name resolve module for nginx upstream
78 stars 32 forks source link

Allow nginx to start even if resolution failed #12

Open rs opened 8 years ago

rs commented 8 years ago

If nginx can't access the resolver during startup for any reason, do not prevent nginx from starting and let it recover later.

Also fixes a crash in case the resolution would suddently return no entities.

(This PR is based on #11)

rs commented 8 years ago

Are you still maintaining this module?

viciious commented 8 years ago

@rs I think I have what might be a better solution to the problem.

    if (ngx_parse_url(cf->pool, &u) != NGX_OK) {
        if (u.err) {
            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                "%s in upstream \"%V\"", u.err, &u.url);
        }

        return NGX_CONF_ERROR;
    }

I think this approach is a bit better/cleaner than both the original and your versions:

    // first pass: parse and validate config option and drop out if failed
    // second pass: actually perform name resolution, continue in case of failure
    for (i=0; i<2; i++) {
        u.no_resolve = (i == 0);
        if (ngx_parse_url(cf->pool, &u) != NGX_OK) {
            if (u.err) {
                ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                    "%s in upstream \"%V\"", u.err, &u.url);
            }
            if (u.no_resolve)
                return NGX_CONF_ERROR;
        }
    }
wdaike commented 8 years ago

if do not prevent nginx from starting , all request to upstream will fail.i don't think its a good idea.

viciious commented 8 years ago

@wdaike imo there's no practical difference whether requests fail at startup or at some later stage in case the DNS server becomes temporarily unavailable. On the other hand, a service that fails to startup due to a temporary error can cause a lot of maintenance trouble.

@rs Please see https://github.com/viciious/ngx_upstream_jdomain/tree/lazy-startup

viciious commented 8 years ago

@wdaike I can see how this might be a bad idea in case of a restart though. Perhaps we should add a config option to allow tolerating name resolution failures at startup then.

viciious commented 8 years ago

Added a "no_fail" option which defaults to 0. Turning the option on allows nginx to startup even in case of domain name resolution failure.

https://github.com/viciious/ngx_upstream_jdomain/commits/lazy-startup

@wdaike If that's ok with you, i'll make a new pull request which will supersede this one.