Repository navigation
usockets: stagger connect attempts to the addresses of a name (RFC 8305) #41501
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
base: main
Are you sure you want to change the base?
Changes from all commits
6398e9a
de56f4c
271e7ad
23f5e16
27645b2
445215e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,10 @@ void sweep_timer_cb(struct us_internal_callback_t *cb); | |
| // when the sweep timer is disabled, we don't need to do anything | ||
| void sweep_timer_noop(struct us_timer_t *timer) {} | ||
|
|
||
| uint64_t us_internal_monotonic_ns(void) { | ||
| return uv_hrtime(); | ||
| } | ||
|
|
||
| void us_internal_enable_sweep_timer(struct us_loop_t *loop) { | ||
| loop->data.sweep_timer_count++; | ||
| if (loop->data.sweep_timer_count == 1) { | ||
|
|
@@ -118,7 +122,92 @@ void us_internal_sweep_if_due(struct us_loop_t *loop) { | |
| us_internal_timer_sweep(loop); | ||
| } | ||
|
|
||
| long long us_internal_connect_attempt_timeout_ns(struct us_loop_t *loop) { | ||
| if (loop->data.connect_next_attempt_ns < 0) { | ||
| return -1; | ||
| } | ||
| long long diff = loop->data.connect_next_attempt_ns - (long long) us_internal_monotonic_ns(); | ||
| return diff > 0 ? diff : 0; | ||
| } | ||
|
|
||
| #endif | ||
|
|
||
| /* ── Connection attempt delay (RFC 8305 §5) ─────────────────────────────── */ | ||
|
|
||
| #ifdef LIBUS_USE_LIBUV | ||
| static void connect_timer_cb(struct us_internal_callback_t *cb) { | ||
| us_internal_connect_attempts_if_due(cb->loop); | ||
| } | ||
|
|
||
| /* Arm (or re-arm) the uv timer for loop->data.connect_next_attempt_ns. */ | ||
| static void us_internal_connect_timer_arm(struct us_loop_t *loop) { | ||
| long long deadline = loop->data.connect_next_attempt_ns; | ||
| if (deadline < 0) { | ||
| us_timer_set(loop->data.connect_timer, (void (*)(struct us_timer_t *)) connect_timer_cb, 0, 0); | ||
| return; | ||
| } | ||
| /* Round up: uv measures from its cached loop time, so a truncated delay | ||
| * fires before the deadline. An early fire re-arms from the callback. */ | ||
| long long ms = (deadline - (long long) us_internal_monotonic_ns() + 999999LL) / 1000000LL; | ||
| us_timer_set(loop->data.connect_timer, (void (*)(struct us_timer_t *)) connect_timer_cb, ms > 0 ? (int) ms : 1, 0); | ||
| } | ||
| #endif | ||
|
|
||
| void us_internal_connect_attempt_schedule(struct us_connecting_socket_t *c) { | ||
| if (c->addrinfo_head == NULL) { | ||
| c->next_attempt_ns = 0; | ||
| return; | ||
| } | ||
| struct us_loop_t *loop = c->loop; | ||
| c->next_attempt_ns = (long long) us_internal_monotonic_ns() + LIBUS_CONNECT_ATTEMPT_DELAY_NS; | ||
| /* Every delay is the same length, so a new deadline is never earlier than | ||
| * one already armed. */ | ||
| if (loop->data.connect_next_attempt_ns < 0) { | ||
| loop->data.connect_next_attempt_ns = c->next_attempt_ns; | ||
| #ifdef LIBUS_USE_LIBUV | ||
| us_internal_connect_timer_arm(loop); | ||
| #endif | ||
| } | ||
| } | ||
|
|
||
| void us_internal_connect_attempts_if_due(struct us_loop_t *loop) { | ||
| if (loop->data.connect_next_attempt_ns < 0) { | ||
| return; | ||
| } | ||
| long long now = (long long) us_internal_monotonic_ns(); | ||
| if (now < loop->data.connect_next_attempt_ns) { | ||
| #ifdef LIBUS_USE_LIBUV | ||
| us_internal_connect_timer_arm(loop); | ||
| #endif | ||
| return; | ||
| } | ||
| long long next = -1; | ||
| for (struct us_socket_group_t *group = loop->data.head; group; group = group->next) { | ||
| for (struct us_connecting_socket_t *c = group->head_connecting_sockets; c; c = c->next_pending) { | ||
| if (c->next_attempt_ns <= 0 || c->closed) { | ||
| continue; | ||
| } | ||
| /* A connecting socket with a scheduled attempt always has at least | ||
| * one attempt pending: the failure path of the last pending | ||
| * attempt starts the next address itself and decides the outcome. | ||
| * So nothing here can close `c` or dispatch to the owner, and the | ||
| * lists stay intact while they are walked. */ | ||
| if (c->next_attempt_ns <= now && c->connecting_head != NULL) { | ||
| c->next_attempt_ns = 0; | ||
| if (us_internal_socket_start_next_attempt(c)) { | ||
| us_internal_connect_attempt_schedule(c); | ||
| } | ||
|
robobun marked this conversation as resolved.
Comment on lines
+195
to
+199
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) Operators of a multi-address host that is dropping SYNs now get one pending connect per address from every Bun dial, not the base branch's 4. The timer walk at loop.c:198-203 opens another address every 250 ms while earlier attempts stay pending and never counts them. A 24-record name holds 24 sockets per dial after 6 s; bun install's default 256 concurrent requests make 6144 retransmitting SYN streams against the struggling server. Fix: bound in-flight attempts per connecting socket on the timer path (close the timed-out attempt before dialing the next, as Node does, or cap in-flight at 2 like curl), keeping the immediate advance on failure at context.c:836. The description calls the missing cap intended; this amplification is its cost. Why this was flaggedTrigger: a name with many addresses whose SYNs are dropped rather than refused, dialed through fetch(), Bun.connect(), WebSocket or bun install. Base branch: start_connections opened 4 attempts and refilled only when at most one was pending, so a dial never held more than 4 pending connects. New code: us_internal_socket_start_next_attempt (context.c:698) opens exactly one, us_internal_connect_attempt_schedule (loop.c:157) re-arms 250 ms later, and the walk at loop.c:198-203 starts the next address whenever c->connecting_head != NULL with no count of how many attempts are already pending. For registry.npmjs.org a dial reaches 24 pending sockets after 23 x 250 ms = 5.75 s, each retransmitting SYNs for ~2 min. bun install runs up to MAX_SIMULTANEOUS_REQUESTS = 256 (src/http/AsyncHTTP.rs:70) such dials, so an overloaded registry sees up to 6144 SYN streams instead of 1024. The PR text states 'There is no cap on pending attempts' as a design choice; nothing in the code bounds it, and Node's autoSelectFamily and curl both bound it. Verification: Triggering condition: a name with many addresses whose SYNs are dropped. On the base,
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed that this is a cost, and it is now listed under Downsides in the PR body: with no bound, a name whose N addresses all drop SYNs holds N pending connects per dial after (N-1) x 250 ms, where the old code held 4. A bound has to keep the later addresses reachable, or the stall this PR fixes comes back (four black holes in front of a live fifth address). The two candidates are to close the oldest pending attempt when a new one starts past the bound, or to give each attempt a timeout the way Node does. I have not picked one yet. This thread stays open until I do. |
||
| } | ||
| if (c->next_attempt_ns > 0 && (next < 0 || c->next_attempt_ns < next)) { | ||
| next = c->next_attempt_ns; | ||
| } | ||
| } | ||
| } | ||
| loop->data.connect_next_attempt_ns = next; | ||
| #ifdef LIBUS_USE_LIBUV | ||
| us_internal_connect_timer_arm(loop); | ||
| #endif | ||
| } | ||
|
|
||
|
|
||
| /* -1 if the wakeup async cannot be created; nothing is left allocated in loop->data. */ | ||
|
|
@@ -127,9 +216,11 @@ int us_internal_loop_data_init(struct us_loop_t *loop, void (*wakeup_cb)(struct | |
| // We allocate with calloc, so we only need to initialize the specific fields in use. | ||
| #ifdef LIBUS_USE_LIBUV | ||
| loop->data.sweep_timer = us_create_timer(loop, 1, 0); | ||
| loop->data.connect_timer = us_create_timer(loop, 1, 0); | ||
| #else | ||
| loop->data.sweep_next_tick_ns = -1; | ||
| #endif | ||
| loop->data.connect_next_attempt_ns = -1; | ||
| loop->data.sweep_timer_count = 0; | ||
| loop->data.recv_buf = us_malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2); | ||
| loop->data.send_buf = us_malloc(LIBUS_SEND_BUFFER_LENGTH); | ||
|
|
@@ -144,6 +235,7 @@ int us_internal_loop_data_init(struct us_loop_t *loop, void (*wakeup_cb)(struct | |
| us_free(loop->data.send_buf); | ||
| #ifdef LIBUS_USE_LIBUV | ||
| us_timer_close(loop->data.sweep_timer, 0); | ||
| us_timer_close(loop->data.connect_timer, 0); | ||
| #endif | ||
| return -1; | ||
| } | ||
|
|
@@ -167,6 +259,7 @@ void us_internal_loop_data_free(struct us_loop_t *loop) { | |
|
|
||
| #ifdef LIBUS_USE_LIBUV | ||
| us_timer_close(loop->data.sweep_timer, 0); | ||
| us_timer_close(loop->data.connect_timer, 0); | ||
| if (loop->data.quic_timer) us_timer_close(loop->data.quic_timer, 0); | ||
| #endif | ||
| us_internal_async_close(loop->data.wakeup_async); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.