Implement random resolver option #467
Conversation
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Other comments (1)
- proxy/upstreammode.go (27-27) The comment for `UpstreamModeRandom` is missing a verb. Consider changing to "makes server to use a random upstream" or "makes server to select a random upstream".
💡 To request another review, post a new comment with "/windsurf-review".
proxy/config.go
Outdated
| UpstreamModeLoadBalance, | ||
| UpstreamModeParallel: | ||
| UpstreamModeParallel, | ||
| UpstreamModeRandom: |
There was a problem hiding this comment.
The PR adds UpstreamModeRandom to the switch statement, but this constant isn't defined anywhere in the visible code. You need to define this constant alongside the other UpstreamMode constants (like UpstreamModeFastestAddr, UpstreamModeLoadBalance, etc.).
There was a problem hiding this comment.
This one I believe is fine as it is inherited from AdGuardHome?
Although I realised the switch case is wrong, fixed it up
UpstreamModeFastestAddr,
UpstreamModeLoadBalance,
UpstreamModeRandom,
UpstreamModeParallel:
// Go on.
proxy/upstreammode.go
Outdated
| case | ||
| UpstreamModeLoadBalance, | ||
| UpstreamModeParallel, | ||
| UpstreamModeRandomm, |
There was a problem hiding this comment.
There's a typo in the variable name: UpstreamModeRandomm should be UpstreamModeRandom.
Co-authored-by: windsurf-bot[bot] <189301087+windsurf-bot[bot]@users.noreply.github.com>
|
Thanks for the contribution! We'll review it shortly. |
| }, | ||
| upstreamModeIdx: { | ||
| description: "Defines the upstreams logic mode, possible values: load_balance, parallel, " + | ||
| description: "Defines the upstreams logic mode, possible values: load_balance, parallel, random, " + |
There was a problem hiding this comment.
Please update README.md as well.
| // Simply set ups to a random entry in ups | ||
| // Falls back to default behaviour as if ups <= 1 it will run UpstreamModeLoadBalance as intended | ||
| if len(ups) > 1 { | ||
| ups = []upstream.Upstream{ups[p.randSrc.Intn(len(ups))]} |
There was a problem hiding this comment.
This doesn't build:
proxy/exchange.go:28:44: p.randSrc.Intn undefined (type "math/rand/v2".Source has no field or method Intn)
See go doc math/rand/v2.
| // response time or TCP connection time. | ||
| UpstreamModeFastestAddr UpstreamMode = "fastest_addr" | ||
|
|
||
| // UpstreamModeRandom makes server to a random upstream |
There was a problem hiding this comment.
Please, consider using gofumpt to format the code in this repository:
-
make go-tools
-
./bin/gofumpt --extra -w .
3 participants
Implements the option for a random resolver to go alongside this PR AdguardTeam/AdGuardHome#7950
Hopefully resolves AdguardTeam/AdGuardHome#4756