Skip to content

Fix ActionCable Redis configuration with sentinels #48321

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

Merged

Conversation

moofkit
Copy link
Contributor

@moofkit moofkit commented May 28, 2023

Motivation / Background

This pull request addresses an integration bug between ActionCable and Redis >= 5.0 when using sentinels configuration. The bug causes issues because the sentinels configuration is a nested array of hashes, whereas the Redis client expects keys to be symbols. The proposed changes in this pull request modify the preparation of the Redis configuration to ensure that all keys are properly represented as symbols. This fix resolves the problem and improves the compatibility between ActionCable and Redis with sentinels.

cc: @byroot

Detail

It is possible to reproduce within this repo https://github.com/moofkit/actioncable-redis-sentinel-bug

$ bundle && bundle exec rails test
Running 1 tests in a single process (parallelization threshold is 50)
Run options: --seed 7348

# Running:

E

Error:
ActioncableTest#test_broadcasts_to_test_channel:
ArgumentError: unknown keywords: "host", "port"
    test/integration/actioncable_test.rb:5:in `block in <class:ActioncableTest>'

rails test test/integration/actioncable_test.rb:4

Finished in 0.043971s, 22.7423 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips

The problem is config for the cable parsed from YAML file and sentinels are deep nested hash in the array. But redis client expecting keys to be symbols, in the result it raises error ArgumentError: unknown keywords: "host", "port" in the redis-client

This issue can be fixed within application with this initializer code

# config/initializers/action_cable.rb
require "action_cable/subscription_adapter/redis"

# https://github.com/rails/rails/blob/da309a582e04183a85340ec0e1ed59b2d69ca9c2/actioncable/lib/action_cable/subscription_adapter/redis.rb#LL18C21-L18C61
ActionCable::SubscriptionAdapter::Redis.redis_connector = lambda do |config|
  # BUG: it needs to symbolize keys for redis gem >= 5.0.0
  Redis.new(config.deep_symbolize_keys.except(:adapter, :channel_prefix))
end

A redis-client related discussion: redis-rb/redis-client#121

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

This commit resolves an integration bug between ActionCable and Redis >= 5.0 when using sentinels configuration.
The issue arises from the fact that the sentinels configuration is a nested array of hashes, while the Redis client expects keys to be symbols.
This fix modifies the preparation of the Redis configuration to ensure that all keys are represented as symbols.
@byroot byroot merged commit d381d24 into rails:main May 29, 2023
byroot added a commit that referenced this pull request May 29, 2023
…ngs-arguments

Fix ActionCable Redis configuration with sentinels
@byroot
Copy link
Member

byroot commented May 29, 2023

Thank you. Backported to 7-0-stable as eb85e40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants