search

XTLS / Xray-core

Xray, Penetrates Everything. Also the best v2ray-core, with XTLS support. Fully compatible configuration.
https://t.me/projectXray
Mozilla Public License 2.0
24.59k stars 3.84k forks source link

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

Closed yomnxkcs closed 8 months ago

yomnxkcs commented 8 months ago

测试结果

=== 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 commented 8 months ago

round robin works like mentioned here https://github.com/XTLS/Xray-core/pull/2844#issuecomment-1868881702

yomnxkcs commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

fixed. https://github.com/XTLS/Xray-core/pull/2906 thank you

yuhan6665 commented 8 months ago

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

yomnxkcs commented 8 months ago

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.

yuhan6665 commented 8 months ago

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

hossinasaadi commented 8 months ago

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