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

Prefer QUIC for WAN connections #8626

Closed
bt90 opened this issue Oct 27, 2022 · 16 comments
Closed

Prefer QUIC for WAN connections #8626

bt90 opened this issue Oct 27, 2022 · 16 comments
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)

Comments

@bt90
Copy link
Contributor

bt90 commented Oct 27, 2022

This is a bit of a long shot and depends on the performance of future quic-go releases but QUIC already tends to perform better on high latency connections:

https://forum.syncthing.net/t/prefer-quic-option/19136

https://www.reddit.com/r/Syncthing/comments/ye0429/syncthing_quicserver_quickclient_disconnecting/

Seems like quic-go v0.30.0 already comes with a bunch of performance improvements and batch writing is scheduled for v0.31.0.

Once quic-go is able to saturate a gigabit link, we should change the priority for WAN connections.

As for LAN i don't see any benefits because packetloss and latency is not an issue here.
@bt90 bt90 added enhancement New features or improvements of some kind, as opposed to a problem (bug) needs-triage New issues needed to be validated labels Oct 27, 2022
@calmh
Copy link
Member

calmh commented Oct 28, 2022

We might want to consider some sort of circuit breaker if implementing this, to prevent an eternal connection-upgrade fight between two differently configured devices.

@calmh calmh removed the needs-triage New issues needed to be validated label Oct 28, 2022
@bt90
Copy link
Contributor Author

bt90 commented Oct 28, 2022

At least to avoid issues with nodes running an older Syncthing version.

I wonder if we can already drop the QUIC->TCP upgrade logic.

@AudriusButkevicius
Copy link
Member

I also think the "gigabits" claim needs to be verified.
We have a lot of glue around quic, which might be the bottleneck achieving the said gigabytes.
We don't need that glue for TCP.

Also, if anything, the batch writes might break the existing glue rather than make things better.

@bt90
Copy link
Contributor Author

bt90 commented Oct 28, 2022

There's also been some discussion on caddys issue tracker around the performance of quic-go: caddyserver/caddy#4876

As you're mentioning the glue code support: AudriusButkevicius/pfilter#7

@calmh

This comment was marked as off-topic.

Sign in to view
@bt90 bt90 mentioned this issue Dec 24, 2022
Closed
@bt90
Copy link
Contributor Author

bt90 commented Jan 19, 2023

Looks like quic-go is fast enough for our purposes and is already aiming to be able to saturate 10Gbps links: quic-go/quic-go#3670

@imsodin
Copy link
Member

imsodin commented Jan 19, 2023

Yeah, the bare quic-go implementation is performant enough based on those reports. There's still the glue around it that Audrius mentioned plus real networks: I'd be surprised if UDP traffic works as well (more so on reliability than throughput) in the wild. Not because it couldn't, but because of old hard/software or policies/firewalls getting in the way. I'd be vary of changing the default, and maybe make it an opt-in config first. Plus the mentioned problem of flip-flop-management.

@bt90
Copy link
Contributor Author

bt90 commented Jan 19, 2023

I think the safest option would be to treat QUIC and TCP as equal, but choose QUIC first. This would avoid the flip-flop problem with older nodes. If only new nodes are involved and a QUIC connection could be established, it stays that way. An older device could force TCP, but again this would not cause any problems because the newer nodes would see no reason to switch it to QUIC.

So instead of:

Relay < QUIC WAN < TCP WAN < TCP LAN

We would treat it this way:

Relay < QUIC WAN == TCP WAN < TCP LAN

I'm not that concerened about UDP problems to be honest. The adoption of HTTP/3 forces things to work at least on the provider level. If it's blocked in corporate networks or public wifis we can still fallback to TCP.

@bt90
Copy link
Contributor Author

bt90 commented Jan 20, 2023

Blocked by #7403

@daiaji
Copy link

daiaji commented Jan 22, 2023

QoS on older gateways is actually less of a concern, as HTTP3 advances they have to accommodate large amounts of UDP data.

calmh added a commit to calmh/syncthing that referenced this issue Apr 11, 2023
@calmh
lib/config, lib/connections: Configurable protocol priority (ref sync…
d01ebae 
…thing#8626)

This makes the various protocol priorities configurable among the other
options. With this, it's possible to prefer QUIC over TCP for WAN
connections, for example. Both sides need to be similarly configured for
this to work properly.

The default priority order remains the same as previously (TCP, QUIC,
Relay, with LAN better than WAN).
calmh added a commit to calmh/syncthing that referenced this issue Apr 11, 2023
@calmh
lib/config, lib/connections: Configurable protocol priority (ref sync…
2fe47f2 
…thing#8626)

This makes the various protocol priorities configurable among the other
options. With this, it's possible to prefer QUIC over TCP for WAN
connections, for example. Both sides need to be similarly configured for
this to work properly.

The default priority order remains the same as previously (TCP, QUIC,
Relay, with LAN better than WAN).

To make this happen I made each dialer & listener more priority aware,
and moved the check for whether a connection is LAN or not into the
dialer / listener -- this is the new "lanChecker" type that's passed
around.
calmh added a commit that referenced this issue Apr 16, 2023
@calmh
lib/config, lib/connections: Configurable protocol priority (ref #8626)…
9b660c1 
… (#8868)

This makes the various protocol priorities configurable among the other
options. With this, it's possible to prefer QUIC over TCP for WAN
connections, for example. Both sides need to be similarly configured for
this to work properly.

The default priority order remains the same as previously (TCP, QUIC,
Relay, with LAN better than WAN).

To make this happen I made each dialer & listener more priority aware,
and moved the check for whether a connection is LAN or not into the
dialer / listener -- this is the new "lanChecker" type that's passed
around.
calmh added a commit to calmh/syncthing that referenced this issue Apr 25, 2023
@calmh
Merge branch 'main' into windowsfixtests
5280c4e 
* main:
  build(deps): bump github.com/quic-go/quic-go from 0.33.0 to 0.34.0 (syncthing#8877)
  gui, man, authors: Update docs, translations, and contributors
  lib/ignore: Properly handle non-existing included ignore-files (fixes syncthing#8764) (syncthing#8874)
  lib/connections: Avoid using nil lanChecker
  gui, man, authors: Update docs, translations, and contributors
  lib/config, lib/connections: Configurable protocol priority (ref syncthing#8626) (syncthing#8868)
  build: Upgrade recli (fixes syncthing#8503) (syncthing#8871)
calmh added a commit to calmh/syncthing that referenced this issue Apr 25, 2023
@calmh
Merge branch 'main' into multiquic
2a8dff6 
* main:
  gui, man, authors: Update docs, translations, and contributors
  lib/ignore: Properly handle non-existing included ignore-files (fixes syncthing#8764) (syncthing#8874)
  lib/connections: Avoid using nil lanChecker
  gui, man, authors: Update docs, translations, and contributors
  lib/config, lib/connections: Configurable protocol priority (ref syncthing#8626) (syncthing#8868)
  build: Upgrade recli (fixes syncthing#8503) (syncthing#8871)
calmh added a commit to calmh/syncthing that referenced this issue May 9, 2023
@calmh
Merge branch 'main' into newglobbing
8c282de 
* main: (42 commits)
  all: Correct various typos (syncthing#8870)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Set platform data, incl. copying ownership, for new folders w/ NoPermissions flag (syncthing#8883)
  gui: Add Thai (th) translation template. (syncthing#8887)
  build: Produce nightly release builds
  gui, man, authors: Update docs, translations, and contributors
  build: Bump actions version; fix Node 12 deprecation warning (syncthing#8881)
  build: Build Debian packages
  build: Sign for upgrades
  build: Notarize mac builds
  build(deps): bump github.com/quic-go/quic-go from 0.33.0 to 0.34.0 (syncthing#8877)
  gui, man, authors: Update docs, translations, and contributors
  lib/ignore: Properly handle non-existing included ignore-files (fixes syncthing#8764) (syncthing#8874)
  lib/connections: Avoid using nil lanChecker
  gui, man, authors: Update docs, translations, and contributors
  lib/config, lib/connections: Configurable protocol priority (ref syncthing#8626) (syncthing#8868)
  build: Upgrade recli (fixes syncthing#8503) (syncthing#8871)
  build: Update dependencies (syncthing#8869)
  lib/model: Improve path generation for auto accepted folders (fixes syncthing#8859) (syncthing#8860)
  docs: fix typo (syncthing#8857)
  ...
@pataquets
Copy link

This is a bit of a long shot and depends on the performance of future quic-go releases but QUIC already tends to perform better on high latency connections:

https://forum.syncthing.net/t/prefer-quic-option/19136

https://www.reddit.com/r/Syncthing/comments/ye0429/syncthing_quicserver_quickclient_disconnecting/

Seems like quic-go v0.30.0 already comes with a bunch of performance improvements and batch writing is scheduled for v0.31.0.

Once quic-go is able to saturate a gigabit link, we should change the priority for WAN connections.

+1

As for LAN i don't see any benefits because packetloss and latency is not an issue here.

Just wondering... QUIC (aka UDP) on LAN would be easy on network equipment which might come handy if there's lots of traffic. Also, being UDP easily dropped in need, it would coexist better with TCP streaming and low latency applications.
Just my 2c.

@calmh
Copy link
Member

calmh commented Aug 13, 2023

On further consideration I think this is resolved as you can now tell Syncthing to prefer QUIC, though it's not the default.

@calmh calmh closed this as completed Aug 13, 2023
@pataquets
Copy link

@calmh How do you tell ST to prefer QUIC? Is there any way to make it default (eg. a command line switch)?
I can file another issue, if required.

@calmh
Copy link
Member

calmh commented Aug 16, 2023

@pataquets by setting the connection priority for QUIC to be lower than TCP. Advanced options. I should write a doc article one of these days.

@The0013
Copy link

The0013 commented Aug 17, 2023

THANK YOU VERY MUCH

@nikow
Copy link

nikow commented Aug 21, 2023

image
Looks like we just need to tweak those numbers. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@calmh @AudriusButkevicius @pataquets @bt90 @nikow @imsodin @daiaji @The0013