Opened 23 months ago

Closed 3 months ago

#25601 closed defect (fixed)

Multiplex - one snowflake proxy should be able to support multiple clients

Reported by: arlolra Owned by:
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords: snowflake, tor-pt, anti-censorship-roadmap-september
Cc: dcf, arlolra, cohosh Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor: Sponsor28-can

Description

Migrated from https://github.com/keroserene/snowflake/issues/11

This seems to exist for the proxy-go but the JavaScript side has things like,

MAX_NUM_CLIENTS = 1
CONNECTIONS_PER_CLIENT = 1

so I'm guessing it wasn't finished.

Child Tickets

Change History (10)

comment:1 Changed 14 months ago by arma

Dcf points out that this ticket isn't as urgent as it might seem, (a) because the headless proxy-go can already handle multiple clients at once, and (b) because the vision is that we'll have way way more snowflakes than clients, so most snowflakes will be idle most of the time, so the chances of a collision (more than one client sent to the same snowflake) are low.

I buy this logic in the short term, but it still seems to me that the broker design will get cleaner if the browser snowflakes can handle whichever clients try to use them, even if two show up together.

comment:2 Changed 13 months ago by gaba

Keywords: snowflake tor-pt added
Sponsor: Sponsor19

comment:3 Changed 8 months ago by gaba

Keywords: ex-sponsor-19 added

Adding the keyword to mark everything that didn't fit into the time for sponsor 19.

comment:4 Changed 8 months ago by phw

Sponsor: Sponsor19Sponsor28-can

Moving from Sponsor 19 to Sponsor 28.

comment:5 Changed 6 months ago by gaba

Cc: cohosh added
Keywords: anti-censorship-roadmap-september added; ex-sponsor-19 removed

comment:6 Changed 6 months ago by gaba

Points: 2

comment:7 Changed 3 months ago by cohosh

Status: newneeds_review

Okay so it turns out the multiplexing just works now that #31310 is out the way and we're using the existence of proxypairs to signal how many slots we have.

I did just a small touch-up of the code here to go with a more meaningful variable name that was going unused.

I tested this in snowbox by setting the maxNumClients to values other than 1 and it works great. However, I don't think we should change this yet as our problems right now are definitely not that we have too few browser-based proxies.

comment:8 Changed 3 months ago by cohosh

I should add that when we decide we want to increase the number of clients for browser proxies, we will probably want to make UI changes to allow users to modify the config. Again, I don't want to push out those changes yet until we sort out some of the network health issues we're seeing.

comment:9 Changed 3 months ago by dcf

Status: needs_reviewmerge_ready

Great! Nice work. I agree with leaving it set to 1 for now.

comment:10 Changed 3 months ago by cohosh

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.