From 6d04b4add30524756c8d42c13b10c7ef0dafe120 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 19 Sep 2017 15:28:03 -0600 Subject: [PATCH 01/27] Redis Cluster Support - Deprecated unused `redis` options in favour of options from `ioredis`. - Added support for ioredis Cluster. - Enabled more extended redis client configuration. --- config.js | 37 ++++++++++++++++---- docs/_docs/02-01-configuration.md | 19 +++++----- services/redis.js | 58 ++++++++++++------------------- 3 files changed, 64 insertions(+), 50 deletions(-) diff --git a/config.js b/config.js index 1a54ece5a..3d8b6213c 100644 --- a/config.js +++ b/config.js @@ -86,14 +86,17 @@ const CONFIG = { MONGO_URL: process.env.TALK_MONGO_URL, REDIS_URL: process.env.TALK_REDIS_URL, - // REDIS_RECONNECTION_MAX_ATTEMPTS is the amount of attempts that a redis - // connection will attempt to reconnect before aborting with an error. - REDIS_RECONNECTION_MAX_ATTEMPTS: parseInt(process.env.TALK_REDIS_RECONNECTION_MAX_ATTEMPTS || '100'), + // REDIS_CLIENT_CONFIG is the optional configuration that is merged with the + // function config to provide deep control of the redis connection beheviour. + REDIS_CLIENT_CONFIG: process.env.TALK_REDIS_CLIENT_CONFIGURATION || '{}', - // REDIS_RECONNECTION_MAX_RETRY_TIME is the time in string format for the - // maximum amount of time that a client can be considered "connecting" before - // attempts at reconnection are aborted with an error. - REDIS_RECONNECTION_MAX_RETRY_TIME: ms(process.env.TALK_REDIS_RECONNECTION_MAX_RETRY_TIME || '1 min'), + // REDIS_CLUSTER_MODE allows configuration on the type of cluster mode enabled + // on the redis client. Can be either `NONE` or `CLUSTER`. + REDIS_CLUSTER_MODE: process.env.TALK_REDIS_CLUSTER_MODE || 'NONE', + + // REDIS_CLUSTER_CONFIGURATION contains the json string for the redis cluster + // configuration. + REDIS_CLUSTER_CONFIGURATION: process.env.TALK_REDIS_CLUSTER_CONFIGURATION || '[]', // REDIS_RECONNECTION_BACKOFF_FACTOR is the factor that will be multiplied // against the current attempt count inbetween attempts to connect to redis. @@ -238,6 +241,26 @@ if (process.env.NODE_ENV === 'test' && !CONFIG.REDIS_URL) { CONFIG.REDIS_URL = 'redis://localhost/1'; } +// REDIS_CLUSTER_CONFIGURATION should be parsed when the cluster mode !== none. +if (CONFIG.REDIS_CLUSTER_MODE === 'CLUSTER') { + try { + CONFIG.REDIS_CLUSTER_CONFIGURATION = JSON.parse(CONFIG.REDIS_CLUSTER_CONFIGURATION); + } catch (err) { + throw new Error('TALK_REDIS_CLUSTER_CONFIGURATION is not valid JSON, see https://github.com/luin/ioredis#cluster for valid syntax of the list of cluster nodes'); + } + + if (!Array.isArray(CONFIG.REDIS_CLUSTER_CONFIGURATION)) { + throw new Error('TALK_REDIS_CLUSTER_MODE is CLUSTER, but the TALK_REDIS_CLUSTER_CONFIGURATION is invalid, see https://github.com/luin/ioredis#cluster for valid syntax of the list of cluster nodes'); + } + + if (CONFIG.REDIS_CLUSTER_CONFIGURATION.length === 0) { + throw new Error('TALK_REDIS_CLUSTER_CONFIGURATION must have at least one node specified in the cluster, see https://github.com/luin/ioredis#cluster for valid syntax of the list of cluster nodes'); + } +} + +// Client config is a JSON encoded string, defaulting to `{}`. +CONFIG.REDIS_CLIENT_CONFIG = JSON.parse(CONFIG.REDIS_CLIENT_CONFIG); + //------------------------------------------------------------------------------ // Recaptcha configuration //------------------------------------------------------------------------------ diff --git a/docs/_docs/02-01-configuration.md b/docs/_docs/02-01-configuration.md index 021735fb9..b3e637e27 100644 --- a/docs/_docs/02-01-configuration.md +++ b/docs/_docs/02-01-configuration.md @@ -55,18 +55,21 @@ These are only used during the webpack build. #### Advanced {:.no_toc} -- `TALK_REDIS_RECONNECTION_MAX_ATTEMPTS` (_optional_) - the amount of attempts - that a redis connection will attempt to reconnect before aborting with an - error. (Default `100`) -- `TALK_REDIS_RECONNECTION_MAX_RETRY_TIME` (_optional_) - the time in string - format for the maximum amount of time that a client can be considered - "connecting" before attempts at reconnection are aborted with an error. - (Default `1 min`) +- `TALK_REDIS_CLIENT_CONFIG` (_optional_) - configuration overrides for the + redis client configuration in a JSON encoded string. Configuration is + overridden as the second parameter to the redis client constructor, and is + merged with default configuration. (Default `{}`) - `TALK_REDIS_RECONNECTION_BACKOFF_FACTOR` (_optional_) - the time factor that will be multiplied against the current attempt count inbetween attempts to connect to redis. (Default `500 ms`) - `TALK_REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME` (_optional_) - the minimum time used to delay before attempting to reconnect to redis. (Default `1 sec`) +- `TALK_REDIS_CLUSTER_MODE` (_optional_) - the cluster mode of the redis client. + Can be either `NONE` or `CLUSTER`. (Default `NONE`) +- `TALK_REDIS_CLUSTER_CONFIGURATION` (_optional_) - the json serialized form of + the cluster nodes. Only required when `TALK_REDIS_CLUSTER_MODE=CLUSTER`. See + https://github.com/luin/ioredis#cluster for configuration details. + (Default `[]`) ### Server @@ -131,7 +134,7 @@ is not needed in most situations. use to set a cookie containing a JWT that was issued by Talk. (Default `process.env.TALK_JWT_COOKIE_NAME`) - `TALK_JWT_COOKIE_NAMES` (_optional_) - the different cookie names to check for - a JWT token in, seperated by `,`. By default, we always use the + a JWT token in, separated by `,`. By default, we always use the `process.env.TALK_JWT_COOKIE_NAME` and `process.env.TALK_JWT_SIGNING_COOKIE_NAME` for this value. Any additional cookie names specified here will be appended to the list of cookie names to inspect. diff --git a/services/redis.js b/services/redis.js index 5fd1b78d3..9b00b19db 100644 --- a/services/redis.js +++ b/services/redis.js @@ -1,12 +1,14 @@ const Redis = require('ioredis'); +const merge = require('lodash/merge'); const debug = require('debug')('talk:services:redis'); const enabled = require('debug').enabled('talk:services:redis'); const { REDIS_URL, - REDIS_RECONNECTION_MAX_ATTEMPTS, - REDIS_RECONNECTION_MAX_RETRY_TIME, REDIS_RECONNECTION_BACKOFF_FACTOR, REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME, + REDIS_CLIENT_CONFIG, + REDIS_CLUSTER_MODE, + REDIS_CLUSTER_CONFIGURATION, } = require('../config'); const attachMonitors = (client) => { @@ -16,8 +18,8 @@ const attachMonitors = (client) => { if (enabled) { client.on('connect', () => debug('client connected')); client.on('ready', () => debug('client ready')); - client.on('reconnecting', () => debug('client connection lost, attempting to reconnect')); client.on('close', () => debug('client closed the connection')); + client.on('reconnecting', () => debug('client connection lost, attempting to reconnect')); client.on('end', () => debug('client ended')); } @@ -27,44 +29,30 @@ const attachMonitors = (client) => { console.error('Error connecting to redis:', err); } }); + client.on('node error', (err) => debug('node error', err)); }; -const connectionOptions = { - retry_strategy: function(options) { - if (options.error && options.error.code !== 'ECONNREFUSED') { +function retryStrategy(times) { + const delay = Math.max(times * REDIS_RECONNECTION_BACKOFF_FACTOR, REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME); - debug('retry strategy: none, an error occured'); + debug(`retry strategy: try to reconnect ${delay} ms from now`); - // End reconnecting on a specific error and flush all commands with a individual error - return options.error; - } - if (options.total_retry_time > REDIS_RECONNECTION_MAX_RETRY_TIME) { - - debug('retry strategy: none, exhausted retry time'); - - // End reconnecting after a specific timeout and flush all commands with a individual error - return new Error('Retry time exhausted'); - } - - if (options.attempt > REDIS_RECONNECTION_MAX_ATTEMPTS) { - - debug('retry strategy: none, exhausted retry attempts'); - - // End reconnecting with built in error - return undefined; - } - - // reconnect after - const delay = Math.max(options.attempt * REDIS_RECONNECTION_BACKOFF_FACTOR, REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME); - - debug(`retry strategy: try to reconnect ${delay} ms from now`); - - return delay; - } -}; + return delay; +} const createClient = () => { - let client = new Redis(REDIS_URL, connectionOptions); + let client; + if (REDIS_CLUSTER_MODE === 'NONE') { + client = new Redis(REDIS_URL, merge({}, REDIS_CLIENT_CONFIG, { + retryStrategy, + })); + } else if (REDIS_CLUSTER_MODE === 'CLUSTER') { + client = new Redis.Cluster(REDIS_CLUSTER_CONFIGURATION, merge({ + scaleReads: 'slave', + }, REDIS_CLIENT_CONFIG, { + clusterRetryStrategy: retryStrategy, + })); + } // Attach the monitors that will print debug messages to the console. attachMonitors(client); From b60196e455fab7e0241f1156eae41fc768142299 Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Thu, 21 Sep 2017 12:42:45 +0100 Subject: [PATCH 02/27] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 115d94c12..e669dad7d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "talk", - "version": "3.5.1", + "version": "3.6.0", "description": "A better commenting experience from Mozilla, The New York Times, and the Washington Post. https://coralproject.net", "main": "app.js", "private": true, From 235b2e106870e2ea82d1610a192e2fe923278b96 Mon Sep 17 00:00:00 2001 From: Ryan Yun Date: Fri, 22 Sep 2017 11:10:39 -0400 Subject: [PATCH 03/27] add class name for my profile container --- client/coral-settings/containers/ProfileContainer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-settings/containers/ProfileContainer.js b/client/coral-settings/containers/ProfileContainer.js index 1391fa83f..4531ff7d3 100644 --- a/client/coral-settings/containers/ProfileContainer.js +++ b/client/coral-settings/containers/ProfileContainer.js @@ -71,7 +71,7 @@ class ProfileContainer extends Component { const emailAddress = localProfile && localProfile.id; return ( -
+

{user.username}

{emailAddress ?

{emailAddress}

: null} From d359d915fe017588b21ebc97668966d2882f7d68 Mon Sep 17 00:00:00 2001 From: Ryan Yun Date: Fri, 22 Sep 2017 11:11:22 -0400 Subject: [PATCH 04/27] add class name for configure container --- client/coral-configure/containers/ConfigureStreamContainer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-configure/containers/ConfigureStreamContainer.js b/client/coral-configure/containers/ConfigureStreamContainer.js index c009bbb76..6b1716930 100644 --- a/client/coral-configure/containers/ConfigureStreamContainer.js +++ b/client/coral-configure/containers/ConfigureStreamContainer.js @@ -102,7 +102,7 @@ class ConfigureStreamContainer extends Component { const closedTimeout = dirtySettings.closedTimeout; return ( -
+
Date: Fri, 22 Sep 2017 11:16:56 -0400 Subject: [PATCH 05/27] add static class names for respected and liked buttons --- plugins/talk-plugin-like/client/LikeButton.js | 2 +- plugins/talk-plugin-respect/client/RespectButton.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/talk-plugin-like/client/LikeButton.js b/plugins/talk-plugin-like/client/LikeButton.js index ece85537d..33f68c877 100644 --- a/plugins/talk-plugin-like/client/LikeButton.js +++ b/plugins/talk-plugin-like/client/LikeButton.js @@ -35,7 +35,7 @@ class LikeButton extends React.Component { return (
From 66518f50dd91cf53148e0a2c6c23455a8102bc33 Mon Sep 17 00:00:00 2001 From: Ryan Yun Date: Fri, 22 Sep 2017 11:41:54 -0400 Subject: [PATCH 07/27] add class name to off topic tag --- plugins/talk-plugin-offtopic/client/components/OffTopicTag.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/talk-plugin-offtopic/client/components/OffTopicTag.js b/plugins/talk-plugin-offtopic/client/components/OffTopicTag.js index 5b8de8875..265a175c5 100644 --- a/plugins/talk-plugin-offtopic/client/components/OffTopicTag.js +++ b/plugins/talk-plugin-offtopic/client/components/OffTopicTag.js @@ -2,12 +2,13 @@ import React from 'react'; import styles from './OffTopicTag.css'; import {t} from 'plugin-api/beta/client/services'; import {isTagged} from 'plugin-api/beta/client/utils'; +import cn from 'classnames'; export default (props) => ( { isTagged(props.comment.tags, 'OFF_TOPIC') && props.depth === 0 ? ( - + {t('off_topic')} ) : null From d60b8699376bd500afa5f9f8c004a84aa99f2d36 Mon Sep 17 00:00:00 2001 From: Ryan Yun Date: Fri, 22 Sep 2017 11:48:04 -0400 Subject: [PATCH 08/27] add class name to featured tag --- plugins/talk-plugin-featured-comments/client/components/Tag.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/talk-plugin-featured-comments/client/components/Tag.js b/plugins/talk-plugin-featured-comments/client/components/Tag.js index e7e75a0e6..4c65a52ec 100644 --- a/plugins/talk-plugin-featured-comments/client/components/Tag.js +++ b/plugins/talk-plugin-featured-comments/client/components/Tag.js @@ -34,7 +34,7 @@ export default class Tag extends React.Component { - + {t('talk-plugin-featured-comments.featured')} {tooltip && } From 605c87a7998c123b64615f8a131e660650043b72 Mon Sep 17 00:00:00 2001 From: Ryan Yun Date: Fri, 22 Sep 2017 12:16:31 -0400 Subject: [PATCH 09/27] featured and all comments have same left and right action container class names --- client/coral-embed-stream/src/components/Comment.js | 4 ++-- .../client/components/Comment.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index 9d3cb0218..622ec5e29 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -529,7 +529,7 @@ export default class Comment extends React.Component {
{isActive &&
-
+
}
-
+