From 312f64dcc353d534debf5f54531e6ccbe2230c8c Mon Sep 17 00:00:00 2001 From: davotoula Date: Fri, 12 Jun 2026 17:23:47 +0200 Subject: [PATCH 1/2] fix: don't reset relay reconnect backoff on momentary connections A relay that accepts the WebSocket handshake and then immediately resets the connection (e.g. essayist.decentnewsroom.com) defeated the exponential reconnect backoff. --- .../client/single/basic/BasicRelayClient.kt | 33 +++- .../basic/BasicRelayClientBackoffTest.kt | 185 ++++++++++++++++++ 2 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt index 46be99d3bb..d388bf1a2a 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt @@ -44,8 +44,11 @@ import kotlin.coroutines.cancellation.CancellationException * @property listener Interface to notify the application of relay events and errors. * * Reconnection Strategy: - * - Uses exponential backoff to retry connections, starting with [DELAY_TO_RECONNECT_IN_SECS] (500ms). + * - Uses exponential backoff to retry connections, starting with [DELAY_TO_RECONNECT_IN_SECS]. * - Doubles the delay between reconnection attempts in case of failure. + * - The backoff only resets after a connection stays open for at least + * [STABLE_CONNECTION_IN_SECS], so relays that accept the handshake but + * immediately drop the socket keep backing off instead of reconnecting in a loop. * * Message Handling: * - Processes relay messages (e.g., `EVENT`, `EOSE`, `OK`, `AUTH`) and delegates to appropriate callbacks. @@ -56,10 +59,17 @@ open class BasicRelayClient( override val url: NormalizedRelayUrl, val socketBuilder: WebsocketBuilder, val listener: RelayConnectionListener, + val nowInSeconds: () -> Long = TimeUtils::now, ) : IRelayClient { companion object { // minimum wait time to reconnect: 1 second const val DELAY_TO_RECONNECT_IN_SECS = 1 + + // a connection must survive this long before a disconnect resets the + // reconnect backoff. Relays that accept the handshake and then + // immediately drop the socket would otherwise reset the backoff on + // every onOpen and reconnect in a tight ~3s loop forever. + const val STABLE_CONNECTION_IN_SECS = TimeUtils.ONE_MINUTE } private var socket: WebSocket? = null @@ -74,6 +84,10 @@ open class BasicRelayClient( private var lastConnectTentativeInSeconds: Long = 0L // the beginning of time. private var delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS + // when the current connection became ready; used to decide if the + // connection was stable enough to reset the backoff on disconnect. + private var connectedAtInSeconds: Long = 0L + // Makes sure only one socket is open for each url private var connectingMutex = AtomicBoolean(false) @@ -97,7 +111,7 @@ open class BasicRelayClient( listener.onConnecting(this) - lastConnectTentativeInSeconds = TimeUtils.now() + lastConnectTentativeInSeconds = nowInSeconds() socket = socketBuilder.build(url, MyWebsocketListener()) socket?.connect() @@ -193,12 +207,21 @@ open class BasicRelayClient( fun markConnectionAsReady(usingCompression: Boolean) { this.isReady = true this.usingCompression = usingCompression + this.connectedAtInSeconds = nowInSeconds() - // resets any extra delays added during on offline state - this.delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS + // The backoff delay is NOT reset here: a relay that accepts the + // handshake and then immediately fails would defeat the exponential + // backoff on every cycle. It resets in markConnectionAsClosed once + // the session proves stable (see STABLE_CONNECTION_IN_SECS). } fun markConnectionAsClosed() { + // resets any extra delays added while offline, but only if the + // session was stable; flapping relays keep their growing backoff. + if (isReady && nowInSeconds() - connectedAtInSeconds >= STABLE_CONNECTION_IN_SECS) { + this.delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS + } + this.socket = null this.isReady = false this.usingCompression = false @@ -232,7 +255,7 @@ open class BasicRelayClient( } // waits 60 seconds to reconnect after disconnected. - if (ignoreRetryDelays || TimeUtils.now() > lastConnectTentativeInSeconds + delayToConnectInSeconds) { + if (ignoreRetryDelays || nowInSeconds() > lastConnectTentativeInSeconds + delayToConnectInSeconds) { upRelayDelayToConnect() connect() } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt new file mode 100644 index 0000000000..a4f8e83015 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt @@ -0,0 +1,185 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.relay.client.single.basic + +import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocket +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder +import kotlin.test.Test +import kotlin.test.assertTrue + +/** + * Reproduces the reconnect storm from the 2026-06-12 benchmark sweep: a relay + * that completes the WebSocket handshake and then immediately resets the + * connection must still be subject to exponential backoff. Before the fix, + * onOpen reset the backoff delay to 1s on every cycle, producing a reconnect + * attempt every ~3s indefinitely (733 attempts in 40 min for one relay). + */ +class BasicRelayClientBackoffTest { + private val url = NormalizedRelayUrl("wss://flaky.example.com") + + class FakeWebSocket : WebSocket { + override fun needsReconnect() = false + + override fun connect() {} + + override fun disconnect() {} + + override fun send(msg: String) = true + } + + class FakeWebsocketBuilder : WebsocketBuilder { + var connectAttempts = 0 + lateinit var lastListener: WebSocketListener + + override fun build( + url: NormalizedRelayUrl, + out: WebSocketListener, + ): WebSocket { + connectAttempts++ + lastListener = out + return FakeWebSocket() + } + } + + class NoopListener : RelayConnectionListener + + class MutableClock( + var now: Long = 1_000_000L, + ) + + /** + * Simulates [totalSeconds] of wall-clock time with a pool tick every + * [tickSeconds] (subscription churn calls connectAndSyncFiltersIfDisconnected + * roughly this often during active use). [onConnectAttempt] is invoked after + * every new socket build so the test can drive the relay's behavior. + */ + private fun runTicks( + client: BasicRelayClient, + builder: FakeWebsocketBuilder, + clock: MutableClock, + totalSeconds: Long, + tickSeconds: Long = 3, + onConnectAttempt: (WebSocketListener) -> Unit, + ) { + val end = clock.now + totalSeconds + while (clock.now < end) { + clock.now += tickSeconds + val before = builder.connectAttempts + client.connectAndSyncFiltersIfDisconnected() + if (builder.connectAttempts > before) { + onConnectAttempt(builder.lastListener) + } + } + } + + private fun newClient( + builder: FakeWebsocketBuilder, + clock: MutableClock, + ) = BasicRelayClient( + url = url, + socketBuilder = builder, + listener = NoopListener(), + nowInSeconds = { clock.now }, + ) + + @Test + fun handshakeThenResetRelayStillBacksOffExponentially() { + val builder = FakeWebsocketBuilder() + val clock = MutableClock() + val client = newClient(builder, clock) + + // first connection: handshake completes, then the server resets. + client.connect() + builder.lastListener.onOpen(50, false) + builder.lastListener.onFailure(RuntimeException("Connection reset"), null, null) + + // 40 minutes of pool ticks every 3s; the relay flaps on every attempt. + runTicks(client, builder, clock, totalSeconds = 40 * 60) { listener -> + listener.onOpen(50, false) + listener.onFailure(RuntimeException(), null, null) + } + + // exponential backoff (1,2,4,...,300s cap) allows at most ~17 attempts + // in 40 min. The bug produced one attempt per ~2 ticks (hundreds). + assertTrue( + builder.connectAttempts <= 20, + "Expected exponential backoff to cap reconnects, got ${builder.connectAttempts} attempts in 40 min", + ) + } + + @Test + fun preHandshakeFailuresBackOffExponentially() { + val builder = FakeWebsocketBuilder() + val clock = MutableClock() + val client = newClient(builder, clock) + + client.connect() + builder.lastListener.onFailure(RuntimeException("SSL handshake failed"), null, null) + + runTicks(client, builder, clock, totalSeconds = 40 * 60) { listener -> + listener.onFailure(RuntimeException("SSL handshake failed"), null, null) + } + + assertTrue( + builder.connectAttempts <= 20, + "Expected exponential backoff for pre-handshake failures, got ${builder.connectAttempts} attempts", + ) + } + + @Test + fun stableConnectionResetsBackoffAfterDisconnect() { + val builder = FakeWebsocketBuilder() + val clock = MutableClock() + val client = newClient(builder, clock) + + // grow the backoff with a few flapping cycles. + client.connect() + builder.lastListener.onOpen(50, false) + builder.lastListener.onFailure(RuntimeException(), null, null) + runTicks(client, builder, clock, totalSeconds = 120) { listener -> + listener.onOpen(50, false) + listener.onFailure(RuntimeException(), null, null) + } + + // now the relay recovers: connection stays up for 10 minutes, + // then the server closes it cleanly. + runTicks(client, builder, clock, totalSeconds = 15 * 60) { listener -> + listener.onOpen(50, false) + } + check(client.isConnected()) { "Test setup: relay should have reconnected and stayed up" } + clock.now += 10 * 60 + builder.lastListener.onClosed(1000, "server restart") + + // after a long stable session, the relay should reconnect quickly + // (within a few ticks), not after the accumulated multi-minute delay. + val attemptsBefore = builder.connectAttempts + runTicks(client, builder, clock, totalSeconds = 15) { listener -> + listener.onOpen(50, false) + } + assertTrue( + builder.connectAttempts > attemptsBefore, + "Expected backoff to reset after a stable session: no reconnect within 15s of a clean close", + ) + } +} From 33a7ef3be530f3e434a3e889b9f010afafc8ccf7 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 14 Jun 2026 18:46:17 +0200 Subject: [PATCH 2/2] Code review: - harden relay backoff fields for cross-thread access - reuse EmptyConnectionListener in backoff test - extract shared relay-client test fakes --- .../client/single/basic/BasicRelayClient.kt | 16 +++-- .../basic/BasicRelayClientBackoffTest.kt | 34 +---------- .../single/basic/BasicRelayClientTest.kt | 29 +-------- .../single/basic/RelayClientTestFakes.kt | 61 +++++++++++++++++++ 4 files changed, 76 insertions(+), 64 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/RelayClientTestFakes.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt index d388bf1a2a..3ffaca6d92 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClient.kt @@ -31,6 +31,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.TimeUtils +import kotlin.concurrent.Volatile import kotlin.concurrent.atomics.AtomicBoolean import kotlin.concurrent.atomics.ExperimentalAtomicApi import kotlin.coroutines.cancellation.CancellationException @@ -75,18 +76,22 @@ open class BasicRelayClient( private var socket: WebSocket? = null // True if it has received the onOpen call from the socket. - private var isReady: Boolean = false + // @Volatile: written on the serialized socket-callback thread, read from the + // relay-pool/timer thread (see RelayLoadingCursors for the same pattern). + @Volatile private var isReady: Boolean = false private var usingCompression: Boolean = false // keeps increasing the delay to connect when errors happen. // This avoids the constant desire to connect when the server is - // having trouble or offline. - private var lastConnectTentativeInSeconds: Long = 0L // the beginning of time. - private var delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS + // having trouble or offline. @Volatile: read on the pool thread in + // connectAndSyncFiltersIfDisconnected, written on the socket-callback thread. + @Volatile private var lastConnectTentativeInSeconds: Long = 0L // the beginning of time. + + @Volatile private var delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS // when the current connection became ready; used to decide if the // connection was stable enough to reset the backoff on disconnect. - private var connectedAtInSeconds: Long = 0L + @Volatile private var connectedAtInSeconds: Long = 0L // Makes sure only one socket is open for each url private var connectingMutex = AtomicBoolean(false) @@ -230,6 +235,7 @@ open class BasicRelayClient( override fun disconnect() { lastConnectTentativeInSeconds = 0L // this is not an error, so prepare to reconnect as soon as requested. delayToConnectInSeconds = DELAY_TO_RECONNECT_IN_SECS + connectedAtInSeconds = 0L socket?.disconnect() socket = null isReady = false diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt index a4f8e83015..b9c2cd279c 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientBackoffTest.kt @@ -20,11 +20,9 @@ */ package com.vitorpamplona.quartz.nip01Core.relay.client.single.basic -import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener +import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.EmptyConnectionListener import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl -import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocket import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener -import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder import kotlin.test.Test import kotlin.test.assertTrue @@ -38,32 +36,6 @@ import kotlin.test.assertTrue class BasicRelayClientBackoffTest { private val url = NormalizedRelayUrl("wss://flaky.example.com") - class FakeWebSocket : WebSocket { - override fun needsReconnect() = false - - override fun connect() {} - - override fun disconnect() {} - - override fun send(msg: String) = true - } - - class FakeWebsocketBuilder : WebsocketBuilder { - var connectAttempts = 0 - lateinit var lastListener: WebSocketListener - - override fun build( - url: NormalizedRelayUrl, - out: WebSocketListener, - ): WebSocket { - connectAttempts++ - lastListener = out - return FakeWebSocket() - } - } - - class NoopListener : RelayConnectionListener - class MutableClock( var now: Long = 1_000_000L, ) @@ -99,7 +71,7 @@ class BasicRelayClientBackoffTest { ) = BasicRelayClient( url = url, socketBuilder = builder, - listener = NoopListener(), + listener = EmptyConnectionListener, nowInSeconds = { clock.now }, ) @@ -167,7 +139,7 @@ class BasicRelayClientBackoffTest { runTicks(client, builder, clock, totalSeconds = 15 * 60) { listener -> listener.onOpen(50, false) } - check(client.isConnected()) { "Test setup: relay should have reconnected and stayed up" } + assertTrue(client.isConnected(), "Test setup: relay should have reconnected and stayed up") clock.now += 10 * 60 builder.lastListener.onClosed(1000, "server restart") diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientTest.kt index f46945cde4..065ef0fcaf 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/BasicRelayClientTest.kt @@ -23,36 +23,11 @@ package com.vitorpamplona.quartz.nip01Core.relay.client.single.basic import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl -import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocket import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener -import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertNotNull class BasicRelayClientTest { - private class FakeWebSocket : WebSocket { - override fun needsReconnect() = false - - override fun connect() {} - - override fun disconnect() {} - - override fun send(msg: String) = true - } - - private class FakeWebsocketBuilder : WebsocketBuilder { - var capturedListener: WebSocketListener? = null - - override fun build( - url: NormalizedRelayUrl, - out: WebSocketListener, - ): WebSocket { - capturedListener = out - return FakeWebSocket() - } - } - private class RecordingConnectionListener : RelayConnectionListener { val cannotConnectMessages = mutableListOf() @@ -81,9 +56,7 @@ class BasicRelayClientTest { listener, ) client.connect() - val socketListener = builder.capturedListener - assertNotNull(socketListener) - return Harness(socketListener, listener) + return Harness(builder.lastListener, listener) } @Test diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/RelayClientTestFakes.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/RelayClientTestFakes.kt new file mode 100644 index 0000000000..f30c3eb473 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/single/basic/RelayClientTestFakes.kt @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.relay.client.single.basic + +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocket +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder + +/** + * No-op [WebSocket] for tests. connect/disconnect/send do nothing; the relay's + * behavior is driven by invoking the [WebSocketListener] callbacks captured by + * [FakeWebsocketBuilder]. + */ +class FakeWebSocket : WebSocket { + override fun needsReconnect() = false + + override fun connect() {} + + override fun disconnect() {} + + override fun send(msg: String) = true +} + +/** + * [WebsocketBuilder] that records how many sockets were built ([connectAttempts]) + * and exposes the most recently captured [WebSocketListener] so a test can drive + * onOpen/onFailure/onClosed. [lastListener] is set on every [build]; read it only + * after the client has attempted to connect. + */ +class FakeWebsocketBuilder : WebsocketBuilder { + var connectAttempts = 0 + lateinit var lastListener: WebSocketListener + + override fun build( + url: NormalizedRelayUrl, + out: WebSocketListener, + ): WebSocket { + connectAttempts++ + lastListener = out + return FakeWebSocket() + } +}