Update with pub sub replication and Redis driver #61

snellingio · 2018-12-17T12:14:13Z

Still needing tests, and handling edge cases such as statistics, etc.

snellingio · 2018-12-17T12:14:40Z

This references #6

snellingio

A quick overview of what is going on in #6

snellingio · 2018-12-17T12:17:05Z

PubSub/PubSubInterface.php

+
+use React\EventLoop\LoopInterface;
+
+interface PubSubInterface


We add a new interface, so that we can implement other drivers such as zeroMQ or something similar. I don't need this functionality, but it will be asked for.

snellingio · 2018-12-17T12:17:44Z

PubSub/Redis/RedisClient.php

+class RedisClient implements PubSubInterface
+{
+
+    const REDIS_KEY = ':websockets:replication:';


We could make this configurable. Not sure how to keep this uncluttered in the config however.

snellingio · 2018-12-17T12:18:09Z

PubSub/Redis/RedisClient.php

+    public function __construct()
+    {
+        $this->apps     = collect(config('websockets.apps'));
+        $this->serverId = Str::uuid()->toString();


Set a unique server id, so that we don't get double messages on the current server.

snellingio · 2018-12-17T12:18:49Z

PubSub/Redis/RedisClient.php

+    public function subscribe(LoopInterface $loop): PubSubInterface
+    {
+        $this->loop = $loop;
+        [$this->publishClient, $this->subscribeClient] = Block\awaitAll([$this->publishConnection(), $this->subscribeConnection()], $this->loop);


You may wonder why we need both a publish connection and a subscribe connection.
clue/reactphp-redis#39 (comment)

To be honest I'm more wondering why you are blocking here. Is this bit ran inside the websocket server, or somewhere outside it in blocking PHP land?

If you don't await, you will end up returning a promise. Which isn't okay if you want the publish client to end up in the container. So what we do here is block until the promises resolve on the loop, and then push the actual instances on the variables instead of promises. Hopefully that makes sense.

So how are you restarting the loop? Or is this at the point where the loop hasn't been started yet by the server? Since await* functions stop the loop once the promises resolves/rejects.

Another option would be to do something like https://github.com/PHP-DI-Definitions/clue-redis-client/blob/master/src/WaitingClient.php (would pull out that client into another package if you wish to go that road) and fake the connection.

So this is going to be slightly confusing, because I don't fully understand Ratchet. But my understanding is that there is an event loop that is always running that you can push processes onto. The loop is started from the console command here:

laravel-websockets/src/Console/StartWebSocketServer.php

Line 36 in c1f6ffa

$this->loop = LoopFactory::create();

You don't need to restart the loop. Just block for a split second to get the redis connection promise to resolve, and then the loop continues.

Since this subscribe command only runs once, it will just connect when you boot up the websocket server, and then continue along in the background.

Waiting client stand alone package is up at https://packagist.org/packages/wyrihaximus/react-redis-waiting-client I'll be adding integration tests for subscribing to that over the next few days

Just to clarify, because it's clear the blocking approach was taken from my comment here: simonhamp/laravel-echo-ratchet-server#2 (comment), the reason I did it that way initially I think is just to lower the startup time by doing both connections at once. It's not strictly necessary. And like @snellingio said, it only ever happens right before the server is started, so there's no concern of the loop stopping.

Waiting client stand alone package is up at packagist.org/packages/wyrihaximus/react-redis-waiting-client I'll be adding integration tests for subscribing to that over the next few days

A better alternative for that package is up: clue/reactphp-redis#82

The lazy client release is out. That should make some thing simpler: https://github.com/clue/reactphp-redis/releases/tag/v2.3.0

Thanks @WyriHaximus

I'll try to pick up this PR in a bit, might make a new PR based on this one to finish it up.

snellingio · 2018-12-17T12:19:26Z

PubSub/Redis/RedisClient.php

+        );
+    }
+
+    protected function getConnectionUri()


This needs a test (and maybe a fallback), but it just pulls from the laravel app's config.

snellingio · 2018-12-17T12:22:41Z

composer.json

        "clue/buzz-react": "^2.5",
+        "clue/redis-react": "^2.2",


Unfortunately, we can't use the default redis driver. We could probably use it for publishing, but not for subscribing. Reason is that it is blocking :(

snellingio · 2018-12-17T12:23:05Z

config/websockets.php

@@ -112,6 +112,20 @@
        'passphrase' => null,
    ],

+    /*
+     * You can enable replication to publish and subscribe to messages across the driver


Tried to keep the config light, but this is something we should probably expand on.

snellingio · 2018-12-17T12:23:54Z

src/Console/StartWebSocketServer.php

+
+    protected function configurePubSubReplication()
+    {
+        if (config('websockets.replication.enabled') !== true) {


We should probably allow for cli integration here as well.

snellingio · 2018-12-17T12:24:38Z

src/Console/StartWebSocketServer.php

+        }
+
+        app()->singleton(PubSubInterface::class, function () use ($connection) {
+            return $connection;


This can break if the driver isn't set to redis and replication is enabled. Maybe we should throw an exception if the connection isn't set.

snellingio · 2018-12-17T12:25:20Z

src/WebSockets/Channels/Channel.php

    }

-    public function broadcastToEveryoneExcept($payload, ?string $socketId = null)
+    public function broadcastToEveryoneExcept($payload, ?string $socketId = null, ?string $appId = null)


Again, I don't love adding the appId here, but I didn't see another easy way to get it into the class.

snellingio · 2018-12-17T12:32:56Z

Oh, and since this is my PR, my biggest feature request for this is figuring out how to use the cli logger in the class. I'd like to have a lot more logging in the class. We can easily do this with the ->then() function within the redis class. But adding the logging in a clean way was where I spent a little too much time over the weekend, and then ripped it out.

…ondcode/laravel-websockets#61

snellingio · 2018-12-22T13:03:45Z

I will not be able to pick this back up until after the new year. While I'll be around to communicate, I just won't have the time to advance this pull request during the holidays.

Hope everyone has a great holiday.

francislavoie · 2019-02-08T08:42:36Z

@snellingio any chance you'll be picking this back up soon? I can probably give you a hand in finishing this PR soon, I'm working on a new project which will be using this lib.

snellingio · 2019-02-08T16:56:50Z

@francislavoie I'd love some help finishing the PR. Especially around tests for this change.

snellingio · 2019-03-29T14:34:29Z

Closing in favor of #140 by @francislavoie

Update with pub sub replication and redis driver

08cca69

snellingio commented Dec 17, 2018

View reviewed changes

freekmurze mentioned this pull request Dec 18, 2018

Horizontal scalability #6

Closed

WyriHaximus added a commit to WyriHaximus/reactphp-redis-waiting-client that referenced this pull request Dec 19, 2018

Lowered minimum PHP version to 7.1 to make sure it can be used in bey…

262cca0

…ondcode/laravel-websockets#61

francislavoie mentioned this pull request Mar 25, 2019

Redis as a replication backend for scalability #140

Merged

snellingio closed this Mar 29, 2019

Update with pub sub replication and Redis driver #61

Update with pub sub replication and Redis driver #61

Uh oh!

Conversation

snellingio commented Dec 17, 2018

Uh oh!

snellingio commented Dec 17, 2018

Uh oh!

snellingio left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snellingio commented Dec 17, 2018

Uh oh!

snellingio commented Dec 22, 2018

Uh oh!

francislavoie commented Feb 8, 2019

Uh oh!

snellingio commented Feb 8, 2019

Uh oh!

snellingio commented Mar 29, 2019

Uh oh!

Uh oh!