Commit Graph

83 Commits

Author SHA1 Message Date
Alan Guo Xiang Tan acb6222caf
Revert "DEV: Upgrade Redis to 4.8" (#18099)
This reverts commit 0b5a0fd857.

Pending release of Sidekiq 6.5.6 so that we don't get depreciation
warnings
2022-08-26 15:16:50 +10:00
Alan Guo Xiang Tan 0b5a0fd857 DEV: Upgrade Redis to 4.8 2022-08-25 11:11:08 +08:00
Alan Guo Xiang Tan de9fe907ee
DEV: Readonly Redis support for `DiscourseRedis#multi/pipelined` (#16744)
Follow-up to 2df3c65ba9
2022-05-13 16:18:13 +08:00
Sam 2df3c65ba9
FIX: add support for pipelined and multi redis commands (#16682)
Latest redis interoduces a block form of multi / pipelined, this was incorrectly
passed through and not namespaced.

Fix also updates logster, we held off on upgrading it due to missing functions
2022-05-10 08:19:02 +10:00
Jarek Radosz 32087be531
DEV: Remove DiscourseRedis.namespace (#15993)
It was soft-deprecated 7 years ago.
2022-02-18 18:44:22 +01:00
David Taylor 11c93342dc
DEV: Consolidate Redis evalsha logic into DiscourseRedis::EvalHelper (#15957) 2022-02-15 16:06:12 +00:00
Osama Sayegh 1fc3cf0569
DEV: Make DiscourseRedis#del support deleting multiple keys (#15905)
Redis supports deleting multiple keys at once using the `del` command and so does the `redis` gem: 21ec1dec06/lib/redis/commands/keys.rb (L188-L193). However, our wrapper around the `del` method currently accepts only one argument and expects it to be a string so it's impossible to delete multiple keys at once.

This PR changes the signature of the `DiscourseRedis#del` method so it accepts any number of arguments and makes sure all keys are properly namespaced before calling the `del` implementation of the `redis` gem.
2022-02-11 08:09:32 +03:00
Daniel Waterworth 02245ce41f
PERF: Redis snapshotting during tests (#15260)
We can fake redis transactions so that `fab!` works for redis and PG
data, but it's too slow to be used indiscriminately. Instead, you can
opt into it with the `use_redis_snapshotting` helper.

Insofar as snapshotting allows us to `fab!` more things, it provides a
speedup.
2021-12-10 14:25:26 -06:00
Osama Sayegh d9d877fee7
DEV: Pass kwargs to the redis gem when calling methods/commands that we don't wrap (#14530)
This commit fixes the `eval` and `evalsha` commands/methods and any other methods that don't have a wrapper in `DiscourseRedis` and expect keyword arguments. I noticed this problem in Logster when I was trying to fetch some log messages in JSON format using the rails console and saw the messages were missing the `env` field. Logster uses the `eval` command to fetch messages `env`s:

dc351fd00f/lib/logster/redis_store.rb (L250-L253)

and that code was not fetching anything because `DiscourseRedis` didn't pass the `keys` keyword arg to the redis gem.
2021-10-06 17:42:04 +03:00
Yasuo Honda dbbfad7ed0 FIX: Support Ruby 3 keyword arguments 2021-10-05 11:25:00 -04:00
Yasuo Honda d06d09f479
Fix: Support Ruby 3 keyword arguments for DiscourseRedis (#14444) 2021-09-28 10:11:18 +08:00
Guo Xiang Tan 58e52c0e4f
DEV: Use rails_failover gem for ActiveRecord and Redis failover handling 2020-06-15 15:47:07 +08:00
Guo Xiang Tan 0ff86b00cb
DEV: Upgrade Redis to 4.2.1. 2020-06-15 10:05:22 +08:00
Guo Xiang Tan a3dfd553a1
Revert "Bump redis to 4.2.0."
This reverts commit 98bc28cea2.
2020-06-10 14:52:05 +08:00
Guo Xiang Tan 98bc28cea2
Bump redis to 4.2.0. 2020-06-10 14:28:56 +08:00
Guo Xiang Tan c35649ccf1
DEV: Remove Redis noop logging.
When Redis drops into readonly mode, it floods the log. The log isn't
actionable and we have other means of knowing that an app is in readonly
mode.
2020-06-09 16:45:34 +08:00
Guo Xiang Tan 439db7ca1e
DEV: Add `REDIS_RAILS_FAILOVER` env to test our new redis failover. 2020-06-02 17:24:14 +08:00
Guo Xiang Tan 389bdcf5ab
DEV: Fix implementation for `DiscourseRedis#exists`. 2020-06-01 13:11:32 +08:00
Guo Xiang Tan df62407f35
DEV: Implement multiple keys support for `DiscourseRedis#exists`. 2020-06-01 11:20:26 +08:00
Michael Brown d9a02d1336
Revert "Revert "Merge branch 'master' of https://github.com/discourse/discourse""
This reverts commit 20780a1eee.

* SECURITY: re-adds accidentally reverted commit:
  03d26cd6: ensure embed_url contains valid http(s) uri
* when the merge commit e62a85cf was reverted, git chose the 2660c2e2 parent to land on
  instead of the 03d26cd6 parent (which contains security fixes)
2020-05-23 00:56:13 -04:00
Jeff Atwood 20780a1eee Revert "Merge branch 'master' of https://github.com/discourse/discourse"
This reverts commit e62a85cf6f, reversing
changes made to 2660c2e21d.
2020-05-22 20:25:56 -07:00
Guo Xiang Tan 05beaa7f45
DEV: Remove override of `flushdb` in `DiscourseRedis`. 2020-05-19 10:26:23 +08:00
Sam Saffron d0d5a138c3
DEV: stop freezing frozen strings
We have the `# frozen_string_literal: true` comment on all our
files. This means all string literals are frozen. There is no need
to call #freeze on any literals.

For files with `# frozen_string_literal: true`

```
puts %w{a b}[0].frozen?
=> true

puts "hi".frozen?
=> true

puts "a #{1} b".frozen?
=> true

puts ("a " + "b").frozen?
=> false

puts (-("a " + "b")).frozen?
=> true
```

For more details see: https://samsaffron.com/archive/2018/02/16/reducing-string-duplication-in-ruby
2020-04-30 16:48:53 +10:00
Krzysztof Kotlarek 8cc09fc668
Revert "FIX: Redis fallback handler refactoring (#8771)" (#8776)
This reverts commit 4f677854d3.
2020-01-24 09:20:17 +11:00
Krzysztof Kotlarek 4f677854d3
FIX: Redis fallback handler refactoring (#8771)
* DEV: Add a fake Mutex that for concurrency testing with Fibers

* DEV: Support running in sleep order in concurrency tests

* FIX: A separate FallbackHandler should be used for each redis pair

This commit refactors the FallbackHandler and Connector:

 * There were two different ways to determine whether the redis master
   was up. There is now one way and it is the responsibility of the
   new RedisStatus class.

 * A background thread would be created whenever `verify_master` was
   called unless the thread already existed. The thread would
   periodically check the status of the redis master. However, checking
   that a thread is `alive?` is an ineffective way of determining
   whether it will continue to check the redis master in the future
   since the thread may be in the process of winding down.

   Now, this thread is created when the recorded master status goes from
   up to down. Since this thread runs the only part of the code that is
   able to bring the recorded status up again, we ensure that only one
   thread is probing the redis master at a time and that there is always
   a thread probing redis master when it is recorded as being down.

 * Each time the status of the redis master was checked periodically, it
   would spawn a new thread and immediately join on it. I assume this
   happened to isolate the check from the current execution, but since
   the join rethrows exceptions in the parent thread, this was not
   effective.

 * The logic for falling back was spread over the FallbackHandler and
   the Connector. The connector is now a dumb object that delegates
   responsibility for determining the status of redis to the
   FallbackHandler.

 * Previously, failing to connect to a master redis instance when it was
   not recorded as down would raise an exception. Now, this exception is
   passed to `Discourse.warn_exception` and the connection is made to
   the slave.

This commit introduces the FallbackHandlers singleton:

 * It is responsible for holding the set of FallbackHandlers.

 * It adds callbacks to the fallback handlers for when a redis master
   comes up or goes down. Main redis and message bus redis may exist on
   different or the same redis hosts and so these callbacks may all
   exist on the same FallbackHandler or on separate ones.

These objects are tested using fake concurrency provided by the
Concurrency module:

 * An `around(:each)` hook is used to cause each test to run inside a
   Scenario so that the test body, mocking cleanup and `after(:each)`
   callbacks are run in a different Fiber.

 * Therefore, holting the execution of the Execution abruptly (so that
   the fibers aren't run to completion), prevents the mocking cleaning
   and `after(:each)` callbacks from running. I have tried to prevent
   this by recovering from all exceptions during an Execution.

* FIX: Create frozen copies of passed in config where possible

* FIX: extract start_reset method and remove method used by tests

Co-authored-by: Daniel Waterworth <me@danielwaterworth.com>
2020-01-23 13:39:29 +11:00
Joffrey JAFFEUX 0d3d2c43a0
DEV: s/\$redis/Discourse\.redis (#8431)
This commit also adds a rubocop rule to prevent global variables.
2019-12-03 10:05:53 +01:00
Krzysztof Kotlarek 858cf5836c
FIX: update Redis gem to version 4.1.3
I run our benchmark on commit with hiredis and redis-4.1.3

Results:
type | hidredis | redis 4.1.3 | percent
--- | --- | --- | ---
Categories-50 | 49 | 50 | 102.04%
Categories-75 | 51 | 51 | 100.00%
Categories-90 | 63 | 64 | 101.59%
Categories-99 | 86 | 85 | 98.84%
Home-50 | 55 | 55 | 100.00%
Home-75 | 56 | 57 | 101.79%
Home-90 | 68 | 69 | 101.47%
Home-99 | 102 | 104 | 101.96%
Topic-50 | 36 | 37 | 102.78%
Topic-75 | 37 | 37 | 100.00%
Topic-90 | 47 | 48 | 102.13%
Topic-99 | 60 | 61 | 101.67%
Categories-admin-50 | 124 | 117 | 94.35%
Categories-admin-75 | 130 | 129 | 99.23%
Categories-admin-90 | 147 | 143 | 97.28%
Categories-admin-99 | 204 | 199 | 97.55%
Home-admin-50 | 146 | 148 | 101.37%
Home-admin-75 | 150 | 152 | 101.33%
Home-admin-90 | 169 | 168 | 99.41%
Home-admin-99 | 232 | 223 | 96.12%
Topic-admin-50 | 60 | 61 | 101.67%
Topic-admin-75 | 64 | 63 | 98.44%
Topic-admin-90 | 76 | 73 | 96.05%
Topic-admin-99 | 124 | 94 | 75.81%
Load rails | 2412 | 2360 | 97.84%
rss | 290204 | 295828 | 101.94%
pss | 277948 | 283624 | 102.04%

Redis gem is manipulating Redis config https://github.com/redis/redis-rb/blob/master/lib/redis/client.rb#L95
therefore we cannot pass the frozen config object.

Pass of the copy of the object is protecting original config
2019-10-21 09:59:24 +11:00
Krzysztof Kotlarek 427d54b2b0 DEV: Upgrading Discourse to Zeitwerk (#8098)
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains. 

We no longer need to use Rails "require_dependency" anywhere and instead can just use standard 
Ruby patterns to require files.

This is a far reaching change and we expect some followups here.
2019-10-02 14:01:53 +10:00
Daniel Waterworth 6924f1ab15 FIX: Race-condition in fallback handlers (#8005)
Calling `verify_master` in multiple threads simultaneously would cause
multiple threads to be spawned.
2019-08-21 15:47:44 +02:00
David Taylor afb5ec811d FIX: Don't use DistributedCache to store redis readonly state
This can cause unbound CPU usage in some cases, and excessive logging in other cases. This commit moves redis readonly information into the local process, but maintains the DistributedCache for postgres readonly state.
2019-06-25 11:20:34 +08:00
Sam Saffron 30990006a9 DEV: enable frozen string literal on all files
This reduces chances of errors where consumers of strings mutate inputs
and reduces memory usage of the app.

Test suite passes now, but there may be some stuff left, so we will run
a few sites on a branch prior to merging
2019-05-13 09:31:32 +08:00
Guo Xiang Tan 152238b4cf DEV: Prefer `public_send` over `send`. 2019-05-07 09:33:21 +08:00
Guo Xiang Tan 4dbebb4915 Follow up to f6f2c38183.
Master connection might fail.
2019-01-22 12:51:59 +08:00
Guo Xiang Tan f6f2c38183 FIX: Fallback Redis by checking status on master instead of slave. 2019-01-22 12:38:12 +08:00
Guo Xiang Tan 2cbb513c98 FIX: Don't use `Redis#keys` in production.
As per the documentation for KEYS

```
Warning: consider KEYS as a command that should only be used in production environments with extreme care. It may ruin performance when it is executed against large databases. This command is intended for debugging and special operations, such as changing your keyspace layout.
```

Instead SCAN

```
Since these commands allow for incremental iteration, returning only a small number of elements per call, they can be used in production without the downside of commands like KEYS or SMEMBERS that may block the server for a long time (even several seconds) when called against big collections of keys or elements.
```
2018-12-25 15:12:59 +02:00
Guo Xiang Tan 2adae2a0b6 FIX: Be more aggressive in checking master when trying to fallback. 2018-06-12 00:09:15 +08:00
Guo Xiang Tan 45fe5dc793 `$redis.client` -> `$redis._client`.
See c239abb43c
2018-04-20 13:01:17 +08:00
Sam 877b7be579 FIX: in readonly mode don't double count pages 2017-10-25 13:19:43 +11:00
Sam 61f32ed431 FIX: we can not namespace eval automatically, it is impossible 2017-10-11 16:41:26 +11:00
Guo Xiang Tan b2127600fb Remove use of concurrent timer for Redis failover.
* Uses the same logic for Postgres failover.
2017-10-05 15:57:08 +08:00
Sam aeedecd27c correct erratically failing spec 2017-08-18 15:10:37 -04:00
Guo Xiang Tan 9bc3038728 Fix Redis command errors when trying to start app with a readonly Redis. 2017-08-02 21:33:56 +09:00
Guo Xiang Tan 7d5b6e2b3c Disable MessageBus's keepalive when Redis is readonly. 2017-08-01 23:07:52 +09:00
Guo Xiang Tan 5012d46cbd Add rubocop to our build. (#5004) 2017-07-28 10:20:09 +09:00
Régis Hanol d792e81372 don't pollute test suite with warning message from readonly Redis 2017-07-24 10:22:32 +02:00
Guo Xiang Tan dd4cab3be2 FIX: Set master to true before forcing slave connections to reconnect. 2016-11-23 14:04:43 +08:00
Guo Xiang Tan 02025207d5 FIX: Make sure Redis fallback don't fall into a permanent readonly state. 2016-11-23 11:31:20 +08:00
Guo Xiang Tan fbbcde1230 FIX: Don't treat master as up if it is still loading data. 2016-11-07 15:28:10 +08:00
Guo Xiang Tan 9375dcb6fe PERF: Spawn a seperate timer task to check if Redis master is up. 2016-11-07 15:04:28 +08:00
Matt Palmer aa56100660 Failover messages really aren't warnings
"I'm going to do something entirely sane and reasonable" doesn't warrant a
warning-level log message.  It's perfectly fine and reasonable to just log
that sort of thing at info level.
2016-08-08 16:02:23 +10:00