add redisChannelPrefix option

[kongkx]

Try to fix redis channel subscribe.

#400

[thomaswardiii]

Do we need a change that does this? IMO absolutely. Will this be what we want? Not sure til there are metrics on this change

[thomaswardiii]

I like that this is simple by not touching a lot of code, but is using regexp performant enough for potentially higher traffic sites?

[kongkx]

I'm sorry. I have not yet consider the performance impact. In fact, I'm wondering how can I do a test about this. If you got some idea, let me know. I would like to do a test.

[KristianI]

I tried doing a simple test with a loop for the line in question:

var start = Date.now();

var redisChannelPrefix = 'laravel_database_';
var channel = 'laravel_database_private-user.1234';
var i;

for (i = 0; i < 1000000; i++) {
  var result = redisChannelPrefix ? channel.replace(new RegExp(`^${redisChannelPrefix}`), '') : channel;
}

console.log(Date.now() - start);

It shows that it takes around 300 milliseconds (0.3 seconds) to run the RegExp line 1.000.000 times.

In my book that is fine.

[thomaswardiii]

Cool! I like this, but it's obviously up to the maintainer then

[tthdvd]

When it will be released? It's quite important bug fix for me.

Thanks for the response!

[tlaverdure]

Could we just leverage the keyPrefix in ioredis?

https://github.com/luin/ioredis#transparent-key-prefixing

Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
   "databaseConfig": {
     "redis": {
       "keyPrefix": "laravel_database_"
      }
  }
}

Thanks

[tthdvd]

Could we just leverage the keyPrefix in ioredis?

https://github.com/luin/ioredis#transparent-key-prefixing

Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
  databaseConfig: {
     keyPrefix: "laravel_database_"
  }
}

Thanks

Still does not work for me.
The config:

        "database": "redis",
        "databaseConfig": {
                "redis": {
                             "keyPrefix": "laravel_database_"
                        }
        },

And still can't listen the channel

[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB authenticated for: private-App.User.1
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB joined channel: private-App.User.1
Channel: laravel_database_private-App.User.1
Event: App\Events\AcceptedBidEvent

[thomaswardiii]

Could we just leverage the keyPrefix in ioredis?

https://github.com/luin/ioredis#transparent-key-prefixing

Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
   "databaseConfig": {
     "redis": {
       "keyPrefix": "laravel_database_"
      }
  }
}

Thanks

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
  databaseConfig: {
     keyPrefix: "laravel_database_"
  }
}

Thanks

Still does not work for me.
The config:

        "database": "redis",
        "databaseConfig": {
                "redis": {
                             "keyPrefix": "laravel_database_"
                        }
        },

And still can't listen the channel

[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB authenticated for: private-App.User.1
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB joined channel: private-App.User.1
Channel: laravel_database_private-App.User.1
Event: App\Events\AcceptedBidEvent

Weird. Looks like it should maybe work

[tthdvd]

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
   "databaseConfig": {
     "redis": {
       "keyPrefix": "laravel_database_"
      }
  }
}

Thanks

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
  databaseConfig: {
     keyPrefix: "laravel_database_"
  }
}

Thanks

Still does not work for me.
The config:

        "database": "redis",
        "databaseConfig": {
                "redis": {
                             "keyPrefix": "laravel_database_"
                        }
        },

And still can't listen the channel
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB authenticated for: private-App.User.1
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB joined channel: private-App.User.1
Channel: laravel_database_private-App.User.1
Event: App\Events\AcceptedBidEvent

Weird. Looks like it should maybe work

I don't get it why this could solve the problem.

The problem is that the Laravel framework give prefixes for the key what is stored in redis. The resolution could be this pull request, to match (with regex) and listen for the channels

[thomaswardiii]

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
   "databaseConfig": {
     "redis": {
       "keyPrefix": "laravel_database_"
      }
  }
}

Thanks

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
  databaseConfig: {
     keyPrefix: "laravel_database_"
  }
}

Thanks

Still does not work for me.
The config:

        "database": "redis",
        "databaseConfig": {
                "redis": {
                             "keyPrefix": "laravel_database_"
                        }
        },

And still can't listen the channel
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB authenticated for: private-App.User.1
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB joined channel: private-App.User.1
Channel: laravel_database_private-App.User.1
Event: App\Events\AcceptedBidEvent

Weird. Looks like it should maybe work

I don't get it why this could solve the problem.

The problem is that the Laravel framework give prefixes for the key what is stored in redis. The resolution could be this pull request, to match (with regex) and listen for the channels

According to the documentation, the suggested fix tells the library being utilized (ioredis) to prefix that string infront of any string passed into the various key-related functions, so instead of putting the logic in the laravel-echo-server, we're attempting to use something built-in to the redis library it's using.

[tthdvd]

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
   "databaseConfig": {
     "redis": {
       "keyPrefix": "laravel_database_"
      }
  }
}

Thanks

Could we just leverage the keyPrefix in ioredis?
https://github.com/luin/ioredis#transparent-key-prefixing
Can someone test adding this to their laravel-echo-server.json file to see if it fixes it.

{
  databaseConfig: {
     keyPrefix: "laravel_database_"
  }
}

Thanks

Still does not work for me.
The config:

        "database": "redis",
        "databaseConfig": {
                "redis": {
                             "keyPrefix": "laravel_database_"
                        }
        },

And still can't listen the channel
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB authenticated for: private-App.User.1
[9:30:40 PM] - QPbaisz8B6JlVh7eAAAB joined channel: private-App.User.1
Channel: laravel_database_private-App.User.1
Event: App\Events\AcceptedBidEvent

Weird. Looks like it should maybe work

I don't get it why this could solve the problem.
The problem is that the Laravel framework give prefixes for the key what is stored in redis. The resolution could be this pull request, to match (with regex) and listen for the channels

According to the documentation, the suggested fix tells the library being utilized (ioredis) to prefix that string infront of any string passed into the various key-related functions, so instead of putting the logic in the laravel-echo-server, we're attempting to use something built-in to the redis library it's using.

I see, thanks for the answer.
unfortunately it doesn't work for me :(.

[tlaverdure]

Ok, I see now.

Laravel doesn't add the prefix to channels but it does to event broadcasting. I normally just turn redis prefixing off in my apps, so I haven't experienced this.

I think adding a new config option would add more confusion in getting setup. Let me think about this...

[KristianI]

Ok, I see now.

Laravel doesn't add the prefix to channels but it does to event broadcasting. I normally just turn redis prefixing off in my apps, so I haven't experienced this.

I think adding a new config option would add more confusion in getting setup. Let me think about this...

These two comments explains why using keyPrefix from ioredis will not work solely:
#400 (comment)
#400 (comment)

This package doesn't work with the default Laravel setup as it is currently. Implementing this PR will correct that.

Recommending removal of redis prefix from Laravel to make this package works feels kind of weird, as the Laravel core team decided that the prefix was needed with the following explanation:

This PR prefixes the redis database connection by default to mitigate multiple sites on the same server potentially sharing the same queued jobs. It uses similar logic to that which is used for creating the cache prefix.

Hope we can reach a good solution on this 🙏

[kongkx]

I think there should be more code to be added for handling "multiple sites on the same server potentially sharing the same queued jobs".

• laravel app "App_A" with redis prefix A_database_

• laravel app "App_B" with redix prefix B_database_

• laravel-echo-server A for AppA will also trying to handle B_database_private.User.1 message from App_B, although no message will be broadcasted through "socket-io" server.

Maybe we should test channel before calling the callback in RedisSubscriber.subscribe.

_this._redis.on('pmessage', function(subscribed, channel, message) {
    try {
        message = JSON.parse(message)
        if (_this.options.devMode) {
            log_1.Log.info('Channel: ' + channel)
            log_1.Log.info('Event: ' + message.event)
        }
        // appRedisPrefix may be a better config name
        var internalChannel
        if (this.options.appRedisPrefix) {
            var prefixRegex = new RegExp(
                '^' + _this.options.appRedisPrefix
            )
            internalChannel = channel.replace(prefixRegex, '')
            if (channel === internalChannel) {
                log_1.Log.info('Not target app event, not handled')
                return
            }
        } else {
            internalChannel = channel
        }
        callback(internalChannel, message)
    } catch (e) {
        if (_this.options.devMode) {
            log_1.Log.info('No JSON message')
        }
    }
})

[tlaverdure]

I think using keyPrefix should just work.

I sent a PR to laravel/framework to ignore the prefix when verifying channel authorization in channels.php.

laravel/framework#30597

[kongkx]

I think using keyPrefix should just work.

I sent a PR to laravel/framework to ignore the prefix when verifying channel authorization in channels.php.

laravel/framework#30597

With your pull request for laravel/framework, keyPrefix should work.

But I think it's quite confusing about the usage of keyPrefix (databaseConfig.redis), as it's used both in data storage and subscriber. In my opinion, it needs to be separated.

• data may conflict between laravel and laravel-echo-server in redis, although some cacke key pattern may help to solve this problem.
• laravel-echo-server may using another redis service for data storage which is not for broadcasting

[litan1106]

I ran into this issue as well.

[tlaverdure]

Closed by #464. Please also upgrade Laravel Framework to 6.5.2