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

Errno::ENOPROTOOPT when connecting to Redis with JRuby-9.2.14.0 #6541

Closed
ThomasKoppensteiner opened this issue Jan 27, 2021 · 21 comments
Closed

Comments

@ThomasKoppensteiner
Copy link

Connecting to Redis with tcp_keepalive option set raises an Errno::ENOPROTOOPT error in JRuby-9.2.14.0. It works in JRuby-9.2.13.0. I think this is because this PR removed SOL constants from RubyBasicSocket, but didn't add it to RubyTCPSocket (and others).

Example

#!/usr/bin/env ruby
# frozen_string_literal: true

require "redis"

class ConnectionFactory
  def self.build_connection_without_keepalive
    ::Redis.new(host: REDIS_HOST, port: REDIS_PORT, db: REDIS_DB)
  end

  def self.build_connection_with_keepalive
    ::Redis.new(host: REDIS_HOST, port: REDIS_PORT, db: REDIS_DB, tcp_keepalive: 60)
  end

  REDIS_HOST="127.0.0.1"
  REDIS_PORT=6379
  REDIS_DB=0

  private_constant :REDIS_HOST, :REDIS_PORT, :REDIS_DB
end

begin
  puts "Connection without TCP keepalive..."
  workaround_con = ConnectionFactory.build_connection_without_keepalive
  workaround_con.set("foo1", "bar1")
  val = workaround_con.get("foo1")
  puts "Value fetched: #{val}"
rescue StandardError => e
  puts e.class
  puts e.message
  puts e.backtrace
end

puts "---"

begin
  puts "Connection with TCP keepalive..."
  broken_con = ConnectionFactory.build_connection_with_keepalive
  broken_con.set("foo2", "bar2")
  val = broken_con.get("foo2")
  puts "Value fetched: #{val}"
rescue StandardError => e
  puts e.class
  puts e.message
  puts e.backtrace
end
JRuby-9.2.13.0 Output
Connection without TCP keepalive...
Value fetched: bar1
---
Connection with TCP keepalive...
Value fetched: bar2
JRuby-9.2.14.0 Output
Connection without TCP keepalive...
Value fetched: bar1
---
Connection with TCP keepalive...
Errno::ENOPROTOOPT
Protocol not available - Protocol not available
org/jruby/ext/socket/RubyBasicSocket.java:327:in `setsockopt'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/connection/ruby.rb:320:in `set_tcp_keepalive'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/connection/ruby.rb:310:in `connect'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:354:in `establish_connection'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:112:in `block in connect'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:313:in `with_reconnect'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:111:in `connect'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:386:in `ensure_connected'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:238:in `block in process'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:325:in `logging'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:237:in `process'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis/client.rb:131:in `call'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis.rb:848:in `block in set'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis.rb:69:in `block in synchronize'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/stdlib/monitor.rb:235:in `mon_synchronize'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis.rb:69:in `synchronize'
/Users/thk/.rbenv/versions/jruby-9.2.14.0/lib/ruby/gems/shared/gems/redis-4.2.5/lib/redis.rb:844:in `set'
./connect_to_redis.rb:39:in `<main>'

Environment Information

  • JRuby 9.2.13.0 vs. 9.2.14.0
  • OS: "Darwin ofc-02400-m 20.2.0 Darwin Kernel Version 20.2.0: Wed Dec 2 20:39:59 PST 2020; root:xnu-7195.60.75~1/RELEASE_X86_64 x86_64"
@ahorek
Copy link
Contributor

ahorek commented Jan 28, 2021

hey @ThomasKoppensteiner thanks for the report, based on this condition https://github.com/redis/redis-rb/blob/f7d354fd1f7069ccf5e7392d3bb83f48d9bf8545/lib/redis/connection/ruby.rb#L315 tcp_keepalive should be skipped on Darwin? So I expect you were testing on a Linux environment?

here's the full story
in 9.2.13.0 constants TCP_KEEPIDLE TCP_KEEPINTVL TCP_KEEPCNT weren't defined, so the previous condition in redis failed, which in fact disables tcp_keepalive support.
jnr/jnr-constants@f8ec3c6

in 9.2.14.0, after jnr-constants update, they're available, but setsockopt doesn't support them, which leads to the failure.

a simplified script

require 'socket'
@sock = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
@sock.setsockopt(Socket::SOL_TCP, Socket::TCP_KEEPIDLE, 1)

the good part is it works on master. I think that @headius 's work on TCP_CORK support also fixed this issue, but it wasn't backported back to 9.2.x
c657820 e41c99f ad84f97

@ThomasKoppensteiner
Copy link
Author

@ahorek thank you for reply.

based on this condition ... tcp_keepalive should be skipped on Darwin? So I expect you were testing on a Linux environment?

I don't see why this should be skipped on Darwin/macOS?
I ran the example on my development notebook (macOS). Redis was running inside a linux docker container.
I rerun the example with a native Redis installation => same result.
Our CI pipeline use linux containers for the services and for Redis.

here's the full story
in 9.2.13.0 constants TCP_KEEPIDLE TCP_KEEPINTVL TCP_KEEPCNT weren't defined, so the previous condition in redis failed, which in fact disables tcp_keepalive support.

True, with 9.2.13.0 the call ends up in the empty implementation, when the tcp_keepalive option is set.
This means, as a workaround, we can remove the options in our code, because it didn't do anything in the past.

the good part is it works on master. I think that @headius 's work on TCP_CORK support also fixed this issue, but it wasn't backported back to 9.2.x

Any plans to backport this to 9.2.x?

@headius headius changed the title "Errno::ENOPROTOOPT: Protocol not available - Protocol not available" when connection to Redis with JRuby-9.2.14.0 Errno::ENOPROTOOPT when connection to Redis with JRuby-9.2.14.0 Jan 29, 2021
@headius
Copy link
Member

headius commented Jan 29, 2021

Any plans to backport this to 9.2.x?

We have backported bits and pieces but I am not sure how much effort is required to backport everything needed for this issue.

I have merged #6542 which improves the situation for 9.3, and we hope to get that released within the next month.

Perhaps you could test out a snapshot build of 9.3 and see if it works correctly for you? It would help us confirm that we at least have the right fixes in place.

@ahorek
Copy link
Contributor

ahorek commented Jan 29, 2021

tcp_keepalive is optional, right? Note that in previous versions there was an empty implementation, so it wasn't effective anyway.

@ThomasKoppensteiner it would be great if you could test it out on 9.3.

@ThomasKoppensteiner
Copy link
Author

@ahorek yes the tcp_keepalive setting is optional. I removed it for now, so that the Redis connection setup works (for us) with JRuby-9.2.14.0.

@ThomasKoppensteiner
Copy link
Author

I tested the JRuby-9.3.0.0-SNAPSHOT from here with the above scripts and it works. The Errno::ENOPROTOOPT error is gone.

~/Downloads/jruby-head/bin/ruby --version && ~/Downloads/jruby-head/bin/ruby ./connect_to_redis.rb

jruby 9.3.0.0-SNAPSHOT (2.6.5) 2021-01-29 7b9f596d4c Java HotSpot(TM) 64-Bit Server VM 25.192-b12 on 1.8.0_192-b12 [darwin-x86_64]

Connection without TCP keepalive...
Value fetched: bar1
---
Connection with TCP keepalive...
Value fetched: bar2

@ThomasKoppensteiner
Copy link
Author

ThomasKoppensteiner commented Feb 1, 2021

I get different errors for the "simplified script" from above for each JRuby version. I'm not sure which ones are expected.

require 'socket'
@sock = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
@sock.setsockopt(Socket::SOL_TCP, Socket::TCP_KEEPIDLE, 1)

9.2.13.0

ruby --version && ruby ./socket_test.rb

jruby 9.2.13.0 (2.5.7) 2020-08-03 9a89c94bcc Java HotSpot(TM) 64-Bit Server VM 25.192-b12 on 1.8.0_192-b12 [darwin-x86_64]

NameError: uninitialized constant Socket::TCP_KEEPIDLE
Did you mean?  Socket::TCP_KEEPALIVE
  const_missing at org/jruby/RubyModule.java:3760
         <main> at ./socket_test.rb:6

9.2.14.0

ruby --version && ruby ./socket_test.rb

jruby 9.2.14.0 (2.5.7) 2020-12-08 ebe64bafb9 Java HotSpot(TM) 64-Bit Server VM 25.192-b12 on 1.8.0_192-b12 [darwin-x86_64]

Errno::ENOPROTOOPT: Protocol not available - Protocol not available
  setsockopt at org/jruby/ext/socket/RubyBasicSocket.java:327
      <main> at ./socket_test.rb:6

9.3.0.0-SNAPSHOT

~/Downloads/jruby-head/bin/ruby --version && ~/Downloads/jruby-head/bin/ruby ./socket_test.rb

jruby 9.3.0.0-SNAPSHOT (2.6.5) 2021-01-29 7b9f596d4c Java HotSpot(TM) 64-Bit Server VM 25.192-b12 on 1.8.0_192-b12 [darwin-x86_64]

NameError: uninitialized constant Socket::SOL_TCP
Did you mean?  Socket::SO_TYPE
  const_missing at org/jruby/RubyModule.java:3899
         <main> at ./socket_test.rb:6

@ahorek
Copy link
Contributor

ahorek commented Feb 1, 2021

Socket::SOL_TCP is Linux only, it is expected to fail on Darwin this way. Try it on a Docker / Linux environment.

@headius
Copy link
Member

headius commented Feb 1, 2021

FYI we are debating the level of effort to get the 9.3 socket improvements backported to 9.2, since we will continue to maintain 9.2 until a substantial plurality of users have migrated to 9.3. Worst case we could just copy all the socket code from master to jruby-9.2 but ideally we would like to maintain commit history.

@ThomasKoppensteiner ThomasKoppensteiner changed the title Errno::ENOPROTOOPT when connection to Redis with JRuby-9.2.14.0 Errno::ENOPROTOOPT when connecting to Redis with JRuby-9.2.14.0 Feb 25, 2021
@aboltart
Copy link

aboltart commented Apr 7, 2021

Any plans regarding #6541 (comment) ?

@acrewdson
Copy link

Any plans regarding #6541 (comment) ?

Just to amplify this: backporting the relevant fixes to 9.2 would be hugely appreciated 🙏

@headius
Copy link
Member

headius commented May 4, 2021

At the moment there is no 9.2.18 planned but I will put together a PR that basically just copies the socket code from master to 9.2.

headius added a commit to headius/jruby that referenced this issue May 4, 2021
This is a wholesale duplication of the 9.3 (master at time of
writing) socket implementation into 9.2 to pick up all recent
improvements in the socket library.

This should address jruby#6541 and other socket issues fixed on master.
@headius
Copy link
Member

headius commented May 4, 2021

I have pushed #6664 to address this. I did not bother trying to maintain commit history. All future changes to socket should happen on master (at least until 9.3 branch has been created and we abandon 9.2 updates).

@headius headius added this to the JRuby 9.2.18.0 milestone May 4, 2021
@acrewdson
Copy link

@headius huge thanks for that PR!

At the moment there is no 9.2.18 planned

Just to understand that a bit better, does it foreclose the possibility of a 9.2.18.0, or does it just mean the release date isn't scheduled at this point?

@headius
Copy link
Member

headius commented May 4, 2021

@acrewdson It just means we have not scheduled it and are trying to get 9.3 out the door. We will continue to maintain 9.2 (including releasing 9.2.18) for at least the six months following the 9.3 release.

@acrewdson
Copy link

@headius got it – thank you!

@headius
Copy link
Member

headius commented May 17, 2021

The socket updates have been backported and JRuby 9.2 should largely match 9.3/master functionality now.

$ jruby redis_test.rb 
jruby 9.2.18.0-SNAPSHOT (2.5.8) 2021-05-17 8e66a60eb3 OpenJDK 64-Bit Server VM 11.0.4+11 on 11.0.4+11 +jit [darwin-x86_64]
Connection without TCP keepalive...
Value fetched: bar1
---
Connection with TCP keepalive...
Value fetched: bar2

@headius headius closed this as completed May 17, 2021
@acrewdson
Copy link

@headius huge thanks 🙇

@aboltart
Copy link

aboltart commented Jun 3, 2021

@headius any plans/timeline when JRuby 9.2.18.0 could be released?

@headius
Copy link
Member

headius commented Jun 3, 2021

@aboltart We are trying to finish up 9.3 to get it out the door, so we are not planning a 9.2.18.0 right now. We will do a 9.2.18.0 once 9.3 is out, or if it looks like 9.3 will be delayed further.

Anything you can do to help us validate and close out 9.3 issues will make this process go faster! 😀

@headius
Copy link
Member

headius commented Jun 3, 2021

@aboltart Actually we just had a chat about this and given some upcoming delays due to summer vacations we will do a 9.2.18.0 very soon.

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

5 participants