-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
SplitHTTP client: Add xmux (multiplex controller) for H3 & H2 #3613
Conversation
感谢 PR |
em 毕竟原理不一样 这个是复用client而不是mux.cool 所以放这还算河里吧 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just commenting on the implementation. maybe rprx wants more settings, but to me it seems this can resolve some quic instabilities in any case, so it's good in my opinion. thanks!
return muxMan.getClient(ctx, dest, streamSettings) | ||
} | ||
|
||
func createHTTPClient(ctx context.Context, dest net.Destination, streamSettings *internet.MemoryStreamConfig) *DefaultDialerClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was confused where this function was used, turns out it is directly pulled in from muxmanager
i suggest to make mux manager unit-testable (and add some tests), so it takes the function as an argument
Good job! |
Using xray version including these commits causes crash after a while when using on client, SplitHTTP with h3 alpn. If needed i can send my configs, i used new mux mechanism with h3. Server uses same version taken from Github Actions. I don't know if this helps. I just enabled new mux and tried "prefer_new", "prefer_reuse" and "prefer_existing", didn't change other options.
|
我这边大致的测试中:prefer_new更友好 在alpn是h2时若使用prefer_reuse,反而会变慢,不如关闭mux |
@ll11l1lIllIl1lll 可以 rebase 一下了,基于 main |
201d620
to
bcb0cb5
Compare
先改了下标题,晚点我比照着 #3560 (comment) 看下代码 |
general comment: i think there are already too many things called mux. most are related to the v2ray universe. there is some dangerous overlap with http with "h2mux" in sing-box. this is a mux but the rfcs don't call it that. can it be called "connection pool settings" or something else? |
首先 mode 不需要那么多别名,其次基础模式是二选一不是共存,prefer_reuse 就对应 concurrency,prefer_new 就对应另一个 好像没看到限制“一个连接最多被累计复用多少次”的代码 #3560 (comment) 下面说的 send rate 和 number of bytes sent/rcvd 等先不加, |
可以叫 XMC:Xray Multiplex Controller,因为这套机制完善后还要加给 Xray 的 Mux、gRPC、H2 等, |
@Fangliding xPaddingBytes 的文档 |
@Fangliding 以及 "Cache-Control: no-store" |
it's too awkward to update the document when the release is not out. starting some updates here XTLS/Xray-docs-next#558 |
#3652 的改动是默认的 没有config选项(((( |
To konsclufka: |
@mmmray |
Is this options for bypassing single connection speed limit of ISPs? |
I'll take over this PR. I might add some more things if there is time. I hope to have it ready by sunday. @APT-ZERO The answer is, yes can be added to grpc and a bunch of others, but it's not planned right now, and it can bypass speed limits like that, but I think if everybody uses it it will become meaningless (especially if you think of Irancell). That's all I'll say here. I suggest to go to github discussions or telegram for general discussion, I think this PR thread is already way too long and hard to navigate |
The PR has been updated, and my big "documentation" comment as well. I think it's good to review now. The mux manager no longer works at round-tripper level, but actually muxes |
改个名,既然 MaxUses 改名 ConnectionReuseTimes |
done, but the "demux" is too strange and different from mux.cool settings. In my mind "adding mux to config" means "configuring mux explicitly", not "adding mux". What about changing the default value of mux to |
|
bold proposal, mux settings tries to use the "transport-native" mux, then falls back to mux.cool if there is no such thing. prevent people from using mux.cool with h3, which is probably never a good idea (?) |
|
SplitHTTPMux->Xmux |
依我看最好别把逻辑搞太复杂 check来check去的 直接在文档警告别启用mux.cool就是了 grpc也有类似的警告 或者这样多层套娃没准对打乱指纹也有用 split这个直接叫muxcontroller或者类似的东西(如标题) xmux这个名字留给以后真port到mux |
我明天再想一下这个,应该还要重命名 |
前两个只能二选一,不能出现突破预期的行为,比如 maxConnections 不能被突破,除非受到后两个限制要开一条新连接 |
前两个只能二选一,均有值则报错,均无值则不限制复用,但后两个限制仍然可以生效 |
fineee, done. why restrict it though? it feels to me that this combination is fairly intuitive, although useless. |
@mmmray json 忘改了,还有字段顺序调整一下
我看了下之前的代码也没有预连接(没有 minConnections),那么 maxConnections 实际上会变得 meaningless |
关于默认值,现在相当于 maxConnections 默认为 1,其它均为 0 或者像 mux 一样改为默认由 maxConcurrency 主导,以及其它两项要不要有默认值,@mmmray |
so i think 0 and 1 are mostly logically equivalent.
I don't really know how one would dominate the other, since they are now mutually exclusive. |
Re-reading the comment, I think you're saying that |
我觉得如果写了 maxConnections,按字面意思它就不该被突破,否则会造成困惑,anyway 我们先不设任何默认值吧 |
Then i think the current state is good. maxConnections is broken if maxConcurrency is set to a value greater than 0. The user is not permitted to do that. The other case is |
终于合了这个 PR,感谢各位的努力! |
) XTLS#3613 (comment) Closes XTLS#3560 (comment) --------- Co-authored-by: mmmray <[email protected]>
我不确定我有没有正确理解其工作原理,并且我写完之后才发现上游已经更新了一堆提交,可能我合并的时候也哪里改炸了。直觉告诉我这可能有资源泄露问题,至少好像能用,所以我提交了。
这里假定 http.Client 会对每个请求复用连接,我实际使用的时候也看起来像是对请求复用连接。所以 idk.
这个代码其实挺烂的,我稍后再改,或者还是等另一个人去以更好的方式实现吧,至少肯定不是我。
注:好像 Interval 更倾向于定时任务的间隔,如果是硬限制最小间隔可能 Delay 更符合表达习惯。并且可能那 3 个 sc 参数也应该移到 mux