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

[1.x] Re-subscribes to the scaling channel when the underlying connection is lost #251

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

ashiquzzaman33
Copy link
Contributor

In scaling mode, when the subscribingClient connection is lost while the app is running, the main app continues to operate without any errors or logs. However, it loses the ability to listen for events on the scaling channel.

According to the clue/reactphp-redis:

When using the createLazyClient() method, the unsubscribe and punsubscribe events will be invoked automatically when the underlying connection is lost. This gives you control over re-subscribing to the channels and patterns as appropriate.

So, we need to re-subscribe to the channel when the unsubscribe event is detected.

@taylorotwell taylorotwell marked this pull request as draft September 22, 2024 15:04
@ashiquzzaman33 ashiquzzaman33 marked this pull request as ready for review September 23, 2024 07:59
@taylorotwell taylorotwell marked this pull request as draft September 23, 2024 13:34
@taylorotwell
Copy link
Member

Drafting until @joedixon can review.

@joedixon
Copy link
Collaborator

Hey @ashiquzzaman33 - did you manage to check if your implementation behaves as expected? According to the createLazyClient section of the docs, you would actually need to create a whole new connection.

Additionally, if the underlying database connection drops, it will automatically send the appropriate unsubscribe and punsubscribe events for all currently active channel and pattern subscriptions. This allows you to react to these events and restore your subscriptions by creating a new underlying connection repeating the above commands again.

@ashiquzzaman33
Copy link
Contributor Author

ashiquzzaman33 commented Sep 24, 2024

Hello @joedixon, thank you for your comment.

Yes, I’ve tested it, and the implementation behaves as expected. The documentation seems a bit misleading.
Re-subscribing should be sufficient unless we explicitly call $this->subscribingClient->close().

It will automatically create a new client (with underlying connection) if we try to re-subscribe.

@joedixon
Copy link
Collaborator

Thanks @ashiquzzaman33 do you think there is any scope to add a test for this functionality?

@ashiquzzaman33
Copy link
Contributor Author

Hi @joedixon,
Yes, I have added a unit test for this. Please review it.

Copy link
Collaborator

@joedixon joedixon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you

@joedixon joedixon marked this pull request as ready for review October 4, 2024 07:41
@joedixon joedixon changed the title Re-subscribe to the scaling channel when the underlying connection is lost [1.x] Re-subscribes to the scaling channel when the underlying connection is lost Oct 4, 2024
@taylorotwell taylorotwell merged commit 65631f1 into laravel:main Oct 4, 2024
9 checks passed
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

Successfully merging this pull request may close these issues.

3 participants