backport a JRuby 9.1.16.0 stdlib resolv.rb patch on earlier versions

[colinsurprenant]

Fixes #40

The installed JRuby version 9.1.13.0 has a bug in the stdlib resolv.rb that has been fixed in 9.1.16.0

This bug result in the resolver hanging after 64k unique IP resolutions.

This patch is also submitted in core elastic/logstash/pull/9750. I do not see a problem having the patch in the 2 places, this one here will apply on any LS version and eventually if a patched LS version is used it wont cause problem since the patching is guarded using a version check.

I testing this locally, see #40.

[jsvd]

It'd be cool to document where this code comes from: https://github.com/jruby/jruby/blob/9.1.16.0/lib/ruby/stdlib/resolv.rb#L775

[jsvd]

ruby's most common way to compare versions is through Gem::Version, we could rewrite:

version = JRUBY_VERSION.split(".").map(&:to_i)

if version[0] == 9 && version[1] == 1 && version[2] < 16

as:

if Gem::Version.new("9.1.16.0") > Gem::Version.new(JRUBY_VERSION)

or

RESOLV_PATCH_CONSTRAINT = Gem::Requirement.new("< 9.1.16.0")

if RESOLV_PATCH_CONSTRAINT.satisfied_by?(Gem::Version.new(JRUBY_VERSION))

[webmat]

Does this work on JRuby 1.7 as well? Have you checked if there has been much changes to resolv.rb since then?

I'm just wondering if the exact same patch will work for Logstash 5.x / JRuby 1.7.

[colinsurprenant]

I stress tested the 1.7.25 and 1.7.27 (the 2 JRuby vesions used across the LS 5 versions) resolver and they are not affected by this bug. Only 9k need patching and 9.1.13.0 is the first 9k version we used.

[jsvd]

for JRuby versions prior..

[colinsurprenant]

? "Fixed JRuby ... for versions prior" ... not sure how this is confusing?

[jsvd]

you're right, I was nitpicking

[webmat]

I agree with both of @jsvd's requests for changes. If they get added and you confirm that the patch works on 5.x as well, this LGTM

[colinsurprenant]

@jsvd thanks for the suggestions, pushed modification. LMK.

[colinsurprenant]

@webmat I am recopying my code comment about this:

I stress tested the 1.7.25 and 1.7.27 (the 2 JRuby vesions used across the LS 5 versions) resolver and they are not affected by this bug. Only 9k need patching and 9.1.13.0 is the first 9k version we used.

[jsvd]

I stress tested the 1.7.25 and 1.7.27 (the 2 JRuby vesions used across the LS 5 versions) resolver and they are not affected by this bug. Only 9k need patching and 9.1.13.0 is the first 9k version we used.

This means we should also constrain the application of the patch to something like ['~> 9', '< 9.1.16.0'], otherwise installing the plugin on a logstash with 1.7.x will still apply the patch and potentially break things? (or do we know it's a no-op in jruby 1.7.x?)

[colinsurprenant]

@jsvd ah yes - totally - my previous check was indeed valid for hard checking on version 9 but my refactor using Gem omits that will change.

[jsvd]

LGTM

[colinsurprenant]

Thanks @jsvd @webmat will publish shortly.

[colinsurprenant]

logstash-filter-dns v 3.0.11 published.