stefanwille / crystal-redis

Full featured Redis client for Crystal
MIT License
380 stars 61 forks source link

Redis tanks when under load #76

Closed jwoertink closed 5 years ago

jwoertink commented 5 years ago

I have a fairly simple call I run when a page loads. If I use the app with just me, everything works great. When I try to benchmark by putting it under a little load, I start getting all kinds of redis errors.

We are in a multi block - call methods on the multi block argument instead of the Redis object (Redis::Error)

ERR MULTI calls can not be nested (Redis::Error)

cast from String to Int64 failed, at /Users/jeremywoertink/Sites/tko/lib/redis/src/redis/command_execution/value_oriented.cr:13:9:13 (TypeCastError)

Here's the code that's run on page load:

  def cycle_images
    images = Redis::Future.new
    redis_client.multi do |mult|
      mult.rpoplpush(key("images"), key("images"))
      images = mult.lrange(key("images"), 0, -1)
    end
    images.value.as(Array(Redis::RedisValue))
  end

Here's the benchmark using wrk

wrk -c 10 -t 2 http://test.lvh.me:5000/

This happens both locally, and in production using openredis small instance (small (200 MB, 256 connections))

I'm running this on macOS 10.14.1 with crystal 0.27, crystal-redis 2.1.1.

kostya commented 5 years ago

what is your redis_client? you should use PooledClient for concurrency

jwoertink commented 5 years ago

@kostya I'm not using a redis client, just the redis server. Locally it's version 3.2.8, but in production it's 4.0.9.

What's PooledClient?

kostya commented 5 years ago

you are using this shard which is a redis client. I am about variable redis_client, what is it? looks like your app use redis client concurrently, so you should use https://github.com/stefanwille/crystal-redis#connection-pooling

jwoertink commented 5 years ago

oh, I see what you mean.

  def redis_client
    Redis.new(url: ENV["REDIS_URL"]?)
  end

Ok, cool. I'll check out that pooling and see if that helps. Thanks!

kostya commented 5 years ago

you create client for every request? this expect to be slow. client for redis created once per app usually.

jwoertink commented 5 years ago

No, it's actually on the class level, and there's some memoization happening there.

jwoertink commented 5 years ago

oh nice! Looks like moving that over to the Redis::PooledClient.new(url: ENV["REDIS_URL"]?) works! I went from 3req/sec to 1400req/sec 😂

I'll go ahead and just close this for now

saturn-network commented 5 years ago

From out experience, interaction of crystal-redis and amber framework has memory leaks and crashes heroku dynos after prolonged usage (they run out of RAM). Tried with both a pool of size 5 and a pool of size 1.

kostya commented 5 years ago

are you sure that it is redis client problem? i use redis with pools in highload app, no memory problems at all.

zaidakram commented 4 years ago

Facing the similar situation, Using sidekiq.cr I get

WARN: Redis::Error: ERR MULTI calls can not be nested

under heavy load. And Sidekiq workers start loosing jobs. It gives same warning on enqueuing a new job. Does anybody have a clue why this is happening. Thanks