search

m13253 / dns-over-https

High performance DNS over HTTPS client & server
https://developers.google.com/speed/public-dns/docs/dns-over-https
MIT License
1.99k stars 221 forks source link

Add backend weight round robin select #34

Closed Sherlock-Holo closed 5 years ago

Sherlock-Holo commented 5 years ago

add

reason

dns-over-https allow us to set multi backends. However, sometimes some backends will be unavailable, we need a algorithm to reduce probability of thems. WRR algorithm is a good way.

m13253 commented 5 years ago

Thank you for the major contribution!

I guess it provides a solution for issue #24. I will test it when I reach home. If it builds on all platforms and worked well, I will merge it.

Sherlock-Holo commented 5 years ago

Actually I don't add any new dependenices, so it should work well for all paltforms as in the past

Sherlock-Holo commented 5 years ago

I thinks we should let user know how to set upstream weight legitimately, and we may need to talk about:

  1. if upstream timeout, how much we should reduce the weight
  2. if upstream returns 5xx error, how much we should reduce the weight

1 and 2 are different, if upstream timeout, usually upstream will be unavailable for a long time, but if it returns 5xx, maybe it just a lot of users query it, it may return to normal quickly

m13253 commented 5 years ago

We could set the penalty increase exponentially (like in power if around 1.6), but switch to linear after some number of fails, and cap it with a maximum value.

And we always make sure we have at least one server to query even if all servers seem unavailable.

Sherlock-Holo commented 5 years ago

yes and we may can learn from some load b balance stuck as traefik

m13253 commented 5 years ago

Please make some very small changes before I merge :-)

Sherlock-Holo commented 5 years ago

The doh-client.conf actually is a toml config file, could we change the name suffix to toml? some text editor may only check name suffix, they won't highlight the content because they don't know what it is, such as Goland :-(

Sherlock-Holo commented 5 years ago

I think I should rewrite the Selector interface because it doesn't provide a way to reduce weight in real time. I think reducing weight in real time is necessary.

m13253 commented 5 years ago

The doh-client.conf actually is a toml config file, could we change the name suffix to toml? some text editor may only check name suffix, they won't highlight the content because they don't know what it is, such as Goland :-(

Actually I didn't intend to use a complex format like TOML to represent our configuration files. What I expect is something like INI or systemd's configuration files. They are just lucky to be compatible with TOML.

Therefore the configuration is actually a simple format (until this pull request) which actually doesn't need any syntax highlighting.

Additionally systemd's configuration files also uses different suffixes (.conf, .service, .target, .socket, etc). I think Goland is to blame for not recognizing this.

Finally, if we break the compatibility of the configuration files, we need to bump the major version number.

m13253 commented 5 years ago

I think I should rewrite the Selector interface because it doesn't provide a way to reduce weight in real time. I think reducing weight in real time is necessary.

I would really appreciate for your contributions! I am sorry that I am too busy these months.

Sherlock-Holo commented 5 years ago

This PR add Selector interface and wrr selector implement, so user should write every upsteam's weight, use TOML syntax will reudce compatible break. So we have to bump the major version number ^_^

Sherlock-Holo commented 5 years ago

I rewrite Selector interface and do a simple test in my PC, the wrr selector works well

m13253 commented 5 years ago

I tested on my PC.

Thank you for the new deadline detection code.

But one more question: should we make Client.Selector private or public? I guess we don't need to share it with other packages?

Also, I hope all weight= 50 can be replaced by weight = 50, Url can be replaced by URL.

Sherlock-Holo commented 5 years ago

it should be private, I will change it tomorrow

Star Brilliant notifications@github.com 于 2019年3月7日周四 23:59写道：

I tested on my PC.

Thank you for the new deadline detection code.

But one more question: should we make Client.Selector private or public? I guess we don't need to share it with other packages?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/m13253/dns-over-https/pull/34#issuecomment-470583472, or mute the thread https://github.com/notifications/unsubscribe-auth/AJoPKRlR_UNqLIvqtRgtp0GJOvXQTx6Xks5vUTdsgaJpZM4bgExh .

Sherlock-Holo commented 5 years ago

Change it pivate now

m13253 commented 5 years ago

Change it pivate now

Thank you for the change!

Shall I make the last 2 changes for you, or you will do the changes before I merge?

m13253 commented 5 years ago

Also, I hope all weight= 50 can be replaced by weight = 50, Url can be replaced by URL.

Sherlock-Holo commented 5 years ago
Sherlock-Holo commented 5 years ago

I upgrade Selector, now can report upstream works well. I add it because when I run it on our school DNS server, after reading logs, it seems wrr selector "drop" some upstreams because wrr selector reduce their weight too fast but icrease weight too slow when they are available, so we should increase weight when upstreams are available in real time