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

Update to JRuby 9.4 #14861

Merged
merged 28 commits into from Jun 28, 2023
Merged

Update to JRuby 9.4 #14861

merged 28 commits into from Jun 28, 2023

Conversation

andsel
Copy link
Contributor

@andsel andsel commented Jan 30, 2023

Release notes

What does this PR do?

  • Redefine space token in LSCL and grammar treetop from _ which would generated methods in the form def _0 (deprecated since 2.7) to sc.
  • I18n.t method doesn't accept hash as second argument
  • URI.encode -> URI.encode_www_form_component
    • URI.encode has been replaced with same functionality with URI::Parser.new.escape
  • YAML.load needs explicit fallback: false to return false when the yaml string is empty (or contains only comments)
  • JRuby's JavaClass has been removed, now it can use java.lang.Class directly
  • explicitly require gem thwait to satisfy require "thwait" (In Gemfile.template and logstash-core/logstash-core.gemspec)
  • fix not args clone to be def clone(*args)
  • fix Enumeration.each_slice which from Ruby 3.1 is chainable and doesn't return nil. JRuby fixed in Ruby 3.1 support jruby/jruby#7015
  • Expanded Down.download arguments map ca16bbe
  • Avoid to pass nil in the list of couples used in Hash[ <list of couples> ] which from Ruby 3.0 generates an ArgumentError
  • Removed space not allowed between method name and parentheses initialize ( is forbidden. 29b607d
  • With Ruby 2.7 the Kernel#open doesn't fallback to URI#open, fixed test code that used that to verify open port. e5b70de
  • Avoid to drop rdoc/ folder from vendored JRuby else bin/logstash -i irb would crash, commit b71f73e
LoadError: no such file to load -- rdoc/version

Why is it important/What is the impact to the user?

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@andsel andsel self-assigned this Jan 30, 2023
@andsel andsel changed the title Fix/updates to jruby 9 4 Update to JRuby 9.4 Jan 30, 2023
@andsel
Copy link
Contributor Author

andsel commented Apr 17, 2023

Jenkins test this please

logstash-core/lib/logstash/api/modules/logging.rb Outdated Show resolved Hide resolved
rubyUtils.gradle Outdated Show resolved Hide resolved
rubyUtils.gradle Outdated
def gemDir = "${projectDir}/vendor/bundle/jruby/2.6.0".toString()
jruby.setLoadPaths(["${projectDir}/vendor/bundle/jruby/2.6.0/gems/bundler-2.4.13/lib".toString(), "${projectDir}/vendor/jruby/lib/ruby/stdlib".toString()])
def gemDir = "${projectDir}/vendor/bundle/jruby/3.1.0".toString()
jruby.setLoadPaths(["${projectDir}/vendor/bundle/jruby/3.1.0/gems/bundler-2.4.13/lib".toString(), "${projectDir}/vendor/jruby/lib/ruby/stdlib".toString()])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either we apply the same change as in #15066, or need to update the path to be bundler-2.4.14, released 3 days ago https://rubygems.org/gems/bundler/versions/2.4.14

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think should use the Bundler shipped with JRuby and avoid bootstrapping one on our own. So, in this perspective, it should include also the changes proposed with #15066. I can rebase this after the other has been merged into main

@@ -67,8 +67,8 @@
it "install the gems" do
expect(::Bundler::LogstashInjector).to receive(:inject!).with(be_kind_of(LogStash::PluginManager::PackInstaller::Pack)).and_return([])

expect(::LogStash::PluginManager::GemInstaller).to receive(:install).with(/logstash-input-packtest/, anything)
expect(::LogStash::PluginManager::GemInstaller).to receive(:install).with(/logstash-input-packtestdep/, anything)
expect(::LogStash::PluginManager::GemInstaller).to receive(:install).with(/logstash-input-packtest-/, anything)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit that introduces the changes is 0c92eb3.

Without this change, the test fail:

SPEC_OPTS="-fd -P spec/unit/plugin_manager/pack_installer/local_spec.rb" ./gradlew clean :logstash-core:rubyTests --tests org.logstash.RSpecTests

with

    Failures:

      1) LogStash::PluginManager::PackInstaller::Local when the local file exist when the pack is valid install the gems
         Failure/Error: expect(::LogStash::PluginManager::GemInstaller).to receive(:install).with(/logstash-input-packtest/, anything)

           (LogStash::PluginManager::GemInstaller (class)).install(/logstash-input-packtest/, anything)
               expected: 1 time with arguments: (/logstash-input-packtest/, anything)
               received: 2 times with arguments: (/logstash-input-packtest/, anything)
         # ./spec/unit/plugin_manager/pack_installer/local_spec.rb:70:in `block in <main>'
         # ./lib/bootstrap/rspec.rb:36:in `<main>'

    Finished in 0.04501 seconds (files took 0.02848 seconds to load)
    5 examples, 1 failure

    Failed examples:

    rspec ./spec/unit/plugin_manager/pack_installer/local_spec.rb:67 # LogStash::PluginManager::PackInstaller::Local when the local file exist when the pack is valid install the gems


org.logstash.RSpecTests > rspecTests[core tests] FAILED
    java.lang.AssertionError: RSpec test suite `core tests` saw at least one failure.
        at org.junit.Assert.fail(Assert.java:88)
        at org.logstash.RSpecTests.rspecTests(RSpecTests.java:72)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the reg exp /logstash-input-packtest/ is matched 2 times:

  • one with logstash-input-packtest...
  • another with logstash-input-packtestdep...

I wasn't able to understand why previously it worked and now it receives 2 matches. Also previously it had been to fail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really strange that it worked before and now doesn't. main and this PR are using the same version of rspec, but bundler and jruby are different.

@andsel andsel requested a review from jsvd June 15, 2023 13:16
@jsvd
Copy link
Member

jsvd commented Jun 15, 2023

check about the problem with lib/jni and glibc, PR to fix in LS is #14890. Verify if port to this or remove the original effort if it's resolved in 9.4.x in the meantime.

We can remove the workaround:

[root@a03afd0551ce ~]# cat /etc/redhat-release
Red Hat Enterprise Linux Server release 7.9 (Maipo)
[root@a03afd0551ce ~]# uname -a
Linux a03afd0551ce 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux[root@a03afd0551ce ~]# ldd jruby-9.4.2.0/lib/jni/x86_64-Linux/libjffi-1.2.so
ldd: warning: you do not have execution permission for `jruby-9.4.2.0/lib/jni/x86_64-Linux/libjffi-1.2.so'
jruby-9.4.2.0/lib/jni/x86_64-Linux/libjffi-1.2.so: /lib64/libc.so.6: version `GLIBC_2.27' not found (required by jruby-9.4.2.0/lib/jni/x86_64-Linux/libjffi-1.2.so)
	libc.so.6 => /lib64/libc.so.6 (0x00007fffff9eb000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fffffddc000)
[root@a03afd0551ce ~]# ldd jruby-9.4.3.0/lib/jni/x86_64-Linux/libjffi-1.2.so
ldd: warning: you do not have execution permission for `jruby-9.4.3.0/lib/jni/x86_64-Linux/libjffi-1.2.so'
	libc.so.6 => /lib64/libc.so.6 (0x00007fffff7ec000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fffffddc000)

@jsvd
Copy link
Member

jsvd commented Jun 15, 2023

Then again, maybe not, still broken on aarch64:

❯ docker run -it oraclelinux:7 bash
[root@5db95de36c6b /]# uname -a
Linux 5db95de36c6b 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
[root@5db95de36c6b /]# curl -s https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.2.21.0/jruby-dist-9.2.21.0-bin.tar.gz | tar -zxf -
[root@5db95de36c6b /]# curl -s https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.3.9.0/jruby-dist-9.3.9.0-bin.tar.gz | tar -zxf -
[root@5db95de36c6b /]# curl -s https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.4.3.0/jruby-dist-9.4.3.0-bin.tar.gz | tar -zxf -
[root@5db95de36c6b /]# ldd jruby-9.2.21.0/lib/jni/aarch64-Linux/libjffi-1.2.so
ldd: warning: you do not have execution permission for `jruby-9.2.21.0/lib/jni/aarch64-Linux/libjffi-1.2.so'
	linux-vdso.so.1 =>  (0x0000ffff89de4000)
	libc.so.6 => /lib64/libc.so.6 (0x0000ffff89c1e000)
	/lib/ld-linux-aarch64.so.1 (0x0000ffff89db6000)
[root@5db95de36c6b /]# ldd jruby-9.3.9.0/lib/jni/aarch64-Linux/libjffi-1.2.so
ldd: warning: you do not have execution permission for `jruby-9.3.9.0/lib/jni/aarch64-Linux/libjffi-1.2.so'
	linux-vdso.so.1 =>  (0x0000ffff9bd3f000)
	libc.so.6 => /lib64/libc.so.6 (0x0000ffff9bb79000)
	/lib/ld-linux-aarch64.so.1 (0x0000ffff9bd11000)
[root@5db95de36c6b /]# ldd jruby-9.4.3.0/lib/jni/aarch64-Linux/libjffi-1.2.so
ldd: warning: you do not have execution permission for `jruby-9.4.3.0/lib/jni/aarch64-Linux/libjffi-1.2.so'
jruby-9.4.3.0/lib/jni/aarch64-Linux/libjffi-1.2.so: /lib64/libc.so.6: version `GLIBC_2.27' not found (required by jruby-9.4.3.0/lib/jni/aarch64-Linux/libjffi-1.2.so)
	linux-vdso.so.1 =>  (0x0000ffffae948000)
	libc.so.6 => /lib64/libc.so.6 (0x0000ffffae763000)
	/lib/ld-linux-aarch64.so.1 (0x0000ffffae91a000)

Which lines up correctly to the comment here: jruby/jruby#7579 (comment)
Since we support aarch64 in all Operating Systems we support x64 in, we need to keep the patch, but only for aarch64.

@andsel
Copy link
Contributor Author

andsel commented Jun 20, 2023

The 🔴 CI errors, once launched locally with

ci/docker_integration_tests.sh split 0

fails to start Logstash:

      using logstash.yml and separate config file
        behaves like it can send 1000 documents to and index from the dlq

org.logstash.integration.RSpecTests > rspecTests STANDARD_ERROR
    /opt/logstash/qa/integration/framework/fixture.rb:59: warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
    /opt/logstash/qa/integration/framework/fixture.rb:59: warning: Passing trim_mode with the 3rd argument of ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.

org.logstash.integration.RSpecTests > rspecTests STANDARD_OUT
    Starting Logstash: /opt/logstash/build/logstash-8.9.0-SNAPSHOT/bin/logstash ["-f", "/tmp/studtmp-153c90c79eb67c09d0029b9bfd8475923a7762f1960d4ba85e1932b9c78e", "--path.settings", "/tmp/studtmp-ac45a5f8ad2ee6d79594c605d7ec24c3b025edf830e4ad1d16fdbcfa2d80"] (pwd: /opt/logstash/qa/integration)
    Logstash started with PID 3408, using java: bundled java
    Using bundled JDK: /opt/logstash/build/logstash-8.9.0-SNAPSHOT/jdk
    [FATAL] 2023-06-20 18:25:45.455 [main] Logstash - Logstash was unable to start due to an unexpected Gemfile change.
    If you are a user, this is a bug.
    If you are a logstash developer, please try restarting logstash with the `--enable-local-plugin-development` flag set.

Also launching locally fails with same error, and happens that because the gradle build

./gradlew clean installDefaultGems

Fails with

ERROR: Installation Aborted, message: uninitialized constant Bundler::SolveFailure

that should be fixed by #15066

@roaksoax roaksoax linked an issue Jun 22, 2023 that may be closed by this pull request
Jruby: 9.3.40 -> 9.4.0.0
Ruby: 2.6 -> 3.1
With Ruby 2.7 was deprecated the usage of `_1` (`_<n>`) identifier for future reseved usage.
With Ruby 3.0 the new synthax for "numbered parameters" was used, avoiding the usage of `_n` as method names.

Treetop's generated grammar contained `def _n` methods, mainly implied to the usage of `_` symbol in LSCL grammar.

This commit rename the `_` rule in the grammar to `cs` and replaces everywhere it's needed.

Explanation at cjheath/treetop#49
rubyUtils.gradle Outdated Show resolved Hide resolved
andsel and others added 3 commits June 28, 2023 11:07
Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
Fixes the warning:
warning: nested repeat operator '?' and '*' was replaced with '*' in regular expression /\s?*,\s*/

happening at:
logstash-core/lib/logstash/instrument/metric_store.rb
The two warnings are:
- lib/concurrent-ruby/concurrent/executor/java_thread_pool_executor.rb:13: warning: method redefined; discarding old to_f
- lib/concurrent-ruby/concurrent/executor/java_thread_pool_executor.rb:13: warning: method redefined; discarding old to_int
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andsel andsel merged commit 26d1c7c into elastic:main Jun 28, 2023
3 of 4 checks passed
andsel added a commit that referenced this pull request Oct 25, 2023
…rohibited format (#15500)

Updates invocations of i18n.t method which are leftovers and missed in the original Ruby 3.1 update PR #14861

Without this, some error reporting logs are hidden by the mismatch of arguments error in translate the error message.
github-actions bot pushed a commit that referenced this pull request Oct 25, 2023
…rohibited format (#15500)

Updates invocations of i18n.t method which are leftovers and missed in the original Ruby 3.1 update PR #14861

Without this, some error reporting logs are hidden by the mismatch of arguments error in translate the error message.

(cherry picked from commit 90964fb)
andsel added a commit that referenced this pull request Oct 25, 2023
…rohibited format (#15500) (#15501)

Updates invocations of i18n.t method which are leftovers and missed in the original Ruby 3.1 update PR #14861

Without this, some error reporting logs are hidden by the mismatch of arguments error in translate the error message.

(cherry picked from commit 90964fb)

Co-authored-by: Andrea Selva <selva.andre@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch Logstash to JRuby 9.4
3 participants