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

[Feature Request] Blocky should start resolving DNS traffic using available upstream ASAP #835

Closed
PeterDaveHello opened this issue Jan 20, 2023 · 10 comments · Fixed by #1283
Labels
🔨 enhancement New feature or request
Milestone

Comments

@PeterDaveHello
Copy link
Contributor

To prevent misunderstanding, this issue is not a duplicate of #566. #566 is addressing the blocking caused by the blocklist/allowlist, I'd like to discuss about the upstream resolver here.

When there are many upstream resolvers in the config, like this: https://github.com/PeterDaveHello/dnslow.me/blob/master/blocky-dns-proxy-config.yml, looks like blocky will take a while to start(almost a minute in my case), if there wasn't any high-availability architecture in the service, the restart usually will cause a down time.

Maybe just like #566/#636, we can allow blocky start to resolve queries once there's any working upstream?

Thanks!

@0xERR0R
Copy link
Owner

0xERR0R commented Jan 24, 2023

Which version of blocky do you use currently? Maybe #749 is relevant for you.

@PeterDaveHello
Copy link
Contributor Author

@0xERR0R v0.20 the latest stable release. Will take a look at #749, thanks.

@PeterDaveHello
Copy link
Contributor Author

@0xERR0R looks like there's still a little bit different, but maybe not important at all.

According to the document, here's the description of startVerifyUpstream

If true, blocky will fail to start unless at least one upstream server per group is reachable.

So looks like current policy is to check all the upstreams once to make sure at least one of them works, or check none of them, and then start to serve. What I'm thing about is still to check them all, but once any of upstreams is working, then start to serve, not to wait for the check completed.

@ThinkChaos
Copy link
Collaborator

If startVerifyUpstream is true, blocky exits directly if all of the upstreams fail.
The easiest way get the behavior you want, is to rely on whatever manages your blocky server (docker, systemd, etc.) to restart it if it fails. That way it will retry the startup checks until at least one upstream works.

To implement this behavior in blocky, we could change startVerifyUpstream to accept something like retry where it retries to startup regularly. But I'm not convinced that's a good fit for blocky.
Maybe just having a specific exit code for "startup checks failed" would be enough so that the process manager can be more intelligent about when to restart blocky.

@PeterDaveHello
Copy link
Contributor Author

Just leave a record that it takes about minutes starting to resolve queries if there are many upstream 😅

@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label May 17, 2023
@ThinkChaos
Copy link
Collaborator

Making Blocky test the resolvers in parallel could be a good improvement here.
We could also add a startStrategy option where fast would start immediately and use the bootstrap resolver (which defaults to the system resolver), and blocking would be the current behavior.

Both of those should not be too hard after #998 is merged.

@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Aug 18, 2023
@PeterDaveHello
Copy link
Contributor Author

Oh, please keep it.

@0xERR0R 0xERR0R added this to the future milestone Aug 18, 2023
@0xERR0R 0xERR0R added 🔨 enhancement New feature or request and removed Stale labels Aug 18, 2023
@ThinkChaos
Copy link
Collaborator

Finally got around to finishing my PR for this :)
See #1283

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants