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.

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: Rename maxUploadSize to maxUploadBytes, reduce server defaults #3611

Merged
merged 5 commits into from
Jul 29, 2024

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Jul 29, 2024

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

感觉 Upload 还是不够精准,全改为 Post:

  • maxConcurrentPosts
  • maxEachPostBytes
  • minPostsIntervalMs

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

你改代码就行了,PR 标题我来改

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

虽然这些名字还是没体现出来 per stream

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

要不后面都加个 PS?

@mmmray
Copy link
Collaborator Author

mmmray commented Jul 29, 2024

I don't have good ideas here. Instead of PS I would attempt maxStreamConcurrentUploads but it is a really long name and is also not intuitive.

I suggest that the parameters are actually not renamed and only the defaults are fixed.

@Fangliding
Copy link
Member

Fangliding commented Jul 29, 2024

我还是同意mmm的 更何况还是已发布的版本已经有的字段 有的修改为了兼容甚至要拖到下下个版本 但是一个config字段却要直接break(btw 我个人觉得这个协议还在beta阶段 break一下很正常 不用把有的改动还拖到下下个版本)
命名本身不用太纠结白话 毕竟大多数时候都是文档解释参数 除非和其他字段造成歧义

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

  • scMaxEachPostBytes
  • scMinPostsIntervalMs
  • scMaxConcurrentPosts

按这个改吧,sc 代表 sub-connection

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

有的修改为了兼容甚至要拖到下下个版本 但是一个config字段却要直接break

主要是 ok 那种协议交互的不好改,但这种几乎没人填的字段就该趁早 break,现有命名歧义太多,一开始我都没理解对

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

093558b 这个尾部也加上 Ms 吧

@Fangliding
Copy link
Member

Fangliding commented Jul 29, 2024

Ms改名部()

@mmmray mmmray force-pushed the splithttp-new-new-defaults branch from 093558b to eb064c9 Compare July 29, 2024 10:01
@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

准备好了后给我说

@mmmray
Copy link
Collaborator Author

mmmray commented Jul 29, 2024

yeah it's ready

@RPRX RPRX merged commit 30af792 into XTLS:main Jul 29, 2024
36 checks passed
mmmray added a commit to mmmray/Xray-core that referenced this pull request Aug 16, 2024
Since XTLS#3603 and
XTLS#3611, iperf and speedtest are
triggering "too large upload" by the server. This is because v2ray's
MultiBuffer pipe can actually return data larger than the configured
size limit.

I'm surprised nobody noticed it so far. In principle, any heavy upload
could disrupt the entire connection. In its infinite wisdom,
speedtest.net hides such errors and only shows low upload instead. I
only noticed this issue myself when inspecting server logs.
mmmray added a commit to mmmray/Xray-core that referenced this pull request Aug 16, 2024
Since XTLS#3603 and
XTLS#3611, iperf and speedtest are
triggering "too large upload" by the server. This is because v2ray's
MultiBuffer pipe can actually return data larger than the configured
size limit.

I'm surprised nobody noticed it so far. In principle, any heavy upload
could disrupt the entire connection. In its infinite wisdom,
speedtest.net hides such errors and only shows low upload instead. I
only noticed this issue myself when inspecting server logs.
mmmray added a commit to mmmray/Xray-core that referenced this pull request Aug 16, 2024
Since XTLS#3603 and
XTLS#3611, iperf and speedtest are
triggering "too large upload" by the server. This is because v2ray's
MultiBuffer pipe can actually return data larger than the configured
size limit.

I'm surprised nobody noticed it so far. In principle, any heavy upload
could disrupt the entire connection. In its infinite wisdom,
speedtest.net hides such errors and only shows low upload instead. I
only noticed this issue myself when inspecting server logs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants