Skip to content

Implement C-level ping? #74

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

Closed
mperham opened this issue Oct 14, 2021 · 11 comments
Closed

Implement C-level ping? #74

mperham opened this issue Oct 14, 2021 · 11 comments

Comments

@mperham
Copy link

mperham commented Oct 14, 2021

Hi, I've been trying to monitor Redis network latency within Sidekiq by using PING but I've learned that a process pegged at 100% CPU will dramatically overstate latency due to thread scheduling latency around the GVL. If you have 10 jobs crunching numbers, it may take 50-100ms to get a Ruby thread scheduled to process the PONG. Would you be interested in a special PING impl which is designed only to calculate round trip time in C, so as to avoid Ruby VM overhead?

I'm thinking something as simple as:

> redis.rtt_us
=> 267

where the result is the calculated RTT in µs.

See also sidekiq/sidekiq#5025

@nateberkopec
Copy link

it may take 50-100ms to get a Ruby thread scheduled to process the PONG

More than that, even. Threads are only interrupted every 100ms, so the worse case scenario is NUMBER_OF_THREADS * 100ms. Ouch!

@mperham
Copy link
Author

mperham commented Apr 26, 2023

@byroot is this something you would be interested in providing in hiredis-client?

@byroot
Copy link
Collaborator

byroot commented Apr 26, 2023

Hum, perhaps, I'd need to have a look at how doable it would be. As I'd need to skip the reader code because it needs the GVL.

I'll have a quick look tomorrow.

casperisfine pushed a commit to redis-rb/redis-client that referenced this issue Apr 27, 2023
Latency is returned as a Float in milliseconds.

For the Ruby driver, it's equivalent to measuring how long `call("PING")`
takes, however with the Hiredis driver, the latency is measured without
ever holding the GVL which allows to give a much more accurate measure
that isn't impacted by GVL contention.

Ref: redis/hiredis-rb#74

Test script:

```ruby
def fibonacci( n )
  return  n  if ( 0..1 ).include? n
  ( fibonacci( n - 1 ) + fibonacci( n - 2 ) )
end

require "redis-client"
if ENV["DRIVER"] == "hiredis"
  require "hiredis-client"
end

client = RedisClient.new

threads = 10.times.map do
  Thread.new do
    loop do
      fibonacci(30)
    end
  end
end

5.times do
  puts "latency: #{client.measure_latency}ms"
  sleep 1
end
```

```
$ DRIVER=ruby bundle exec ruby -I hiredis-client/lib/ /tmp/measure-latency.rb
latency: 1033.9850000143051ms
latency: 1039.7799999713898ms
latency: 1040.0930000543594ms
latency: 1050.2749999761581ms
latency: 1044.6280000209808ms
```

```
$ DRIVER=hiredis bundle exec ruby -I hiredis-client/lib/ /tmp/measure-latency.rb
latency: 0.307ms
latency: 0.351ms
latency: 0.236ms
latency: 0.221ms
latency: 0.321ms
```
casperisfine pushed a commit to redis-rb/redis-client that referenced this issue Apr 27, 2023
Latency is returned as a Float in milliseconds.

For the Ruby driver, it's equivalent to measuring how long `call("PING")`
takes, however with the Hiredis driver, the latency is measured without
ever holding the GVL which allows to give a much more accurate measure
that isn't impacted by GVL contention.

Ref: redis/hiredis-rb#74

Test script:

```ruby
def fibonacci( n )
  return  n  if ( 0..1 ).include? n
  ( fibonacci( n - 1 ) + fibonacci( n - 2 ) )
end

require "redis-client"
if ENV["DRIVER"] == "hiredis"
  require "hiredis-client"
end

client = RedisClient.new

threads = 10.times.map do
  Thread.new do
    loop do
      fibonacci(30)
    end
  end
end

5.times do
  puts "latency: #{client.measure_latency}ms"
  sleep 1
end
```

```
$ DRIVER=ruby bundle exec ruby -I hiredis-client/lib/ /tmp/measure-latency.rb
latency: 1033.9850000143051ms
latency: 1039.7799999713898ms
latency: 1040.0930000543594ms
latency: 1050.2749999761581ms
latency: 1044.6280000209808ms
```

```
$ DRIVER=hiredis bundle exec ruby -I hiredis-client/lib/ /tmp/measure-latency.rb
latency: 0.307ms
latency: 0.351ms
latency: 0.236ms
latency: 0.221ms
latency: 0.321ms
```
casperisfine pushed a commit to redis-rb/redis-client that referenced this issue May 2, 2023
Latency is returned as a Float in milliseconds.

For the Ruby driver, it's equivalent to measuring how long `call("PING")`
takes, however with the Hiredis driver, the latency is measured without
ever holding the GVL which allows to give a much more accurate measure
that isn't impacted by GVL contention.

Ref: redis/hiredis-rb#74

Test script:

```ruby
def fibonacci( n )
  return  n  if ( 0..1 ).include? n
  ( fibonacci( n - 1 ) + fibonacci( n - 2 ) )
end

require "redis-client"
if ENV["DRIVER"] == "hiredis"
  require "hiredis-client"
end

client = RedisClient.new

threads = 10.times.map do
  Thread.new do
    loop do
      fibonacci(30)
    end
  end
end

5.times do
  puts "latency: #{client.measure_round_trip_delay}ms"
  sleep 1
end
```

```
$ DRIVER=ruby bundle exec ruby -I hiredis-client/lib/ /tmp/measure-latency.rb
latency: 1033.9850000143051ms
latency: 1039.7799999713898ms
latency: 1040.0930000543594ms
latency: 1050.2749999761581ms
latency: 1044.6280000209808ms
```

```
$ DRIVER=hiredis bundle exec ruby -I hiredis-client/lib/ /tmp/measure-latency.rb
latency: 0.307ms
latency: 0.351ms
latency: 0.236ms
latency: 0.221ms
latency: 0.321ms
```
@byroot byroot closed this as completed Jan 23, 2025
@mperham
Copy link
Author

mperham commented Jan 23, 2025

If I want to use this number to distinguish between Redis latency issues and GVL latency issues, how can I do that? Right now I can't tell if the returned value is from Ruby (and includes GVL latency) or from C (and does not). Is there a way to capture both types of latency?

I guess I could do something like:

start = Time.now
rtt = redis.measure_round_trip_delay
stop = Time.now
gvl = (stop - start - rtt)

Any thoughts or ideas?

@byroot
Copy link
Collaborator

byroot commented Jan 23, 2025

I guess I could do something like:

Yes, that's one way. That is assuming hiredis-client is used. If you're using pure ruby redis-client, then that won't make any difference.

You can also alternatively look at the various gems that hook into the GVL instrumentation API: https://github.com/byroot/byroot.github.io/pull/8/files#diff-32ffcd1b6c378d7bdbf5debfa717cfff28a6a806d12857d951c0e250371800ecR64

@mperham
Copy link
Author

mperham commented Jan 23, 2025

Speedshop's middleware (and really all of those tools/gems) look really promising but suffer from the same problem as all advanced tools: the people that need it most don't know about it or understand it. I can document its usage with Sidekiq but 95% of people won't use it.

One of my dependency rules for Sidekiq is to never depend on native code/extensions. hiredis is fine because the user opts in but I can't directly pull in any of those native gems. I might need to wait for Ruby itself to offer better GVL visibility.

Does rediss: imply hiredis usage? If so, that means most people will be using hiredis in production and the logic above useable. I could limit the above logic only when using rediss:.

@byroot
Copy link
Collaborator

byroot commented Jan 23, 2025

Does rediss: imply hiredis usage?

No, rediss: implies SSL.

@mperham
Copy link
Author

mperham commented Jan 23, 2025

Ok, an OpenSSL socket. Got it. I presume this latency API will be in 0.23.3?

@byroot
Copy link
Collaborator

byroot commented Jan 23, 2025

I presume this latency API will be in 0.23.3?

It was released two years ago in 0.15.0

@mperham
Copy link
Author

mperham commented Jan 23, 2025

Oops, my client compatibility layer was hiding it. It's working great now.

@nateberkopec
Copy link

the people that need it most don't know about it or understand it. I can document its usage with Sidekiq but 95% of people won't use it.

The middleware is a stepping stone to an automatic adaptive concurrency tool. Our end goal is that the user will not need to configure anything.

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

No branches or pull requests

3 participants