THREESCALE-10697: Add debug mode for the worker and add RubyMine configurations for both worker and listener

[lvillen]

What this PR does / why we need it This PR adds a debug mode for the worker and suggest RubyMine configs for both worker and listeners so developers can easily configure them in their RubyMine scripts. It comes from the request in #352, which suggested to create some scripts, but simplifies the process (thanks @mayorova).

Verification steps Following the proposed configurations at DEVELOPMENT.md create the three RubyMine configurations.

Then launch both worker a listener in debug mode with a breaking point and confirm it's working.

[mayorova]

To be honest, I don't see much value in explicit scripts for listeners. The developer needs to configure the Run Configurations in the IDE anyway, and they can just add the already existing Ruby script (bin/3scale_backend), and Script arguments, I don't think that's much more effort for a one-time setup. And it's also easier to change if the dev wants to, for example, configure the listener on another port.

Alternatively, we could add .idea/runConfigurations directory to git, and it would have the configurations pre-defined.

This is how for example the configuration looks like for falcon listener:

<component name="ProjectRunConfigurationManager">
  <configuration default="false" name="listener (falcon)" type="RubyRunConfigurationType" factoryName="Ruby">
    <module name="apisonator" />
    <RUBY_RUN_CONFIG NAME="RUBY_ARGS" VALUE="" />
    <RUBY_RUN_CONFIG NAME="WORK DIR" VALUE="$MODULE_DIR$" />
    <RUBY_RUN_CONFIG NAME="SHOULD_USE_SDK" VALUE="false" />
    <RUBY_RUN_CONFIG NAME="ALTERN_SDK_NAME" VALUE="" />
    <RUBY_RUN_CONFIG NAME="myPassParentEnvs" VALUE="true" />
    <envs>
      <env name="LISTENER_WORKERS" value="1" />
      <env name="RACK_ENV" value="development" />
    </envs>
    <EXTENSION ID="BundlerRunConfigurationExtension" BUNDLE_MODE="AUTO" bundleExecEnabled="true" />
    <EXTENSION ID="RubyCoverageRunConfigurationExtension" track_test_folders="true" runner="rcov" ENABLE_BRANCH_COVERAGE="true" ENABLE_FORKED_COVERAGE="true">
      <COVERAGE_PATTERN ENABLED="true">
        <PATTERN REGEXPS="/.rvm/" INCLUDED="false" />
      </COVERAGE_PATTERN>
    </EXTENSION>
    <EXTENSION ID="org.jetbrains.plugins.ruby.rails.run.RailsRunConfigurationExtension" SCRATCH_USE_RAILS_RUNNER="false" />
    <RUBY_RUN_CONFIG NAME="SCRIPT_PATH" VALUE="$MODULE_DIR$/bin/3scale_backend" />
    <RUBY_RUN_CONFIG NAME="SCRIPT_ARGS" VALUE="-s falcon start -p 3001" />
    <method v="2" />
  </configuration>
</component>

[mayorova]

For worker, indeed debugging doesn't work without an extra script, because 3scale_backend_worker demonizes the process, and even with --no-daemonize argument, daemonizing is simulated (IO streams get closed, and I guess this is what makes the debugger shutdown.

But IMO, rather than creating another script, just for the debugging purposes, I'd introduce a new argument in 3scale_backend_worker, say --debug, which will skip deamonizing. Something along the lines of the following (maybe there is a better way, and in any case that would need to be documented):

if ARGV.delete '--debug'
  ThreeScale::Backend::Worker.work
else
  Daemons.run_proc('3scale_backend_worker', options) do
    ThreeScale::Backend::Worker.work
  end
end

And then in the run configuration we'd just have /bin/3scale_backend_worker in Ruby script, with script arguments --debug.

[mayorova]

So, in conclusion, rather than creating extra scripts, I would just document properly how to start the listener and worker processes locally, also in a way that is suitable for debugging (for example, making sure that only one worker/thread is listening in the listener).

I think it's good to not only be able to run the app as quickly as possible, but also understand to some extent what happens behind the scenes.

[jlledom]

@mayorova booo spoilsport!! 👎👎

I agree on reuse as much as possible and not create new scripts if existing ones can be reused. However:

1. Running the worker script as non-root fails at line 25 due to access denied to /var/run/3scale. That should be corrected.
2. Simply calling ThreeScale::Backend::Worker.work is not enough for sentinels, you need to run it in a loop to simulate the failover that Kubernetes provides in production
3. You probably need to wrap ThreeScale::Backend::Worker.work under a Sync/Async call. I'm not sure all calls to Redis during the worker execution happen under a reactor. If there's only one that doesn't, it will fail without a general reactor. So better add it.
4. I imported the falcon launcher to my .idea/runConfigurations folder and it doesn't work for me. I get the error:

You have already activated json 2.5.1, but your Gemfile requires json 2.3.1. Since json is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports json as a default gem. (Gem::LoadError)

I also see that error with @lvillen scripts. That's because both scripts end up calling system or exec which create a new process and causes that error.

[mayorova]

Thanks for your comments @jlledom

Running the worker script as non-root fails at line 25 due to access denied to /var/run/3scale. That should be corrected.

Yes, this is true, I forgot about it, because at some point I created that folder manually :sweat_smile: I'd say this should also be fixed in 3scale_backend_worker itself, IMO it's not good to hardcode such a path, I think it should be configurable (it's OK to keep the current path as default).

Simply calling ThreeScale::Backend::Worker.work is not enough for sentinels, you need to run it in a loop to simulate the failover that Kubernetes provides in production

But is it only to simulate failovers? I mean, if the dev is not working on the Redis failover feature specifically, would they need it?

I remembered now your point also that it is nice to catch all exceptions and print errors in the logs and keep the worker running instead of shutting down on exceptions... But - is it a common scenario? And also - wouldn't it "mask" some incorrect behavior? (I mean if the dev just misses something, because the worker keeps running, so "looks OK")

You probably need to wrap ThreeScale::Backend::Worker.work under a Sync/Async call. I'm not sure all calls to Redis during the worker execution happen under a reactor. If there's only one that doesn't, it will fail without a general reactor. So better add it.

Shouldn't it rather be fixed by this PR? https://github.com/3scale/apisonator/pull/361/files#diff-2f501e81093c9eaa568d634cc294ab565cada69ebbc59570588c314b0f159cfe

Here (as of today) we opted for not adding a global reactor. As I understand it, currently the production runs just the 3scale_backend_worker run script, with no global reactor wrapping (unlike falcon-based listener, where, if I understand correctly, falcon takes care of creating the needed reactor).

So, if we add it just in the development script, then what would happen in production?

I imported the falcon launcher to my .idea/runConfigurations folder and it doesn't work for me. I get the error:

Hm... interesting, not sure why this would happen, I can't reproduce.

[jlledom]

Simply calling ThreeScale::Backend::Worker.work is not enough for sentinels, you need to run it in a loop to simulate the failover that Kubernetes provides in production

But is it only to simulate failovers? I mean, if the dev is not working on the Redis failover feature specifically, would they need it?

Yeah, maybe you're right.

You probably need to wrap ThreeScale::Backend::Worker.work under a Sync/Async call. I'm not sure all calls to Redis during the worker execution happen under a reactor. If there's only one that doesn't, it will fail without a general reactor. So better add it.

Shouldn't it rather be fixed by this PR? https://github.com/3scale/apisonator/pull/361/files#diff-2f501e81093c9eaa568d634cc294ab565cada69ebbc59570588c314b0f159cfe

That PR should ensure there's always a reactor for the worker tasks. OK, let's remove the Async and consider adding it back if we found a scenario where it fails.

So, if we add it just in the development script, then what would happen in production?

Sure, the more closer to production behavior, the better.

[jlledom]

With the changes from https://github.com/3scale/apisonator/pull/364 I can launch successfully the listener and the worker in both sync ans async mode. Only two issues:

1. The listener in sync mode (puma) shows me a deprecation warning:

   [DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/jlledom/.asdf/installs/ruby/3.0.2/lib/ruby/gems/3.0.0/bundler/gems/puma-e7d3c53583db/lib/puma/launcher.rb:289)

2. The listener in sync mode (puma) doesn't stop in breakpoints when being launched in debug mode.

Don't you see this errors? This is my sync listener launcher:

[lvillen]

1. The listener in sync mode (puma) shows me a deprecation warning:

[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/jlledom/.asdf/installs/ruby/3.0.2/lib/ruby/gems/3.0.0/bundler/gems/puma-e7d3c53583db/lib/puma/launcher.rb:289)

I don't see this deprecation warning. After the json upgrade I did a bundle clean and after the bundle install everything seems ok.

2. The listener in sync mode (puma) doesn't stop in breakpoints when being launched in debug mode.

I've put a breakpoint in backend.rb:56 and it is stopping.

Also, I've integrated your suggestions @jlledom in this last commit. I'm going to share here my configs hoping it helps:

Worker

Listener (puma)

Listener (falcon)

[jlledom]

It all works for me now 👍

[mayorova]

2. The listener in sync mode (puma) doesn't stop in breakpoints when being launched in debug mode.

Don't you see this errors? This is my sync listener launcher:

@jlledom The important piece here is -X "-w 0" in the arguments, it starts puma in so-called "single mode". Compare the logs without this arg:

[3736875] Puma starting in cluster mode...
[3736875] * Version 4.3.9 (ruby 3.0.2-p107), codename: Mysterious Traveller
[3736875] * Min threads: 1, max threads: 1
[3736875] * Environment: development
[3736875] * Process workers: 1
[3736875] * Phased restart available
[3736875] * Listening on tcp://0.0.0.0:3001
[3736875] Use Ctrl-C to stop
[3736875] * Starting control server on unix:///home/dmayorov/Projects/3scale/apisonator/3scale_backend.sock

and with it:

Puma starting in single mode...
* Version 4.3.9 (ruby 3.0.2-p107), codename: Mysterious Traveller
* Min threads: 1, max threads: 1
* Environment: development
* Listening on tcp://0.0.0.0:3001
* Starting control server on unix:///home/dmayorov/Projects/3scale/apisonator/3scale_backend.sock
Use Ctrl-C to stop

In the second case (with "-s ="), the breakpoints work fine for me. Also, in this case LISTENER_WORKERS env var doesn't make any difference, so no need to include it in the run configuration.

[mayorova]

The changes in bin/3scale_backend_worker look good to me. I don't know what @akostadinov thinks :grimacing:

But the PR title and description need some changes too. Also, I would like to see the RubyMine configurations somewhere, maybe in DEVELOPMENT.md file.

[jlledom]

@jlledom The important piece here is -X "-w 0" in the arguments, it starts puma in so-called "single mode". Compare the logs without this arg:

Yes, it's working now, thanks!

In the second case (with "-s ="), the breakpoints work fine for me. Also, in this case LISTENER_WORKERS env var doesn't make any difference, so no need to include it in the run configuration.

"-s =" ?

[mayorova]

"-s =" ?

I meant "-w 0" :sweat_smile:

[jlledom]

I think we can merge this one, right @lvillen @mayorova

[mayorova]

I think we can merge this one, right @lvillen @mayorova

I would like to see my comments here addressed first :innocent: