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.

Sign up for GitHub

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

Jump to bottom

最近添加的round-robin balancing strategy好像有点问题 #2898

Closed
yomnxkcs opened this issue Jan 5, 2024 · 9 comments
Closed

最近添加的round-robin balancing strategy好像有点问题 #2898

yomnxkcs opened this issue Jan 5, 2024 · 9 comments

Comments

@yomnxkcs
Copy link

yomnxkcs commented Jan 5, 2024

测试结果

=== RUN   TestRoundRobinBalancingStrategy
    round_robin_test.go:24: [ok] expected: a picked: a
    round_robin_test.go:24: [ok] expected: b picked: b
    round_robin_test.go:24: [ok] expected: c picked: c
    round_robin_test.go:22: [fail] expected: b picked: a
    round_robin_test.go:22: [fail] expected: c picked: b
    round_robin_test.go:22: [fail] expected: d picked: c
    round_robin_test.go:22: [fail] expected: e picked: a
--- FAIL: TestRoundRobinBalancingStrategy (0.00s)
FAIL
FAIL    github.com/xtls/xray-core/testing       0.122s
FAIL

测试代码

package balancer_test

import (
	"testing"

	"github.com/xtls/xray-core/app/router"
)

func TestRoundRobinBalancingStrategy(t *testing.T) {

	tags1 := []string{"a", "b", "c"}

	// ./xray api rmo "a"
	// ./xray api ado "d" "e"
	tags2 := []string{"b", "c", "d", "e"}

	rr := router.NewRoundRobin(tags1)
	for _, tags := range [][]string{tags1, tags2} {
		for _, exp := range tags {
			tag := rr.PickOutbound(tags)
			if tag != exp {
				t.Error("[fail]", "expected:", exp, "picked:", tag)
			} else {
				t.Log("[ok]", "expected:", exp, "picked:", tag)
			}
		}
	}
}
@hossinasaadi
Copy link
Contributor

round robin works like mentioned here #2844 (comment)

@yomnxkcs
Copy link
Author

yomnxkcs commented Jan 7, 2024

round robin works like mentioned here #2844 (comment)

Parameter of PickOutbound() is not a constant. It could be changed by api call.
e.g. ./xray api rmo "a"

@hossinasaadi
Copy link
Contributor

Parameter of PickOutbound() is not a constant. It could be changed by api call. e.g. ./xray api rmo "a"

you add balancer selector as prefix ("proxy") :

  "balancers" : [{
            "tag": "b1",
            "selector": ["proxy"],
            "strategy": {
                "type":"roundRobin"
            }
          }
        ]

so when add new outbound with api you should consider prefix as well.

func TestRoundRobinBalancingStrategy(t *testing.T) {

	tags1 := []string{"proxyA", "proxyB", "proxyC"}

	// ./xray api rmo "proxyA"
	// ./xray api ado "proxyD" "proxyE"
	tags2 := []string{"proxyB", "proxyC", "proxyD" "proxyE"}

	rr := router.NewRoundRobin(tags1)
	for _, tags := range [][]string{tags1, tags2} {
		for _, exp := range tags {
			tag := rr.PickOutbound(tags)
			if tag != exp {
				t.Error("[fail]", "expected:", exp, "picked:", tag)
			} else {
				t.Log("[ok]", "expected:", exp, "picked:", tag)
			}
		}
	}
}

@yomnxkcs
Copy link
Author

yomnxkcs commented Jan 7, 2024

Sorry about my english. Let me try again.

Suppose i start an xray client with the following configuration.

{
  "log": {
    "loglevel": "warning"
  },
  "inbounds": [
    {
      "tag": "http",
      "protocol": "http",
      "port": 8888,
      "listen": "127.0.0.1",
      "settings": {}
    },
    {
      "listen": "127.0.0.1",
      "port": 10085,
      "protocol": "dokodemo-door",
      "settings": {
        "address": "127.0.0.1"
      },
      "tag": "api"
    }
  ],
  "outbounds": [
    {
      "tag": "proxyA",
      "protocol": "freedom",
      "settings": {}
    },
    {
      "tag": "proxyB",
      "protocol": "freedom",
      "settings": {}
    },
    {
      "tag": "proxyC",
      "protocol": "freedom",
      "settings": {}
    }
  ],
  "api": {
    "tag": "api",
    "services": [
      "HandlerService"
    ]
  },
  "routing": {
    "rules": [
      {
        "inboundTag": [
          "api"
        ],
        "outboundTag": "api",
        "type": "field"
      },
      {
        "type": "field",
        "inboundTag": [
          "http"
        ],
        "balancerTag": "rr"
      }
    ],
    "balancers": [
      {
        "tag": "rr",
        "selector": [
          "proxy"
        ],
        "strategy": {
          "type": "roundRobin"
        }
      }
    ]
  }
}

Then i use this proxy to surf the internet. Here is the log.

Xray 1.8.6 (Xray, Penetrates Everything.) Custom (go1.21.5 windows/amd64)
A unified platform for anti-censorship.
2024/01/07 20:40:04 [Info] infra/conf/serial: Reading config: stdin:
2024/01/07 20:40:04 [Warning] core: Xray 1.8.6 started
2024/01/07 20:40:07 127.0.0.1:14005 accepted //data.bing.com:443 [http -> proxyA]
2024/01/07 20:40:10 127.0.0.1:14024 accepted //api.bing.com:443 [http -> proxyB]
2024/01/07 20:40:11 127.0.0.1:14030 accepted //upmirrors.bing.com:443 [http -> proxyC]
2024/01/07 20:40:12 127.0.0.1:14038 accepted //hw.bing.net:443 [http -> proxyA]
2024/01/07 20:40:25 127.0.0.1:14106 accepted //upmirrors.bing.com:443 [http -> proxyB]

So far so good.

Then i use api command to delete proxyB and add proxyD proxyE

xray.exe api rmo --server=127.0.0.1:10085 "proxyB"
xray.exe api ado --server=127.0.0.1:10085 add.json

add.json

{
  "outbounds": [
    {
      "tag": "proxyD",
      "protocol": "freedom",
      "settings": {}
    },
    {
      "tag": "proxyE",
      "protocol": "freedom",
      "settings": {}
    }
  ]
}

Now i surf the internet again.

2024/01/07 20:40:41 127.0.0.1:14179 accepted //www.bing.com:443 [http -> proxyC]
2024/01/07 20:40:41 127.0.0.1:14180 accepted //s1.bing.com:443 [http -> proxyA]
2024/01/07 20:40:43 [Warning] [3554809321] app/dispatcher: non existing outTag: proxyB
2024/01/07 20:40:43 127.0.0.1:14189 accepted //i0.bing.com:443 [http >> proxyA]
2024/01/07 20:40:43 127.0.0.1:14191 accepted //api.bing.com:443 [http -> proxyC]
2024/01/07 20:40:43 127.0.0.1:14194 accepted //s1.bing.com:443 [http -> proxyA]
2024/01/07 20:40:44 [Warning] [2439707019] app/dispatcher: non existing outTag: proxyB
2024/01/07 20:40:44 127.0.0.1:14201 accepted //s1.bing.com:443 [http >> proxyA]
2024/01/07 20:40:44 127.0.0.1:14204 accepted //passport.bing.com:443 [http -> proxyC]
2024/01/07 20:40:44 127.0.0.1:14206 accepted //cm.bing.com:443 [http -> proxyA]
2024/01/07 20:40:44 [Warning] [1430060763] app/dispatcher: non existing outTag: proxyB
2024/01/07 20:40:44 127.0.0.1:14209 accepted //cm.bing.com:443 [http >> proxyA]

As you can see the log above. RoundRobin is selecting a non-existing tag "proxyB" and never select the new added proxyD or proxyE.

The ">>" in [http >> proxyA] means xray can not find any outbound tag, and then use the default outbound, which is proxyA.

@hossinasaadi hossinasaadi mentioned this issue Jan 7, 2024
Merged
@hossinasaadi
Copy link
Contributor

fixed. #2906
thank you

@yuhan6665
Copy link
Member

Thanks both for your great collaboration! Should be fixed in latest version

@yomnxkcs
Copy link
Author

yomnxkcs commented Jan 8, 2024

i tested the new patch. "non-existing tag" warning is gone. But the order of selecting proxies is still not as expected. So i add one line of code.

fmt.Printf("tags: %v\n", tags)
if s.roundRobin == nil || !reflect.DeepEqual(s.roundRobin.tags, tags) {

Here is the log.

tags: [proxyA proxyD proxyC proxyE]
2024/01/08 08:42:09 127.0.0.1:12161 accepted //login.live.com:443 [http -> proxyA]
tags: [proxyC proxyE proxyA proxyD]
2024/01/08 08:42:10 127.0.0.1:12164 accepted //login.live.com:443 [http -> proxyC]
tags: [proxyE proxyA proxyD proxyC]
2024/01/08 08:42:11 127.0.0.1:12167 accepted //www.msn.cn:443 [http -> proxyE]
tags: [proxyA proxyD proxyC proxyE]
2024/01/08 08:42:13 127.0.0.1:12179 accepted //img-s.msn.cn:443 [http -> proxyA]
tags: [proxyC proxyE proxyA proxyD]
2024/01/08 08:42:13 127.0.0.1:12180 accepted //img-s.msn.cn:443 [http -> proxyC]
tags: [proxyA proxyD proxyC proxyE]
2024/01/08 08:42:13 127.0.0.1:12184 accepted //ts3.cn.mm.bing.net:443 [http -> proxyA]
tags: [proxyA proxyD proxyC proxyE]
2024/01/08 08:42:13 127.0.0.1:12185 accepted //ts3.cn.mm.bing.net:443 [http -> proxyD]
tags: [proxyE proxyA proxyD proxyC]
2024/01/08 08:42:13 127.0.0.1:12188 accepted //ts3.cn.mm.bing.net:443 [http -> proxyE]
tags: [proxyA proxyD proxyC proxyE]
2024/01/08 08:42:18 127.0.0.1:12209 accepted //login.live.com:443 [http -> proxyA]
tags: [proxyD proxyC proxyE proxyA]
2024/01/08 08:42:37 127.0.0.1:12283 accepted //edge.microsoft.com:443 [http -> proxyD]

Turns out tags passed to PickOutbound() is not in a fixed order.
But that might be another issue, so i will close this one now.
Thanks for your work.

@yomnxkcs yomnxkcs closed this as completed Jan 8, 2024
@yuhan6665
Copy link
Member

yuhan6665 commented Jan 8, 2024 •
edited
Loading

hmm interesting let's add tags to a set and compare ;) except there is no set in Go..

@hossinasaadi hossinasaadi mentioned this issue Jan 8, 2024
Merged
@hossinasaadi
Copy link
Contributor

@yomnxkcs , @yuhan6665 i think now it's fixed properly :)

@yuhan6665 yuhan6665 mentioned this issue Jan 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@yuhan6665 @hossinasaadi @yomnxkcs