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

permit redis pub/sub to work when host != localhost #1277

Merged
merged 1 commit into from
Jan 3, 2016
Merged

permit redis pub/sub to work when host != localhost #1277

merged 1 commit into from
Jan 3, 2016

Conversation

msimerson
Copy link
Member

it's complicated

Previously, the redis plugin worked well, except when the pub/sub channel was on a host other than 127.0.0.1. PR #1275 sorta fixed the karma plugin so that it could subscribe to a remote redis when the redis plugin (and by extension, result_store) could pub/sub to that remote redis host. It was not an optimal way to fix it though, because it duplicated the redis.ini parsing and connection logic.

With these changes, the redis connection and subscription functions are consolidated into the redis plugin, where they can be called by plugins that subscribe to result_store messages (currently karma & watch). Now the redis plugin has a config section for pubsub and all publishers and subscribers can use the same settings, without each subscriber having to duplicate the pubsub connection logic. That logic was complicated by the fact that subscribers must have a different redis client than the publisher.

the changes

  • redis.md: add Publish/Subscribe usage
  • redis.js:
    • rename plugin.cfg -> plugin.redisCfg (make it safe to inherit redis.js)
    • refactored get_redis_client() so it can be reused
    • add channel naming logic consistent with result_store publishing (so subscribers don't have to specify)
    • cleaned up connection log messages (see below diff)
      • client.host and client.port aren't always defined
  • karma.ini: rename redis config settings to match redis.ini (with backwards compatible shims)
  • karma.js:
    • inherit redis
    • remove load_redis_ini()
    • move config defaults into config loading function
    • use inherited redis pub/sub for sub connection
    • use new redis.get_redis_client for getting client
  • watch:
    • newer version that uses pub/sub instead of traversing connection object
    • inherits redis.js (so it too can subscribe to a remote redis)

connection message

-Dec 23 14:12:00 node haraka[10834]: [INFO] [-] [redis] connected to undefined/3 v3.0.4
-Dec 23 14:59:12 node haraka[50398]: [INFO] [-] [redis] dbid 3 selected
+Dec 23 17:29:15 node haraka[84229]: [INFO] [-] [redis] connected to redis://172.16.15.16:6379 v3.0.4

* redis.md: add Publish/Subscribe usage
* redis.js:
    * rename plugin.cfg -> plugin.redisCfg (make it safe to inherit
redis.js)
    * refactored get_redis_client() so it can be reused
    * add channel naming logic consistent with result_store publishing
* karma.ini: rename redis config settings to match redis.ini
* karma.js:
    * inherit redis
    * remove load_redis_ini()
    * move config defaults into config loading function
    * use inherited redis pub/sub for sub connection
    * use new redis.get_redis_client for getting client
* watch: update for redis pub/sub
* tame some overly verbose logging
msimerson added a commit that referenced this pull request Jan 3, 2016
permit redis pub/sub to work when host != localhost
@msimerson msimerson merged commit 7287665 into haraka:master Jan 3, 2016
@msimerson msimerson deleted the redis-pubsub-refactor branch January 3, 2016 05:35
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.

1 participant