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

Proposal: Add a Method to Reset Reconnect State in Centrifugo Client #106

Closed
shalom-aviv opened this issue Nov 27, 2024 · 6 comments
Closed

Comments

@shalom-aviv
Copy link
Contributor

Description:

Currently, the Centrifugo client uses an exponential backoff strategy for reconnecting after a disconnection. While this approach is generally effective, it can lead to a suboptimal scenario where:

  • The client is in the longest delay phase of the exponential backoff.
  • The network connection has already been restored and is stable.
  • The client is still waiting for the next reconnect attempt due to the backoff delay.

This situation results in unnecessary delays in reestablishing the connection, even though the network is ready.

Proposed Solutions:

  1. Manual Reconnection:
  • As a workaround, developers can manually disconnect and reconnect the Centrifugo client.
  • While this is functional, it adds complexity and requires developers to track the client's reconnecting state manually.
  1. Introduce a Method to Reset Reconnect State:
  • Add a method to the Centrifugo client that allows resetting the reconnect state and immediately starting a reconnect attempt if necessary.
  • This would simplify handling scenarios where the network connection has been restored, but the client is still in a long backoff phase.

Example of the proposed method:

public class CentrifugeClient {
    //...

    /**
     Reset reconnecting state to initial and schedule reconnect if needed.
     */
    public func resetReconnecting() {
        self.syncQueue.async { [weak self] in
            guard let strongSelf = self else { return }
            strongSelf.reconnectAttempts = 0

            guard strongSelf.reconnectTask != nil else { return }
            strongSelf.scheduleReconnect()
        }
    }

    //...
}

Benefits:

  • Improved UX: Allows quicker recovery from network interruptions by reconnecting as soon as the network is ready.
  • Developer Convenience: Simplifies reconnect management without requiring manual disconnection and reconnection.
  • Backward Compatibility: This change is additive and does not interfere with the existing reconnection logic.
@FZambia
Copy link
Member

FZambia commented Nov 29, 2024

Thanks for the suggestion @shalom-aviv ! Yep, it's important to have, in centrifuge-js we already react on browser's online/offline events so the connection is quickly re-established upon going online, but other SDKs do not provide hooks for this.

Generally, I am thinking now whether we need a separate method for that, or maybe can have this logic directly in connect method.

The downside of having it directly in the connect though is that it will be required to check whether client is in connecting state before calling connect to not occasionally move disconnected client to the connected state. Pseudocode:

// on online event
if client.state == 'connecting' {
  client.connect()
}

In this perspective having a separate method to resetReconnectingState and not bother about checking client state may be preferred.

WDYT?

@shalom-aviv
Copy link
Contributor Author

shalom-aviv commented Dec 2, 2024

Thanks for the suggestion @shalom-aviv ! Yep, it's important to have, in centrifuge-js we already react on browser's online/offline events so the connection is quickly re-established upon going online, but other SDKs do not provide hooks for this.

Generally, I am thinking now whether we need a separate method for that, or maybe can have this logic directly in connect method.

The downside of having it directly in the connect though is that it will be required to check whether client is in connecting state before calling connect to not occasionally move disconnected client to the connected state. Pseudocode:

// on online event
if client.state == 'connecting' {
  client.connect()
}

In this perspective having a separate method to resetReconnectingState and not bother about checking client state may be preferred.

WDYT?

I agree that adding logic directly into the connect method could potentially create confusion. The current lifecycle of connection through connect and disconnect is clear and intuitive, and adding an additional behavior for connect—especially one that requires checking the client state—might complicate its usage.

A separate resetReconnectingState method, seems to align better with the existing architecture and provides a clean and explicit way to manage the reconnection logic. Here's why I believe this approach is preferable:

  1. Consistency with Existing Lifecycle:
    The current lifecycle (connect to start, disconnect to stop) remains unchanged. A dedicated method for resetting the reconnection state complements this lifecycle without introducing ambiguity.

  2. Clear and Predictable Behavior:
    The purpose of resetReconnectingState is clear: reset the attempt count and delay to zero, and, if there is a scheduled reconnect attempt, start it immediately. This explicit behavior avoids the need to "peek" into the client's state before calling connect.

  3. Developer-Focused API:
    Developers can use resetReconnectingState in scenarios like restoring network connectivity without having to manage additional checks or worry about accidentally transitioning a client from disconnected to connected.

Here’s how the method could be described:


resetReconnectingState

  • What it does:
    • Resets the reconnect attempt counter to 0.
    • Resets the reconnect delay to 0.
    • If a reconnect attempt was already scheduled, it will start immediately.
  • What it doesn’t do:
    • It doesn’t initiate a new connection on its own. Developers still need to call connect if the client is in a disconnected state.

With this approach, the API remains intuitive and easy to use, while providing flexibility for scenarios where immediate reconnection is required.

WDYT?

@FZambia
Copy link
Member

FZambia commented Dec 4, 2024

Yep, I agree – decoupled method seems preferable. Please feel free to send PR, or I may address this a bit later – currently mostly spending available time on Centrifugo v6.

@shalom-aviv
Copy link
Contributor Author

Yep, I agree – decoupled method seems preferable. Please feel free to send PR, or I may address this a bit later – currently mostly spending available time on Centrifugo v6.

Are there any announcements or details available about Centrifugo v6? Will there be breaking changes, and how compatible will v6 be with v5?
Are there any exciting new features planned for v6? 😊

@FZambia
Copy link
Member

FZambia commented Dec 5, 2024

Take a look at centrifugal/centrifugo#832, also sharing updates in Telegram group (https://centrifugal.dev/docs/getting-started/community), from client protocol/SDK perspective I believe it should be fully compatible.

@FZambia
Copy link
Member

FZambia commented Dec 8, 2024

Many thanks @shalom-aviv for an awesome improvement! Released in https://github.com/centrifugal/centrifuge-swift/releases/tag/0.7.4

@FZambia FZambia closed this as completed Dec 8, 2024
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

No branches or pull requests

2 participants