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

Add configuration to control shutdown behavior of event loop groups #3582

Closed
3 tasks done
wololock opened this issue Jun 28, 2020 · 4 comments · Fixed by #4319
Closed
3 tasks done

Add configuration to control shutdown behavior of event loop groups #3582

wololock opened this issue Jun 28, 2020 · 4 comments · Fixed by #4319
Assignees
Labels
type: improvement A minor improvement to an existing feature
Milestone

Comments

@wololock
Copy link

wololock commented Jun 28, 2020

Task List

  • Steps to reproduce provided
  • Example that reproduces the problem uploaded to Github
  • Full description of the issue provided (see below)

Description

I experienced one side effect of using a commonly shared event loop group for handling incoming requests and sending requests using the declarative HTTP client in the command-line application scenario. The default shutdownGracefully method implementation uses 2 seconds shutdown quiet period. It does not affect much long-running microservice like applications. However, it adds an unnecessary 2 seconds lag to the short-living apps like the command-line app or the serverless function. I created a demo app that calls https://api.chucknorris.io/jokes/random endpoint using Micronaut's declarative HTTP client as well as a regular Java 11 HTTP client. While the second one responds in ~300ms, the first one keeps the process running for 2 seconds after the output gets printed to the console.

asciicast

I checked if there is a simple way to override this behavior in the current state of the framework, but I couldn't find a single easy to apply solution. I tried to configure a separate eventLoopGroup for the http.client and then inject extended EventLoopGroup that overrides the default behavior of the shutdownGracefully method. I was thinking about injecting the default eventLoopGrooup and decorating it with the custom implementation of that interface, but when I've seen tons of methods that had to be decorated, I said "ok, enough."

I would like to volunteer and create a PR with a working solution, but I would like to ask what do you think about it in the first place. Micronaut's @Bean(preDestroy) allows us to define a method via string and it expects that the destroy method does not take any parameters. I see two potential options:

  • extend @Bean(preDestroy) to support a parameterized method, so one can pre-configure the bean to use zero seconds quite period when necessary,
  • provide Micronaut's wrapper for the EventLoopGroup with configurable options passed to shutdownGracefully method.

The declarative HTTP client is powerful, and I think people love it so much that they would like to use it in the command-line apps or serverless functions. However, when every cli/function execution adds 2 seconds to something that takes less than a second, it feels like something could be improved in this area.

Steps to Reproduce

git clone https://github.com/wololock/micronaut-declarative-http-client-app-demo.git

cd micronaut-declarative-http-client-app-demo

./gradlew --no-daemon assemble

sdk use java 20.1.0.r11-grl

native-image --no-server -cp build/libs/demo-cli-0.1-all.jar

time ./demo-cli dynamic-http-client

time ./demo-cli java-http-client

Expected Behaviour

There should be a way to configure an event loop group to shut down instantly.

Actual Behaviour

There is no simple way to override the default shutdownGracefully method to shut down an even loop group instantly.

Environment Information

  • Operating System: Fedora Linux 32 (kernel 5.6.19-300.fc32.x86_64)
  • Micronaut Version: 2.0.0
  • JDK Version: GraalVM CE 20.1.0 (build 11.0.7+10-jvmci-20.1-b02)

Example Application

@jameskleeh jameskleeh changed the title Using the declarative HTTP client in the command-line app keeps the process running for 2 seconds while the event loop group shuts down gracefully Add configuration to control shutdown behavior of event loop groups Jun 29, 2020
@jameskleeh jameskleeh added the type: improvement A minor improvement to an existing feature label Jun 29, 2020
@jameskleeh jameskleeh added this to the 2.1.0 milestone Jun 29, 2020
@jameskleeh
Copy link
Contributor

@wololock This would be a matter of adding support for configuration like the following in NettyHttpServerConfiguration

micronaut:
  server:
    netty:
      worker:
        shutdown:
          quiet-period: 500ms
          timeout: 500ms
      parent:
          ....

The client also can create an event loop so the configuration should probably be added there as well - HttpClientConfiguration

In addition we have configuration for event loops with DefaultEventLoopGroupConfiguration

@wololock
Copy link
Author

Thanks, @jameskleeh! Your suggestion is very helpful. I'm going to investigate this solution. Feel free to assign this ticket to me for better visibility. I will get back to you soon.

@wololock
Copy link
Author

wololock commented Jul 2, 2020

@jameskleeh I have discovered one thing and I would like to ask for your opinion. I put a breakpoint at AbstractEventExecutorGroup.shutdownGracefully() method and it turns out that it is being called by the generated $DefaultEventLoopGroupRegistry$DefaultEventLoopGroup1Definition class. Here's the body of the dispose method:

    public EventLoopGroup dispose(BeanResolutionContext var1, BeanContext var2, EventLoopGroup var3) {
        EventLoopGroup var4 = (EventLoopGroup)var3;
        super.preDestroy(var1, (DefaultBeanContext)var2, var3);
        var4.shutdownGracefully();
        return var4;
    }

The problem is that this default shutdown quiet period is not configurable at the classes implementing EventLoopGroup interface level, but rather it's just a matter of calling either noargs shutdownGracefully() or the shutdownGracefully(period,timeout,unit) one.

I guess there are many ways to solve it, but here are two things that came to my mind.

1. Add preDestroyArgs to the @Bean annotation

If present, BeanDefinitionWriter could generate a class that calls var4.shutdownGracefully(args) taken from the annotation. It's far from being perfect, and it would require pre-defining the default bean with the custom preDestroyArgs.

Question: Is possible to read micronaut.netty.event-loops.{name} configuration properties while generating the bean definition class? If so, it would make things a lot easier. We could define the defaults and create a bean definition that always generates code like var4.shutdownGracefully(quietPeriod, timeout, unit), where all three arguments are taken from the configuration file.

2. Decorate EventLoopGroup produced by DefaultEventLoopGroupRegistry.

Alternatively, we could decorate event loop groups produced by the EventLoopGroupRegistry with a dedicated type that decorates any EventLoopGroup and only overrides the default shutdownGracefully method to invoke the target method using arguments read from the configuration file.

What do you think about it? Do you see other alternatives?

@wololock
Copy link
Author

There is one more thing users creating CLI apps with Micronaut and the declarative HTTP client can do - adding System.exit(code) at the end of the command's run() method shuts down the app instantly, without waiting for the Netty to gracefully shut down all event loop groups. I looked at my previous ideas and none of them solves the problem in the most straightforward and easy to implement way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
None yet
4 participants