Cleanup gradle tasks and dependency installation (#10942)

- have `bootstrap` task do as little as possible: install gems in Gemfile.template that don't belong to groups - have test tasks depend on the `installTestGems` task instead of `bootstrap` - logstash es output is now a dependency because of license checking - fix out of memory problem in SharedHelpers.trap Also use release lockfile during installDefaultGems The release lockfile is only copied to Gemfile.lock if it doesn't exist. During the `installDefaultGems` task other plugin installation tasks already occurred, generating a lock file. This commit removes it before running the plugin installation.
elastic · Jul 12, 2019 · 15fb308 · 15fb308
1 parent 0c85857
commit 15fb308
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 45 deletions.
diff --git a/Gemfile.template b/Gemfile.template
@@ -11,6 +11,7 @@ gem "paquet", "~> 0.2"
 gem "pleaserun", "~>0.0.28"
 gem "rake", "~> 12"
 gem "ruby-progressbar", "~> 1"
+gem "logstash-output-elasticsearch"
 gem "childprocess", "~> 0.9", :group => :build
 gem "fpm", "~> 1.3.3", :group => :build
 gem "gems", "~> 1", :group => :build

diff --git a/build.gradle b/build.gradle
@@ -119,46 +119,24 @@ clean {
   delete "${projectDir}/build/rubyDependencies.csv"
 }
 
-task bootstrap {}
-
-project(":logstash-core") {
-  ["rubyTests", "test"].each { tsk ->
-    tasks.getByPath(":logstash-core:" + tsk).configure {
-      dependsOn bootstrap
-    }
-  }
-}
-
-task installDefaultGems(dependsOn: downloadAndInstallJRuby) {
-  inputs.files file("${projectDir}/Gemfile.template")
-  inputs.files fileTree("${projectDir}/rakelib")
-  inputs.files file("${projectDir}/versions.yml")
-  outputs.file("${projectDir}/Gemfile")
-  outputs.file("${projectDir}/Gemfile.lock")
-  outputs.dir("${projectDir}/logstash-core/lib/jars")
-  outputs.dir("${projectDir}/vendor/bundle/jruby/2.5.0")
+task bootstrap {
   doLast {
-      gem(projectDir, buildDir, "rake", "12.3.1", "${projectDir}/vendor/bundle/jruby/2.5.0")
-      gem(projectDir, buildDir, "json", "1.8.6", "${projectDir}/vendor/bundle/jruby/2.5.0")
-      rake(projectDir, buildDir, 'plugin:install-default')
+      rake(projectDir, buildDir, 'plugin:install-base')
   }
 }
 
 def assemblyDeps = [downloadAndInstallJRuby, assemble] + subprojects.collect {
   it.tasks.findByName("assemble")
 }
 
+task installDefaultGems(dependsOn: assemblyDeps) {
+  doLast {
+      rake(projectDir, buildDir, 'plugin:install-default')
+  }
+}
+
 task installTestGems(dependsOn: assemblyDeps) {
-  inputs.files file("${projectDir}/Gemfile.template")
-  inputs.files fileTree("${projectDir}/rakelib")
-  inputs.files file("${projectDir}/versions.yml")
-  outputs.file("${projectDir}/Gemfile")
-  outputs.file("${projectDir}/Gemfile.lock")
-  outputs.dir("${projectDir}/logstash-core/lib/jars")
-  outputs.dir("${projectDir}/vendor/bundle/jruby/2.5.0")
   doLast {
-      gem(projectDir, buildDir, "rake", "12.3.1", "${projectDir}/vendor/bundle/jruby/2.5.0")
-      gem(projectDir, buildDir, "json", "1.8.6", "${projectDir}/vendor/bundle/jruby/2.5.0")
       rake(projectDir, buildDir, 'plugin:install-development-dependencies')
   }
 }
@@ -175,7 +153,6 @@ task assembleTarDistribution(dependsOn: assemblyDeps) {
   inputs.files fileTree("${projectDir}/x-pack")
   outputs.files file("${buildDir}/logstash-${project.version}-SNAPSHOT.tar.gz")
   doLast {
-      gem(projectDir, buildDir, "rake", "12.3.1", "${projectDir}/vendor/bundle/jruby/2.5.0")
       rake(projectDir, buildDir, 'artifact:tar')
   }
 }
@@ -226,6 +203,14 @@ task assembleOssZipDistribution(dependsOn: assemblyDeps) {
   }
 }
 
+project(":logstash-core") {
+  ["rubyTests", "test"].each { tsk ->
+    tasks.getByPath(":logstash-core:" + tsk).configure {
+      dependsOn installTestGems
+    }
+  }
+}
+
 def logstashBuildDir = "${buildDir}/logstash-${project.version}-SNAPSHOT"
 
 task unpackTarDistribution(dependsOn: assembleTarDistribution, type: Copy) {
@@ -318,13 +303,13 @@ task generateLicenseReportInputs() {
     }
 }
 
-task generatePluginsVersion(dependsOn: bootstrap) {
+task generatePluginsVersion(dependsOn: installDefaultGems) {
   doLast {
       rake(projectDir, buildDir, 'generate_plugins_version')
   }
 }
 
-bootstrap.dependsOn installTestGems
+bootstrap.dependsOn assemblyDeps
 
 runIntegrationTests.shouldRunAfter tasks.getByPath(":logstash-core:test")
 check.dependsOn runIntegrationTests
@@ -421,7 +406,7 @@ if (!oss) {
   project(":logstash-xpack") {
     ["rubyTests", "rubyIntegrationTests", "test"].each { tsk ->
       tasks.getByPath(":logstash-xpack:" + tsk).configure {
-        dependsOn bootstrap
+        dependsOn installTestGems
       }
     }
     tasks.getByPath(":logstash-xpack:rubyIntegrationTests").configure {

diff --git a/rakelib/plugin.rake b/rakelib/plugin.rake
@@ -8,6 +8,18 @@ namespace "plugin" do
     LogStash::PluginManager::Main.run("bin/logstash-plugin", ["install"] + args)
   end
 
+  task "install-base" => "bootstrap" do
+    puts("[plugin:install-base] Installing base dependencies")
+    install_plugins("--development",  "--preserve")
+    task.reenable # Allow this task to be run again
+  end
+
+  def remove_lockfile
+    if ::File.exist?(LogStash::Environment::LOCKFILE)
+      ::File.delete(LogStash::Environment::LOCKFILE)
+    end
+  end
+
   task "install-development-dependencies" => "bootstrap" do
     puts("[plugin:install-development-dependencies] Installing development dependencies")
     install_plugins("--development",  "--preserve")
@@ -26,6 +38,8 @@ namespace "plugin" do
 
   task "install-default" => "bootstrap" do
     puts("[plugin:install-default] Installing default plugins")
+
+    remove_lockfile # because we want to use the release lockfile
     install_plugins("--no-verify", "--preserve", *LogStash::RakeLib::DEFAULT_PLUGINS)
 
     task.reenable # Allow this task to be run again

diff --git a/rakelib/plugins_docs_dependencies.rake b/rakelib/plugins_docs_dependencies.rake
@@ -83,15 +83,7 @@ class PluginVersionWorking
 
     plugins_to_install.each do |plugin|
       begin
-        builder = Bundler::Dsl.new
-        gemfile = LogStash::Gemfile.new(File.new(LogStash::Environment::GEMFILE_PATH, "r+")).load
-        gemfile.update(plugin)
-
-        builder.eval_gemfile("bundler file", gemfile.generate())
-        definition = builder.to_definition(LogStash::Environment::LOCKFILE, {})
-        definition.resolve_remotely!
-        from = PLUGIN_METADATA.fetch(plugin, {}).fetch("default-plugins", false) ? :default : :missing
-        extract_versions(definition, successful_dependencies, from)
+        try_plugin(plugin, successful_dependencies)
         puts "Successfully installed: #{plugin}"
       rescue => e
         puts "Failed to install: #{plugin}"
@@ -107,13 +99,25 @@ class PluginVersionWorking
     puts "Failures: #{failures.size}/#{total}"
   end
 
+  def try_plugin(plugin, successful_dependencies)
+    builder = Bundler::Dsl.new
+    gemfile = LogStash::Gemfile.new(File.new(LogStash::Environment::GEMFILE_PATH, "r+")).load
+    gemfile.update(plugin)
+
+    builder.eval_gemfile("bundler file", gemfile.generate())
+    definition = builder.to_definition(LogStash::Environment::LOCKFILE, {})
+    definition.resolve_remotely!
+    from = PLUGIN_METADATA.fetch(plugin, {}).fetch("default-plugins", false) ? :default : :missing
+    extract_versions(definition, successful_dependencies, from)
+  end
+
   def extract_versions(definition, dependencies, from)
     #definition.specs.select { |spec| spec.metadata && spec.metadata["logstash_plugin"] == "true" }.each do |spec|
     #
     # Bundler doesn't seem to provide us with `spec.metadata` for remotely
     # discovered plugins (via rubygems.org api), so we have to choose by
     # a name pattern instead of by checking spec.metadata["logstash_plugin"]
-    definition.specs.select { |spec| spec.name =~ /^logstash-(input|filter|output|codec)-/ }.each do |spec|
+    definition.resolve.select { |spec| spec.name =~ /^logstash-(input|filter|output|codec)-/ }.each do |spec|
       dependencies[spec.name] ||= []
       dependencies[spec.name] << VersionDependencies.new(spec.version, from)
     end
@@ -135,5 +139,45 @@ task :generate_plugins_version do
   require "pluginmanager/gemfile"
   require "bootstrap/environment"
 
+  # This patch comes after an investigation of `generate_plugins_version`
+  # causing OOM during `./gradlew generatePluginsVersion`.
+  # Why does this patch fix the issue? Hang on, this is going to be wild ride:
+  # In this rake task we compute a manifest that tells us, for each logstash plugin,
+  # what is the latest version that can be installed.
+  # We do this by (again for each plugin):
+  # * adding the plugin to the current Gemfile
+  # * instantiate a `Bundler::Dsl` instance with said Gemfile
+  # * retrieve a Bundler::Definition by passing in the Gemfile.lock
+  # * call `definition.resolve_remotely!
+  #
+  # Now, these repeated calls to `resolve_remotely!` on new instances of Definitions
+  # cause the out of memory. Resolving remote dependencies uses Bundler::Worker instances
+  # who trap the SIGINT signal in their `initializer` [1]. This shared helper method creates a closure that is
+  # passed to `Signal.trap`, and capture the return [2], which is the previous proc (signal handler).
+  # Since the variable that stores the return from `Signal.trap` is present in the binding, multiple calls
+  # to this helper cause each new closures to reference the previous one. The size of each binding
+  # accumulates and OOM occurs after 70-100 iterations.
+  # This is easy to replicate by looping over `Bundler::SharedHelpers.trap("INT") { 1 }`.
+  # 
+  # This workaround removes the capture of the previous binding. Not calling all the previous handlers
+  # may cause some threads to not be cleaned up, but this rake task has a short life so everything 
+  # ends up being cleaned up on exit anyway.
+  # We're confining this patch to this task only as this is the only place where we need to resolve 
+  # dependencies many many times.
+  #
+  # You're still here? You're awesome :) Thanks for reading!
+  #
+  # [1] https://github.com/bundler/bundler/blob/d9d75807196b91f454de48d5afd0c43b395243a3/lib/bundler/worker.rb#L29
+  # [2] https://github.com/bundler/bundler/blob/d9d75807196b91f454de48d5afd0c43b395243a3/lib/bundler/shared_helpers.rb#L173
+  module ::Bundler
+    module SharedHelpers
+      def trap(signal, override = false, &block)
+        Signal.trap(signal) do
+          block.call
+        end
+      end
+    end
+  end
+
   PluginVersionWorking.new.generate
-end
+end